diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b9fd6891..95483d67 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -103,6 +103,13 @@ jobs: echo "::error::artifact is not stamped as a release build"; exit 1; } printf '%s' "$OUT" | grep -q "DIRTY" && { echo "::error::artifact reports a dirty tree"; exit 1; } || true + # `--version` speaks for the keel-trader distribution ALONE, so it cannot see a + # sibling left behind at an older version -- the failure that had `~/keel` running + # keel-trader 0.5.7 against keel-core 0.5.5. `keel versions` checks every keel + # distribution in the venv and exits non-zero when they disagree, which also proves + # the `==` pins in the wheels actually pulled the siblings from dist/. + /tmp/verify/bin/keel versions || { + echo "::error::the installed wheels do not agree on a version"; exit 1; } # The release ships a ready-for-live config as a downloadable asset. It must be in # `confirm` mode: a config that trades unattended straight off a download is exactly what @@ -168,7 +175,12 @@ jobs: echo "\`keel_trader\` wheel **by path**:" echo echo ' pip install --find-links . ./keel_trader-${{ inputs.version }}-py3-none-any.whl' - echo " keel --version" + echo " keel versions" + echo + echo "\`keel versions\` — not \`keel --version\` — is the check: it reports **every**" + echo "keel distribution in the venv and exits non-zero if a sibling was left behind at" + echo "an older version, which \`--version\` cannot see. Upgrading an existing" + echo "deployment: see \"Deploying a new version\" in the README." echo echo "⚠️ **Never install by bare name.** The distribution is \`keel-trader\`; the name" echo "\`keel\` on PyPI belongs to an unrelated project, so \`pip install keel\` fetches" diff --git a/README.md b/README.md index 89386c5c..2f6def14 100644 --- a/README.md +++ b/README.md @@ -129,25 +129,61 @@ Cutting a release is `docs/RELEASING.md`. Installing one into a deployment (e.g. commands, run **from the deployment directory** — every path below is relative to it: ```bash -gh release download v0.3.1 --repo CodeGateSoftware/keel --pattern '*.whl' --dir Release/ -uv pip install --python .venv --find-links Release Release/keel_trader-0.3.1-py3-none-any.whl -.venv/bin/keel --version +V=0.6.0 +gh release download "v$V" --repo CodeGateSoftware/keel --pattern '*.whl' --dir Release/ +uv pip install --python .venv --find-links Release \ + Release/keel_core-$V-py3-none-any.whl \ + Release/keel_broker_api-$V-py3-none-any.whl \ + Release/keel_broker_coinbase-$V-py3-none-any.whl \ + Release/keel_trader-$V-py3-none-any.whl +.venv/bin/keel versions .venv/bin/keel status ``` -Substitute the version being deployed in both of the first two lines. `--find-links Release` is -what lets the single `keel_trader` wheel resolve its `keel-core` / `keel-broker-*` siblings from -that same directory — which is why step 1 downloads them all. Installing **by path** rather than -by bare name is deliberate: `keel` on PyPI is an unrelated project, so `pip install keel` fetches a -stranger's code (see `keel/version.py`). - -Step 3 is the check that matters. It must report the version you just installed, bound to a -commit, from source `[release]`: +Set `V` to the version being deployed; nothing else changes between releases. + +**Every wheel is named, and that is the fix for a real bug.** Installing `keel_trader` alone +upgraded *only* `keel_trader`: its siblings were required without a version, so the `keel-core` +already on disk satisfied `keel-core` and stayed put. `~/keel` ran `keel-trader 0.5.7` against +`keel-core 0.5.5` for two releases that way. A wheel **path** is a direct requirement — that exact +file is installed whatever is already there — so naming all four is what actually moves them. +The wheels now also pin their siblings exactly (`Requires-Dist: keel-core==0.6.0`), which forces +the upgrade even for someone who installs `keel_trader` alone; the four paths are the same +guarantee stated where the operator can see it. + +**Not `Release/*.whl`.** The release ships *every* workspace wheel, two of which a deployment must +not have: `keel_broker_fake`, a dev-only fake venue that registers a `fake` entry point under +`keel.brokers`, and `keel_broker_robinhood`, an optional venue that pulls an Ed25519 stack +(`pynacl`, `cffi`) in for an adapter nothing constructs. The four named wheels are production's +whole dependency closure. `--find-links Release` still points at that directory so the pinned +siblings resolve locally rather than from PyPI, where they do not exist — which is why step 1 +downloads them all. Installing **by path** rather than by bare name is deliberate and unchanged: +`keel` on PyPI is an unrelated project, so `pip install keel` fetches a stranger's code (see +`keel/version.py`). + +Step 3 is the check that matters, and it is `keel versions` — **not** `keel --version`, which +could not fail. `--version` reports the `keel-trader` distribution's version and nothing else, so +it printed `0.6.0` while `keel-core` sat at `0.5.5`: a verification step blind to the failure mode, +which is worse than none, because it is trusted. `keel versions` prints the same build identity, +then every keel distribution in that venv, and **exits non-zero** when they disagree: ``` -keel 0.3.1+deb8fa7e978d [release] +keel 0.6.0+deb8fa7e978d [release] + +keel-broker-api 0.6.0 +keel-broker-coinbase 0.6.0 +keel-core 0.6.0 +keel-trader 0.6.0 + +ok: 4 keel distributions, all at 0.6.0. ``` +A partial upgrade fails it, with the numbers: `error: PARTIAL INSTALL: 4 keel distributions at 2 +different versions (0.5.5, 0.6.0)`. So does finding `keel-broker-fake` installed — it was, at +`0.5.5`, in `~/keel`. Remove it: `uv pip uninstall --python .venv keel-broker-fake`. Nothing calls +`load_broker()` today so it is inert, but that is a property of this release, not of the package, +and no reason to leave a fake venue registered on the box that moves money. + A build reporting `(DIRTY)` or `[checkout]` corresponds to no commit and **must not be run against live funds**. Step 4 is a read-only snapshot — no orders, no writes — confirming the new build opens the database and reaches the venue. diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 52cfb3a5..e96a2082 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -18,6 +18,11 @@ commit hash (`+` is the semver / PEP 440 build-metadata separator). The version this". A build reporting `(DIRTY)` or `[checkout]` corresponds to no commit and **must not be run against live funds**; `keel --version` warns loudly when so. +It answers for the `keel-trader` distribution **only**, though, and keel installs as six of them. +`keel versions` reports every one and exits non-zero if they disagree — that is the check a +deployment runs (README, "Deploying a new version"), and the release workflow runs it too against +the wheels it just built. + ## Cutting a release 1. **Bump the version in a reviewed PR.** Edit `version` in `pyproject.toml`. The release workflow @@ -36,7 +41,8 @@ against live funds**; `keel --version` warns loudly when so. | asset | what it is | |---|---| | `keel_trader--py3-none-any.whl` | the CLI. Install **by path**, never by bare name. | -| `keel_core-*`, `keel_broker_*` wheels | workspace members `keel` depends on; download them all. | +| `keel_core-*`, `keel_broker_api-*`, `keel_broker_coinbase-*` | what `keel-trader` depends on, pinned `==` to this same version. Install all four wheels by path. | +| `keel_broker_fake-*`, `keel_broker_robinhood-*` | built by `--all-packages` and published, but **not** part of a deployment: the fake is a dev-only venue, Robinhood is optional (and drags in an Ed25519 stack). Do not install them into one. | | `config.yaml` | the **production** config: real allowlist/caps in `auto_trade.mode: confirm`. | `config.yaml` is `keel/templates/config.live.yaml`, committed and reviewed like any other code. diff --git a/docs/go-live-runbook.md b/docs/go-live-runbook.md index 9fbf4ec2..65cec7d8 100644 --- a/docs/go-live-runbook.md +++ b/docs/go-live-runbook.md @@ -15,6 +15,7 @@ exchange, correctly, and we have a record of it". | | check | why | |---|---|---| | ☐ | `keel --version` reports `[release]`, no `DIRTY`, no `[checkout]` | a build that matches no commit is not reproducible; do not run it against funds | +| ☐ | `keel versions` exits **0** | `--version` speaks for the `keel-trader` distribution alone. This one checks every keel distribution in the venv and fails on a partial upgrade — new engine, old libraries — which `--version` cannot see | | ☐ | you have a **trade-enabled** CDP key (the read-only one cannot place orders) | | | ☐ | `.env` holds `CDP_API_KEY` / `CDP_API_SECRET`, and `.env` is git-ignored | credentials live only here — there is no vault | | ☐ | you are at a real terminal | confirmation and every halt-releasing command fail closed off a TTY | diff --git a/keel/cli.py b/keel/cli.py index 43cbe0aa..bd03c2d2 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -57,10 +57,11 @@ broker-touching commands (`fetch`, `agent`, `monitor`, `simulate`, `assets`) that share the `_build_broker` seam, and the remaining top-level commands. The broker-free command groups live in `keel/commands/*` and are registered here via `cli.add_command(...)`: `db`, `trials`, -`withdrawals`, `autonomy`, `rules`, `subscription`. The shared seams (`with_disclaimer`, the -confirmation gate, `_open_repo`/`_load_cfg`/`_build_broker`) live in `keel.commands._common` and -are re-imported here; `_is_interactive` is reached as `_common._is_interactive()` so a single -patch point in `keel.commands._common` drives every gate wherever its command is defined. +`withdrawals`, `autonomy`, `rules`, `subscription`, `versions`. The shared seams +(`with_disclaimer`, the confirmation gate, `_open_repo`/`_load_cfg`/`_build_broker`) live in +`keel.commands._common` and are re-imported here; `_is_interactive` is reached as +`_common._is_interactive()` so a single patch point in `keel.commands._common` drives every gate +wherever its command is defined. """ from __future__ import annotations @@ -103,6 +104,7 @@ from keel.commands.subscription import subscription_group from keel.commands.trials import trials_group from keel.commands.tui import tui_cmd +from keel.commands.versions import versions_cmd from keel.commands.withdrawals import withdrawals_group from keel.compliance import purification as purification_mod from keel.compliance import screen as screen_mod @@ -123,7 +125,7 @@ from keel.sim import tiers as tiers_mod from keel.strategy import promotion as promotion_mod from keel.types import Candle, Granularity -from keel.version import build_info +from keel.version import build_info, check_install # -- root group --------------------------------------------------------------------------------- @@ -134,6 +136,12 @@ def _print_version(ctx: click.Context, param: object, value: bool) -> None: Prints the working-tree state too. For a tool that can place orders, "0.1.0 (abc123, DIRTY)" and "0.1.0 (abc123)" are materially different claims -- the first corresponds to no commit and cannot be reproduced. + + The line describes the `keel-trader` distribution ONLY, which is exactly how a deployment came + to run `keel-trader 0.5.7` against `keel-core 0.5.5` while this reported the new number. It + cannot be widened without changing what `--version` means, so instead it warns when the rest + of the install disagrees and points at `keel versions`, which reports all of them and exits + non-zero. The warning goes to stderr so the string this prints stays exactly what it was. """ if not value or ctx.resilient_parsing: return @@ -145,6 +153,12 @@ def _print_version(ctx: click.Context, param: object, value: bool) -> None: "Do not run it against live funds.", err=True, ) + if not check_install(source=info.source).is_consistent: + click.echo( + "warning: PARTIAL INSTALL -- this line reports the keel-trader distribution only, " + "and the other keel distributions do not agree with it. Run `keel versions`.", + err=True, + ) ctx.exit() @@ -2159,6 +2173,14 @@ def simulate( cli.add_command(insights_group) +# -- versions (the deploy check: every keel distribution, not just this one) --------------------- + +# `--version` above answers for `keel-trader` alone and therefore cannot see a partial upgrade; +# this reports the whole install and exits non-zero when it disagrees with itself. Defined in +# `keel.commands.versions` and registered here. +cli.add_command(versions_cmd) + + # -- kill / resume ------------------------------------------------------------------------------ diff --git a/keel/commands/versions.py b/keel/commands/versions.py new file mode 100644 index 00000000..e7ea5bde --- /dev/null +++ b/keel/commands/versions.py @@ -0,0 +1,66 @@ +"""`keel versions` -- the deploy check that can actually fail. + +`keel --version` answers "which build is this?" for the `keel-trader` distribution and nothing +else, so it cannot see the failure it is used to rule out: installing the `keel_trader` wheel +alone leaves `keel-core` and the adapters at their old versions, and `--version` still prints the +new number. `~/keel` ran `keel-trader 0.5.7` against `keel-core 0.5.5` across two releases with +that check passing every time. A verification step blind to the failure mode is worse than none, +because it is trusted. + +This command prints the same build-identity line and then every `keel-*` distribution installed +in the running interpreter's environment, and **exits non-zero** when they disagree -- so it can +be the last line of a deploy script and mean something. The rules live in +`keel.version.InstallReport.problems`, which is a pure value and unit-tested without installing +anything; this module is only the rendering and the exit code. + +No config, no database, no network: nothing about it can fail for environmental reasons, which is +what makes a non-zero exit unambiguous. + +**Why a CLI command and not `scripts/`.** `scripts/` is operator tooling that is not shipped in +the wheel, and a deployment is a `.venv` beside a `Release/` directory with no checkout of this +repository at all -- a script there could not be run without first fetching it. The check has to +travel inside the artifact it is checking. +""" + +from __future__ import annotations + +import click + +from keel.version import build_info, check_install + + +@click.command("versions") +@click.pass_context +def versions_cmd(ctx: click.Context) -> None: + """Verify the whole install: every keel distribution's version, not just keel-trader's.""" + info = build_info() + report = check_install(source=info.source) + + click.echo(info.describe()) + if not info.is_reproducible: + click.echo( + "warning: this build is NOT reproducible -- it does not correspond to a commit. " + "Do not run it against live funds.", + err=True, + ) + + if not report.distributions: + # Nothing installed: a source checkout run via `uv run` with the workspace on the path. + # There is no install to disagree with itself, so there is nothing to fail on. + click.echo("no keel distributions installed -- nothing to compare.") + return + + width = max(len(name) for name in report.distributions) + 2 + click.echo("") + for name, version in sorted(report.distributions.items()): + click.echo(f"{name.ljust(width)}{version}") + click.echo("") + + if not report.problems: + n = len(report.distributions) + click.echo(f"ok: {n} keel distributions, all at {report.versions[0]}.") + return + + for problem in report.problems: + click.echo(f"error: {problem}", err=True) + ctx.exit(1) diff --git a/keel/version.py b/keel/version.py index 1ae9c1ef..56cc608a 100644 --- a/keel/version.py +++ b/keel/version.py @@ -15,10 +15,19 @@ Falls back to `unknown` rather than raising: failing to identify the build is a reason to warn loudly, not a reason to prevent the tool from starting. + +**One build identity is not enough.** `BuildInfo` describes the `keel-trader` distribution only, +and keel is installed as *several* distributions (`keel-core`, `keel-broker-*`). A deployment +upgraded by installing the `keel_trader` wheel alone leaves the rest at whatever version they +were -- `~/keel` ran `keel-trader 0.5.7` against `keel-core 0.5.5` for two releases, and +`keel --version` reported the new number throughout, because that is all it can see. So this +module also reports the *install*: `check_install()` reads every `keel-*` distribution present in +the running interpreter's environment, which is the only view that can show a partial upgrade. """ from __future__ import annotations +import re import subprocess from dataclasses import dataclass from importlib import metadata @@ -141,3 +150,116 @@ def build_info() -> BuildInfo: dirty=status is None or bool(status), source="checkout", ) + + +# -- the whole install, not just this distribution ---------------------------------------------- + +#: Everything the workspace publishes is named `keel-`, so a prefix match enumerates +#: the install without a hand-maintained list that a new package would silently fall out of. The +#: bare name `keel` is excluded on purpose: it belongs to an unrelated PyPI project (see +#: `DISTRIBUTION`), and folding a stranger's version number into this check would be nonsense. +_FAMILY_PREFIX = "keel-" + +#: Distributions that must not exist in a deployment. `keel-broker-fake` is a dev-only fake venue +#: (see the `dev` group in `pyproject.toml`) whose reason to exist is proving two-plugin +#: discovery -- it registers a `fake` entry under `keel.brokers`, so an engine that has it +#: installed advertises a venue that trades nothing. It was found installed in `~/keel`. Nothing +#: calls `load_broker()` today, so it is inert; "inert" is a property of this release, not of the +#: package, and is not a reason to leave it on a box that moves money. +DEV_ONLY_DISTRIBUTIONS = frozenset({"keel-broker-fake"}) + + +def _canonical(name: str) -> str: + """PEP 503 normalisation: `keel_broker_api` and `Keel-Broker-API` are the same distribution.""" + return re.sub(r"[-_.]+", "-", name).strip().lower() + + +@dataclass(frozen=True) +class InstallReport: + """Every `keel-*` distribution visible to the running interpreter, and what is wrong with it. + + A pure value: `check_install()` builds it from the environment, everything else here is + derived, so the rules below are testable without installing anything. + """ + + #: canonical distribution name -> version, e.g. `{"keel-core": "0.6.0"}`. + distributions: dict[str, str] + #: `build_info().source` at the time of the check; `"release"` means a deployment. + source: str = "unknown" + + @property + def versions(self) -> list[str]: + return sorted(set(self.distributions.values())) + + @property + def is_consistent(self) -> bool: + """False when the distributions disagree -- i.e. a partial upgrade. + + An empty install is vacuously consistent: running from a source checkout with nothing + installed is a legitimate state, and it is not this check's job to invent a failure. + """ + return len(self.versions) <= 1 + + @property + def dev_only_installed(self) -> list[str]: + return sorted(n for n in self.distributions if n in DEV_ONLY_DISTRIBUTIONS) + + @property + def problems(self) -> list[str]: + """Every reason this install must not be trusted, worst first. Empty means healthy. + + A dev-only package is a problem in a **release** build only: a checkout is exactly where + `keel-broker-fake` is supposed to be, and a check that cried wolf on every developer's + machine would be ignored by the time it mattered. Build state (`DIRTY`, `[checkout]`) is + deliberately NOT a problem here -- `describe()` already says it, and this report is about + what is installed, not about which commit it came from. + """ + found: list[str] = [] + if not self.is_consistent: + found.append( + f"PARTIAL INSTALL: {len(self.distributions)} keel distributions at " + f"{len(self.versions)} different versions ({', '.join(self.versions)}). " + "`keel --version` reports keel-trader's version alone and cannot see this. " + "Reinstall every wheel by path (README, 'Deploying a new version')." + ) + if self.source == "release": + for name in self.dev_only_installed: + found.append( + f"{name} is installed. It is a dev-only package that registers a venue " + "entry point and must not exist in a deployment. Remove it: " + f"`uv pip uninstall --python .venv {name}`." + ) + return found + + +def installed_distributions() -> dict[str, str]: + """Canonical name -> version for every installed `keel-*` distribution. Never raises. + + Scoped to the interpreter that is running, so `.venv/bin/keel` reports that venv -- which is + what makes this answerable in a deployment with no repository and no git. + """ + found: dict[str, str] = {} + try: + dists = list(metadata.distributions()) + except Exception: # pragma: no cover -- a broken environment must not stop the CLI + return found + for dist in dists: + try: + name = _canonical(dist.metadata["Name"] or "") + version = dist.version + except Exception: # pragma: no cover -- one unreadable dist must not hide the rest + continue + if not name.startswith(_FAMILY_PREFIX): + continue + # First occurrence wins: that is the copy the import machinery resolves, so it is the + # code that would actually run. A second copy further down `sys.path` is unreachable. + found.setdefault(name, version) + return found + + +def check_install(source: str | None = None) -> InstallReport: + """Read the environment into an `InstallReport`. `source` defaults to this build's.""" + return InstallReport( + distributions=installed_distributions(), + source=build_info().source if source is None else source, + ) diff --git a/packages/keel-broker-api/pyproject.toml b/packages/keel-broker-api/pyproject.toml index 0150fa7e..59b12d63 100644 --- a/packages/keel-broker-api/pyproject.toml +++ b/packages/keel-broker-api/pyproject.toml @@ -3,7 +3,9 @@ name = "keel-broker-api" version = "0.6.0" description = "Broker port, domain types, capability model, and conformance suite for keel" requires-python = ">=3.14.4" -dependencies = ["keel-core"] +# Pinned `==`: see the note in the root `pyproject.toml`. An unpinned sibling is satisfied by +# whatever is already installed, which is how a deployment ends up running mixed versions. +dependencies = ["keel-core==0.6.0"] [project.optional-dependencies] conformance = ["pytest>=9.1.1"] diff --git a/packages/keel-broker-coinbase/pyproject.toml b/packages/keel-broker-coinbase/pyproject.toml index edceb8d5..122bd8cc 100644 --- a/packages/keel-broker-coinbase/pyproject.toml +++ b/packages/keel-broker-coinbase/pyproject.toml @@ -3,7 +3,9 @@ name = "keel-broker-coinbase" version = "0.6.0" description = "Coinbase Advanced Trade adapter for keel" requires-python = ">=3.14.4" -dependencies = ["keel-core", "keel-broker-api", "coinbase-advanced-py>=1.8.4"] +# Siblings pinned `==` (see the root `pyproject.toml`); the third-party SDK is not, because it +# has its own release cycle and is not cut from this repo. +dependencies = ["keel-core==0.6.0", "keel-broker-api==0.6.0", "coinbase-advanced-py>=1.8.4"] [project.entry-points."keel.brokers"] coinbase = "keel_broker_coinbase:CoinbaseAdapter" diff --git a/packages/keel-broker-fake/pyproject.toml b/packages/keel-broker-fake/pyproject.toml index 9fcab4a0..67423aaa 100644 --- a/packages/keel-broker-fake/pyproject.toml +++ b/packages/keel-broker-fake/pyproject.toml @@ -3,7 +3,8 @@ name = "keel-broker-fake" version = "0.6.0" description = "A deliberately divergent second broker adapter, to keep the port honest" requires-python = ">=3.14.4" -dependencies = ["keel-core", "keel-broker-api"] +# Pinned `==`: see the note in the root `pyproject.toml`. +dependencies = ["keel-core==0.6.0", "keel-broker-api==0.6.0"] [project.entry-points."keel.brokers"] fake = "keel_broker_fake:FakeAdapter" diff --git a/packages/keel-broker-robinhood/pyproject.toml b/packages/keel-broker-robinhood/pyproject.toml index cea49804..d917d7f7 100644 --- a/packages/keel-broker-robinhood/pyproject.toml +++ b/packages/keel-broker-robinhood/pyproject.toml @@ -7,7 +7,7 @@ requires-python = ">=3.14.4" # Ed25519 key, which no other venue keel talks to requires. Keeping it a dependency of this # package alone means an engine that never installs this adapter never installs the crypto stack # either -- the whole point of adapters being separate distributions. -dependencies = ["keel-core", "keel-broker-api", "pynacl>=1.5.0", "requests>=2.32.0"] +dependencies = ["keel-core==0.6.0", "keel-broker-api==0.6.0", "pynacl>=1.5.0", "requests>=2.32.0"] [project.entry-points."keel.brokers"] robinhood = "keel_broker_robinhood:RobinhoodAdapter" diff --git a/pyproject.toml b/pyproject.toml index 509b303e..d768e383 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,11 +13,20 @@ dependencies = [ # them any more (config parsing and secret loading both moved into keel-core), so they # arrive transitively via the `keel-core` dependency below. Re-add them here only if # `keel/` starts importing them directly again. - "keel-core", - "keel-broker-api", + # ⚠️ The workspace siblings are pinned `==` to this exact version, and every workspace + # pyproject does the same. Unpinned, an ALREADY-INSTALLED older sibling satisfies the + # requirement, so installing the new `keel_trader` wheel upgrades nothing else and leaves a + # deployment running new code against old libraries -- `~/keel` ran `keel-trader 0.5.7` + # against `keel-core 0.5.5` for two releases exactly that way. These are not third-party + # dependencies with independent release cycles: they are cut from this repo, at this version, + # in one build, so `==` states a fact rather than guessing at compatibility. The cost is that + # a version bump must move these numbers too -- the bump already edits every package's + # `version`, and `tests/test_packaging.py` fails the build if a pin is left behind. + "keel-core==0.6.0", + "keel-broker-api==0.6.0", # keel/cli.py and keel/data/cb_client.py still import the Coinbase SDK directly; depending # on the adapter keeps it available transitively. Phase B deletes those imports and this. - "keel-broker-coinbase", + "keel-broker-coinbase==0.6.0", ] [project.scripts] diff --git a/tests/commands/test_versions.py b/tests/commands/test_versions.py new file mode 100644 index 00000000..34374564 --- /dev/null +++ b/tests/commands/test_versions.py @@ -0,0 +1,91 @@ +"""Tests for `keel versions` -- the deploy check that must be able to FAIL. + +The rules it enforces are `keel.version.InstallReport`'s and are tested as pure values in +`tests/test_version.py`. What is left here is the part a deploy script depends on: the exit code, +and that every installed distribution actually reaches the operator's screen. Both are driven +through `CliRunner` with the environment scan stubbed, so the tests describe an install rather +than requiring one. +""" + +from __future__ import annotations + +from click.testing import CliRunner + +from keel.cli import cli +from keel.version import BuildInfo, InstallReport + +_RELEASE = BuildInfo(version="0.6.0", commit="deadbeef", dirty=False, source="release") + + +def _install(monkeypatch, dists, source="release", info=_RELEASE): + monkeypatch.setattr("keel.commands.versions.build_info", lambda: info) + monkeypatch.setattr( + "keel.commands.versions.check_install", + lambda source=source: InstallReport(distributions=dict(dists), source=source), + ) + + +def test_a_healthy_install_exits_zero_and_lists_every_distribution(monkeypatch): + _install( + monkeypatch, + {"keel-trader": "0.6.0", "keel-core": "0.6.0", "keel-broker-api": "0.6.0"}, + ) + result = CliRunner().invoke(cli, ["versions"]) + assert result.exit_code == 0 + assert "keel 0.6.0+deadbeef [release]" in result.output + for name in ("keel-trader", "keel-core", "keel-broker-api"): + assert name in result.output + assert "ok: 3 keel distributions, all at 0.6.0." in result.output + + +def test_a_partial_upgrade_exits_NON_ZERO(monkeypatch): + """A check that cannot fail is worse than none. This is the one that has to.""" + _install(monkeypatch, {"keel-trader": "0.6.0", "keel-core": "0.5.5"}) + result = CliRunner().invoke(cli, ["versions"]) + assert result.exit_code == 1 + assert "PARTIAL INSTALL" in result.output + assert "0.5.5" in result.output and "0.6.0" in result.output + + +def test_the_dev_only_fake_venue_exits_NON_ZERO_in_a_deployment(monkeypatch): + _install(monkeypatch, {"keel-trader": "0.6.0", "keel-broker-fake": "0.6.0"}) + result = CliRunner().invoke(cli, ["versions"]) + assert result.exit_code == 1 + assert "keel-broker-fake" in result.output + assert "uv pip uninstall" in result.output + + +def test_the_dev_only_fake_venue_does_not_fail_a_checkout(monkeypatch): + checkout = BuildInfo(version="0.6.0", commit="deadbeef", dirty=False, source="checkout") + _install( + monkeypatch, + {"keel-trader": "0.6.0", "keel-broker-fake": "0.6.0"}, + source="checkout", + info=checkout, + ) + result = CliRunner().invoke(cli, ["versions"]) + assert result.exit_code == 0 + + +def test_an_uninstalled_checkout_reports_nothing_to_compare(monkeypatch): + checkout = BuildInfo(version="0.6.0", commit="deadbeef", dirty=False, source="checkout") + _install(monkeypatch, {}, source="checkout", info=checkout) + result = CliRunner().invoke(cli, ["versions"]) + assert result.exit_code == 0 + assert "nothing to compare" in result.output + + +def test_a_non_reproducible_build_still_warns(monkeypatch): + dirty = BuildInfo(version="0.6.0", commit="deadbeef", dirty=True, source="checkout") + _install(monkeypatch, {"keel-trader": "0.6.0"}, source="checkout", info=dirty) + result = CliRunner().invoke(cli, ["versions"]) + assert "NOT reproducible" in result.output + assert result.exit_code == 0 # build state is `--version`'s claim, not this command's gate + + +def test_it_needs_no_config_and_no_database(tmp_path, monkeypatch): + """Nothing environmental may make this fail, or a non-zero exit stops meaning anything.""" + _install(monkeypatch, {"keel-trader": "0.6.0"}) + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(cli, ["--db", "nope.db", "--config", "nope.yaml", "versions"]) + assert result.exit_code == 0 diff --git a/tests/test_packaging.py b/tests/test_packaging.py new file mode 100644 index 00000000..6c6ae542 --- /dev/null +++ b/tests/test_packaging.py @@ -0,0 +1,75 @@ +"""The workspace's own dependency metadata: one version, pinned everywhere. + +keel ships as six distributions cut from this repo in a single build, so "which version" is one +answer, not six. Left unpinned, a `keel-core` requirement is satisfied by whatever `keel-core` is +already installed, and installing the new `keel_trader` wheel upgrades nothing else -- which is +how `~/keel` came to run `keel-trader 0.5.7` against `keel-core 0.5.5` across two releases with +`keel --version` reporting the new number the whole time. + +The pins are what make that impossible regardless of how someone installs. Their cost is that a +version bump has to move them too, and a forgotten pin is silent: the build still succeeds and the +wheel still installs, it just stops forcing the upgrade. These tests are what make it loud instead. +They read the `pyproject.toml` files from the source tree, so they are a repo-hygiene check, not a +runtime one -- `keel versions` is the runtime side, and it checks what is actually installed. +""" + +from __future__ import annotations + +import tomllib +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] + + +def _pyprojects() -> dict[str, dict]: + """distribution name -> parsed `pyproject.toml`, for the root and every workspace member.""" + found = {} + for path in [_ROOT / "pyproject.toml", *sorted((_ROOT / "packages").glob("*/pyproject.toml"))]: + data = tomllib.loads(path.read_text()) + found[data["project"]["name"]] = data + return found + + +def _requirement_name(spec: str) -> str: + """`keel-core==0.6.0` -> `keel-core`; + `coinbase-advanced-py>=1.8.4` -> `coinbase-advanced-py`.""" + for sep in ("==", ">=", "<=", "~=", "!=", ">", "<", "[", ";", " "): + spec = spec.split(sep)[0] + return spec.strip().lower().replace("_", "-") + + +def test_every_workspace_package_carries_the_same_version(): + """Six distributions, one release. A version that moved alone is a packaging bug.""" + versions = {name: data["project"]["version"] for name, data in _pyprojects().items()} + assert len(set(versions.values())) == 1, f"workspace versions disagree: {versions}" + + +@pytest.mark.parametrize("name", sorted(_pyprojects())) +def test_workspace_siblings_are_pinned_to_the_exact_version(name): + """Every intra-workspace dependency must be `==`, in every package. + + Not `>=`: a lower bound is satisfied by a newer sibling too, and the point is to forbid a + MIXED install, not merely an old one. + """ + projects = _pyprojects() + version = projects[name]["project"]["version"] + for spec in projects[name]["project"].get("dependencies", []): + dep = _requirement_name(spec) + if dep not in projects: + continue # a third-party dependency, with its own release cycle + assert spec == f"{dep}=={version}", ( + f"{name} depends on {spec!r}; it must be '{dep}=={version}' so that installing " + f"{name} cannot leave an older {dep} in place" + ) + + +def test_the_dev_only_fake_venue_is_not_a_runtime_dependency_of_anything(): + """It registers a `fake` venue entry point; a deployment must never install it. + + `keel versions` fails a release build that has it. This asserts nothing can drag it in. + """ + for name, data in _pyprojects().items(): + deps = [_requirement_name(s) for s in data["project"].get("dependencies", [])] + assert "keel-broker-fake" not in deps, f"{name} must not depend on keel-broker-fake" diff --git a/tests/test_version.py b/tests/test_version.py index 09c213fc..017e074f 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -170,3 +170,128 @@ def test_describe_still_flags_dirty_and_source(): assert "DIRTY" in dirty and "checkout" in dirty clean = BuildInfo(version="0.1.0", commit="abc", dirty=False, source="release").describe() assert "DIRTY" not in clean and "release" in clean + + +# -- the whole install (InstallReport) ------------------------------------------------------ + + +def _report(dists, source="release"): + return version_mod.InstallReport(distributions=dict(dists), source=source) + + +def test_an_install_at_one_version_is_healthy(): + report = _report({"keel-trader": "0.6.0", "keel-core": "0.6.0"}) + assert report.is_consistent is True + assert report.problems == [] + assert report.versions == ["0.6.0"] + + +def test_the_real_deployment_failure_is_caught(): + """`keel-trader 0.5.7` against `keel-core 0.5.5` -- what `--version` could not see.""" + report = _report({"keel-trader": "0.5.7", "keel-core": "0.5.5", "keel-broker-api": "0.5.5"}) + assert report.is_consistent is False + assert len(report.problems) == 1 + assert "PARTIAL INSTALL" in report.problems[0] + assert "0.5.5, 0.5.7" in report.problems[0] + + +def test_an_empty_install_is_not_invented_into_a_failure(): + """A source checkout with nothing installed is a legitimate state, not a partial upgrade.""" + report = _report({}) + assert report.is_consistent is True + assert report.problems == [] + + +def test_the_dev_only_fake_venue_fails_a_RELEASE_build(): + report = _report({"keel-trader": "0.6.0", "keel-broker-fake": "0.6.0"}, source="release") + assert report.is_consistent is True # versions agree; the package itself is the problem + assert [p for p in report.problems if "keel-broker-fake" in p] + + +def test_the_dev_only_fake_venue_is_fine_in_a_CHECKOUT(): + """A checkout is exactly where it belongs; crying wolf there would train the check away.""" + report = _report({"keel-trader": "0.6.0", "keel-broker-fake": "0.6.0"}, source="checkout") + assert report.dev_only_installed == ["keel-broker-fake"] + assert report.problems == [] + + +def test_a_dirty_or_checkout_build_is_NOT_an_install_problem(): + """`describe()` already reports build state; this report is about what is installed.""" + assert _report({"keel-trader": "0.6.0"}, source="checkout").problems == [] + assert _report({"keel-trader": "0.6.0"}, source="unknown").problems == [] + + +def test_distribution_names_are_canonicalised(): + assert version_mod._canonical("Keel_Broker_API") == "keel-broker-api" + + +def test_only_the_keel_family_is_enumerated(monkeypatch): + """The bare name `keel` on PyPI is a stranger's project -- its version means nothing here.""" + + class FakeDist: + def __init__(self, name, ver): + self.metadata = {"Name": name} + self.version = ver + + monkeypatch.setattr( + version_mod.metadata, + "distributions", + lambda: [ + FakeDist("keel_trader", "0.6.0"), + FakeDist("keel-core", "0.6.0"), + FakeDist("keel", "9.9.9"), # the unrelated PyPI project + FakeDist("click", "8.4.2"), + ], + ) + assert version_mod.installed_distributions() == {"keel-trader": "0.6.0", "keel-core": "0.6.0"} + + +def test_the_first_copy_on_the_path_wins(monkeypatch): + """A second copy further down `sys.path` is unreachable, so it must not be reported.""" + + class FakeDist: + def __init__(self, name, ver): + self.metadata = {"Name": name} + self.version = ver + + monkeypatch.setattr( + version_mod.metadata, + "distributions", + lambda: [FakeDist("keel-core", "0.6.0"), FakeDist("keel-core", "0.1.0")], + ) + assert version_mod.installed_distributions() == {"keel-core": "0.6.0"} + + +def test_enumeration_never_raises(monkeypatch): + """A broken environment is a reason to report nothing, not to stop the CLI.""" + + def boom(): + raise RuntimeError("no metadata here") + + monkeypatch.setattr(version_mod.metadata, "distributions", boom) + assert version_mod.installed_distributions() == {} + + +def test_version_flag_warns_when_the_rest_of_the_install_disagrees(monkeypatch): + """The whole point: the line still says 0.6.0, but it no longer says it alone.""" + monkeypatch.setattr( + "keel.cli.build_info", + lambda: BuildInfo(version="0.6.0", commit="deadbeef", dirty=False, source="release"), + ) + monkeypatch.setattr( + "keel.cli.check_install", + lambda source=None: _report({"keel-trader": "0.6.0", "keel-core": "0.5.5"}), + ) + result = CliRunner().invoke(cli, ["--version"]) + assert result.exit_code == 0 # still a diagnostic; `keel versions` is the gate + assert "keel 0.6.0+deadbeef [release]" in result.output + assert "PARTIAL INSTALL" in result.output + + +def test_version_flag_says_nothing_extra_when_the_install_agrees(monkeypatch): + monkeypatch.setattr( + "keel.cli.check_install", + lambda source=None: _report({"keel-trader": "0.6.0", "keel-core": "0.6.0"}), + ) + result = CliRunner().invoke(cli, ["--version"]) + assert "PARTIAL INSTALL" not in result.output