From bf235d5d432715acdde16592a08ce3e6f1eb7485 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Mon, 20 Jul 2026 16:11:03 -0400 Subject: [PATCH] feat: build identity (--version) + manual release workflow Prerequisites for running against live funds. Verified state before this: NO CI at all, no git tags, version never moved off 0.1.0, no --version -- "run it live" meant running whatever happened to be checked out. keel --version reports version + commit + WORKING-TREE STATE. The last part is the point: "0.1.0 (abc123, DIRTY)" and "0.1.0 (abc123)" are materially different claims, and the first corresponds to no commit at all. Non-reproducible builds print a loud warning against live use. Two sources: keel/_build_info.py stamped by the release workflow (so an INSTALLED artifact identifies itself with no git and no repo present), else git. A STALE stamp in a modified checkout would otherwise claim [release] and hide a dirty tree -- the exact misreport this exists to prevent -- so when git disagrees with the stamp, git wins. Workflows: ci.yml (tests+ruff on push/PR, plus a --version smoke) and release.yml (workflow_dispatch ONLY -- nothing that moves money ships on a merge). The release job refuses to bump the version itself: that is a human decision in a reviewed PR, so CI never writes to main. It also refuses a non-semver input, a mismatch with pyproject, or an existing tag. TWO REAL PROBLEMS FOUND BY TESTING THE RELEASE PATH LOCALLY RATHER THAN TRUSTING THE YAML: 1. uv build alone produces an UNINSTALLABLE wheel -- keel depends on workspace members (keel-core, keel-broker-*) published nowhere. Fixed with --all-packages plus --find-links. 2. THE DISTRIBUTION NAME "keel" IS TAKEN ON PyPI by an unrelated project ("Kill proccesses effectively and easily"). pip preferred it over our local wheel and silently installed a STRANGER'S PACKAGE. For a tool that places orders with a live API key that is a supply-chain hazard, not a cosmetic clash. Distribution renamed to keel-trader; the import package and the CLI command both stay keel. Release notes instruct installing BY PATH and warn against installing by name. Verified end to end locally: stamp -> uv build --all-packages -> install into a clean venv by path -> keel --version from outside any git repo reports "keel 0.1.0 () [release]", no DIRTY, exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 42 ++++++++++ .github/workflows/release.yml | 133 ++++++++++++++++++++++++++++++ .gitignore | 3 + keel/cli.py | 29 +++++++ keel/version.py | 131 +++++++++++++++++++++++++++++ pyproject.toml | 6 +- tests/test_version.py | 151 ++++++++++++++++++++++++++++++++++ uv.lock | 76 ++++++++--------- 8 files changed, 532 insertions(+), 39 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/release.yml create mode 100644 keel/version.py create mode 100644 tests/test_version.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..bf3d70b6 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,42 @@ +name: CI + +# The release workflow refuses to publish unless tests pass. That guarantee is only worth +# something if tests also run on the way IN -- otherwise `main` can drift red between releases +# and the gate discovers it at the worst moment. +on: + push: + branches: [main] + pull_request: + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@v4 + + - name: Install uv + uses: astral-sh/setup-uv@v6 + with: + enable-cache: true + + - name: Set up Python + run: uv python install 3.12 + + - name: Sync dependencies + run: uv sync --all-extras --dev + + - name: Lint + run: uv run ruff check keel tests packages + + - name: Test + run: uv run pytest -q + + # A build that cannot identify itself must not reach a release. This also catches an + # import-time break in the CLI, which `pytest` alone would not surface as sharply. + - name: Build identity + run: uv run keel --version diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..aa061f4f --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,133 @@ +name: Release + +# MANUAL ONLY. Nothing about a money-moving tool should ship on a merge. +# +# The workflow deliberately does NOT bump the version itself: a version bump is a human decision +# and belongs in a reviewed PR. This job asserts that `pyproject.toml` already carries the version +# being released and fails loudly otherwise, so CI never writes to `main`. +on: + workflow_dispatch: + inputs: + version: + description: "Semver to release, without a leading v (e.g. 0.2.0). Must already match pyproject.toml." + required: true + type: string + +permissions: + contents: write # create the tag and the release + +jobs: + release: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 # tags, and a real commit history for the build stamp + + - name: Validate the version input + run: | + set -euo pipefail + VERSION="${{ inputs.version }}" + if ! printf '%s' "$VERSION" | grep -Eq '^[0-9]+\.[0-9]+\.[0-9]+$'; then + echo "::error::'$VERSION' is not a bare semver (expected N.N.N, no leading v)"; exit 1 + fi + PYPROJECT="$(grep -m1 '^version' pyproject.toml | cut -d'"' -f2)" + if [ "$PYPROJECT" != "$VERSION" ]; then + echo "::error::pyproject.toml says '$PYPROJECT' but you asked to release '$VERSION'." + echo "::error::Bump the version in a reviewed PR first -- this workflow will not write to main." + exit 1 + fi + if git rev-parse "v$VERSION" >/dev/null 2>&1; then + echo "::error::tag v$VERSION already exists -- releases are immutable"; exit 1 + fi + + - name: Install uv + uses: astral-sh/setup-uv@v6 + with: + enable-cache: true + + - name: Set up Python + run: uv python install 3.12 + + - name: Sync dependencies + run: uv sync --all-extras --dev + + # The release gate. A red suite must never produce an artifact that could touch funds. + - name: Lint + run: uv run ruff check keel tests packages + + - name: Test + run: uv run pytest -q + + # Stamp the build so an INSTALLED artifact can identify itself with no git and no repo + # present. `keel/version.py` prefers this over shelling out to git. + - name: Stamp build info + run: | + set -euo pipefail + COMMIT="$(git rev-parse --short=12 HEAD)" + cat > keel/_build_info.py < Any: # pragma: no cover -- exercised only # -- root group --------------------------------------------------------------------------------- +def _print_version(ctx: click.Context, param: object, value: bool) -> None: + """Eager `--version`: print the build identity and exit before any command runs. + + 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. + """ + if not value or ctx.resilient_parsing: + return + info = build_info() + 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, + ) + ctx.exit() + + @click.group() +@click.option( + "--version", + is_flag=True, + callback=_print_version, + expose_value=False, + is_eager=True, + help="Show the running version, commit and working-tree state, then exit.", +) @click.option( "--db", "db_path", default=DEFAULT_DB_PATH, show_default=True, help="SQLite DB path." ) diff --git a/keel/version.py b/keel/version.py new file mode 100644 index 00000000..91ab6fc9 --- /dev/null +++ b/keel/version.py @@ -0,0 +1,131 @@ +"""Report exactly which code is running (version + commit + working-tree state). + +For a tool that can move money, *"which build was that?"* has to have an answer. It currently +does not: `version` in `pyproject.toml` has never moved off `0.1.0`, there are no tags, and +`uv run keel` executes whatever happens to be checked out -- including a half-finished edit. + +Two sources, in priority order: + +1. **`keel/_build_info.py`**, written by the release workflow immediately before `uv build`. An + installed release therefore reports the exact commit it was built from, with no git and no + repository present at runtime. +2. **git**, when running from a checkout. Reports the working commit AND whether the tree is + **dirty** -- the distinction that matters most here, because a dirty tree means the running + code corresponds to no commit at all and the run is not reproducible. + +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. +""" + +from __future__ import annotations + +import subprocess +from dataclasses import dataclass +from importlib import metadata + +_GIT_TIMEOUT_SEC = 3 + + +@dataclass(frozen=True) +class BuildInfo: + version: str + commit: str + dirty: bool + source: str # "release" | "checkout" | "unknown" + + @property + def is_reproducible(self) -> bool: + """False when the running code corresponds to no commit -- a dirty tree, or no idea. + + A `release` build is only reproducible if it is also clean: a stale stamp in a modified + checkout is exactly the case that must not pass. + """ + return self.source in {"release", "checkout"} and not self.dirty + + def describe(self) -> str: + parts = [f"keel {self.version}"] + if self.commit != "unknown": + parts.append(f"({self.commit}{', DIRTY' if self.dirty else ''})") + parts.append(f"[{self.source}]") + return " ".join(parts) + + +#: The DISTRIBUTION name. Deliberately not "keel": that name is already taken on PyPI by an +#: unrelated project ("Kill proccesses effectively and easily"), so `pip install keel` fetches a +#: stranger's package. For a tool that places live orders, an install path that can resolve to +#: someone else's code is a supply-chain hazard, not a cosmetic clash. The IMPORT package and the +#: CLI command both remain `keel`; only the distribution is renamed. +DISTRIBUTION = "keel-trader" + + +def _package_version() -> str: + for name in (DISTRIBUTION, "keel"): + try: + return metadata.version(name) + except metadata.PackageNotFoundError: + continue + return "unknown" + + +def _git(*args: str) -> str | None: + try: + result = subprocess.run( + ["git", *args], + capture_output=True, + text=True, + timeout=_GIT_TIMEOUT_SEC, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + if result.returncode != 0: + return None + return result.stdout.strip() + + +def _embedded(): + """The release stamp, or `None`. A seam so tests can exercise the git path deterministically + regardless of whether a stamp happens to exist on the machine.""" + try: + from keel import _build_info as embedded # type: ignore[attr-defined] + except ImportError: + return None + return embedded + + +def build_info() -> BuildInfo: + """Resolve the running build. Never raises.""" + embedded = _embedded() + + if embedded is not None: + stamped_commit = getattr(embedded, "COMMIT", "unknown") + dirty = bool(getattr(embedded, "DIRTY", False)) + # ⚠️ A STALE stamp in a working checkout would otherwise claim `[release]` and hide a + # dirty tree -- which is precisely the misreport this module exists to prevent. If git + # is present and disagrees with the stamp, believe git. + head = _git("rev-parse", "--short=12", "HEAD") + if head is not None: + if head != stamped_commit or _git("status", "--porcelain"): + dirty = True + return BuildInfo( + version=getattr(embedded, "VERSION", _package_version()), + commit=stamped_commit, + dirty=dirty, + source="release", + ) + + commit = _git("rev-parse", "--short=12", "HEAD") + if commit is None: + return BuildInfo( + version=_package_version(), commit="unknown", dirty=False, source="unknown" + ) + + status = _git("status", "--porcelain") + return BuildInfo( + version=_package_version(), + commit=commit, + # `status` is None only if the second git call failed after the first succeeded -- + # treat that as dirty, because "we could not tell" must not read as "clean". + dirty=status is None or bool(status), + source="checkout", + ) diff --git a/pyproject.toml b/pyproject.toml index 707c862b..9b113ce0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [project] -name = "keel" +name = "keel-trader" version = "0.1.0" description = "Offline-first, broker-agnostic, rule-based spot-trading agent (halal policy by default)" readme = "README.md" @@ -30,6 +30,10 @@ build-backend = "uv_build" [tool.uv.build-backend] module-root = "" +# The DISTRIBUTION is `keel-trader` (the name `keel` is taken on PyPI by an unrelated project), +# but the import package and the CLI command stay `keel`. Without this the backend would infer +# `keel_trader/` from the distribution name. +module-name = "keel" [tool.uv.workspace] members = ["packages/*"] diff --git a/tests/test_version.py b/tests/test_version.py new file mode 100644 index 00000000..5f396c95 --- /dev/null +++ b/tests/test_version.py @@ -0,0 +1,151 @@ +"""Build identity (`keel --version`).""" + +from __future__ import annotations + +import subprocess + +import pytest +from click.testing import CliRunner + +from keel import version as version_mod +from keel.cli import cli +from keel.version import BuildInfo, build_info + + +def test_a_dirty_checkout_is_not_reproducible(): + info = BuildInfo(version="0.1.0", commit="abc", dirty=True, source="checkout") + assert info.is_reproducible is False + assert "DIRTY" in info.describe() + + +def test_a_clean_checkout_is_reproducible(): + info = BuildInfo(version="0.1.0", commit="abc", dirty=False, source="checkout") + assert info.is_reproducible is True + assert "DIRTY" not in info.describe() + + +def test_a_clean_release_build_is_reproducible_even_without_git(): + info = BuildInfo(version="1.2.3", commit="abc", dirty=False, source="release") + assert info.is_reproducible is True + assert "release" in info.describe() + + +def test_a_release_build_reporting_DIRTY_is_NOT_reproducible(): + """A stale stamp in a modified checkout must not pass as a release.""" + info = BuildInfo(version="1.2.3", commit="abc", dirty=True, source="release") + assert info.is_reproducible is False + + +def test_an_unknown_build_is_NOT_treated_as_reproducible(): + """Failing to identify the build must never read as 'fine'.""" + info = BuildInfo(version="unknown", commit="unknown", dirty=False, source="unknown") + assert info.is_reproducible is False + + +def test_build_info_never_raises_when_git_is_unavailable(monkeypatch): + monkeypatch.setattr(version_mod, "_embedded", lambda: None) + monkeypatch.setattr(version_mod, "_git", lambda *a: None) + info = build_info() + assert info.source == "unknown" + + +def test_a_failed_status_call_reads_as_DIRTY_not_clean(monkeypatch): + """"We could not tell" must not be reported as a clean tree.""" + calls = {"n": 0} + + def fake_git(*args): + calls["n"] += 1 + return "abc123def456" if args[0] == "rev-parse" else None + + monkeypatch.setattr(version_mod, "_embedded", lambda: None) + monkeypatch.setattr(version_mod, "_git", fake_git) + info = build_info() + assert info.source == "checkout" + assert info.dirty is True + + +def test_git_timeout_is_bounded(monkeypatch): + """A hung git must not hang the CLI.""" + seen = {} + + def fake_run(cmd, **kwargs): + seen.update(kwargs) + raise subprocess.TimeoutExpired(cmd, 1) + + monkeypatch.setattr(subprocess, "run", fake_run) + assert version_mod._git("rev-parse", "HEAD") is None + assert seen.get("timeout") == version_mod._GIT_TIMEOUT_SEC + + +# -- CLI ----------------------------------------------------------------------- + + +def test_version_flag_prints_and_exits_before_any_command(): + result = CliRunner().invoke(cli, ["--version"]) + assert result.exit_code == 0 + assert "keel" in result.output + + +def test_version_flag_warns_loudly_when_the_build_is_not_reproducible(monkeypatch): + monkeypatch.setattr( + "keel.cli.build_info", + lambda: BuildInfo(version="0.1.0", commit="abc", dirty=True, source="checkout"), + ) + result = CliRunner().invoke(cli, ["--version"]) + assert result.exit_code == 0 + assert "NOT reproducible" in result.output + assert "live funds" in result.output + + +def test_version_flag_is_silent_about_reproducibility_for_a_release_build(monkeypatch): + monkeypatch.setattr( + "keel.cli.build_info", + lambda: BuildInfo(version="1.0.0", commit="deadbeef", dirty=False, source="release"), + ) + result = CliRunner().invoke(cli, ["--version"]) + assert "NOT reproducible" not in result.output + assert "1.0.0" in result.output + + +@pytest.mark.parametrize("flag", ["--version"]) +def test_version_does_not_require_a_database(tmp_path, flag): + """`--version` must work before anything is configured -- it is a diagnostic.""" + result = CliRunner().invoke(cli, [flag]) + assert result.exit_code == 0 + + +def test_a_STALE_release_stamp_in_a_modified_checkout_is_reported_dirty(monkeypatch): + """The misreport this module exists to prevent. + + A leftover `_build_info.py` from a local build would otherwise make a dev checkout claim + `[release]` and hide a dirty tree. When git disagrees with the stamp, believe git. + """ + + class _Stamp: + VERSION = "9.9.9" + COMMIT = "aaaaaaaaaaaa" + DIRTY = False + + monkeypatch.setattr(version_mod, "_embedded", lambda: _Stamp) + monkeypatch.setattr( + version_mod, "_git", lambda *a: "bbbbbbbbbbbb" if a[0] == "rev-parse" else "" + ) + info = build_info() + assert info.source == "release" + assert info.dirty is True + assert info.is_reproducible is False + + +def test_a_matching_stamp_on_a_clean_tree_stays_reproducible(monkeypatch): + class _Stamp: + VERSION = "9.9.9" + COMMIT = "aaaaaaaaaaaa" + DIRTY = False + + monkeypatch.setattr(version_mod, "_embedded", lambda: _Stamp) + monkeypatch.setattr( + version_mod, "_git", lambda *a: "aaaaaaaaaaaa" if a[0] == "rev-parse" else "" + ) + info = build_info() + assert info.dirty is False + assert info.is_reproducible is True diff --git a/uv.lock b/uv.lock index c3b963e7..04e393d0 100644 --- a/uv.lock +++ b/uv.lock @@ -8,11 +8,11 @@ resolution-markers = [ [manifest] members = [ - "keel", "keel-broker-api", "keel-broker-coinbase", "keel-broker-fake", "keel-core", + "keel-trader", ] [[package]] @@ -325,43 +325,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/cb/b1/3846dd7f199d53cb17f49cba7e651e9ce294d8497c8c150530ed11865bb8/iniconfig-2.3.0-py3-none-any.whl", hash = "sha256:f631c04d2c48c52b84d0d0549c99ff3859c98df65b3101406327ecc7d53fbf12", size = 7484, upload-time = "2025-10-18T21:55:41.639Z" }, ] -[[package]] -name = "keel" -version = "0.1.0" -source = { editable = "." } -dependencies = [ - { name = "click" }, - { name = "cryptography" }, - { name = "keel-broker-api" }, - { name = "keel-broker-coinbase" }, - { name = "keel-core" }, -] - -[package.dev-dependencies] -dev = [ - { name = "keel-broker-fake" }, - { name = "mypy" }, - { name = "pytest" }, - { name = "ruff" }, -] - -[package.metadata] -requires-dist = [ - { name = "click", specifier = ">=8.4.2" }, - { name = "cryptography", specifier = ">=49.0.0" }, - { name = "keel-broker-api", editable = "packages/keel-broker-api" }, - { name = "keel-broker-coinbase", editable = "packages/keel-broker-coinbase" }, - { name = "keel-core", editable = "packages/keel-core" }, -] - -[package.metadata.requires-dev] -dev = [ - { name = "keel-broker-fake", editable = "packages/keel-broker-fake" }, - { name = "mypy", specifier = ">=1.18.0" }, - { name = "pytest", specifier = ">=9.1.1" }, - { name = "ruff", specifier = ">=0.15.21" }, -] - [[package]] name = "keel-broker-api" version = "0.1.0" @@ -429,6 +392,43 @@ requires-dist = [ { name = "pyyaml", specifier = ">=6.0.3" }, ] +[[package]] +name = "keel-trader" +version = "0.1.0" +source = { editable = "." } +dependencies = [ + { name = "click" }, + { name = "cryptography" }, + { name = "keel-broker-api" }, + { name = "keel-broker-coinbase" }, + { name = "keel-core" }, +] + +[package.dev-dependencies] +dev = [ + { name = "keel-broker-fake" }, + { name = "mypy" }, + { name = "pytest" }, + { name = "ruff" }, +] + +[package.metadata] +requires-dist = [ + { name = "click", specifier = ">=8.4.2" }, + { name = "cryptography", specifier = ">=49.0.0" }, + { name = "keel-broker-api", editable = "packages/keel-broker-api" }, + { name = "keel-broker-coinbase", editable = "packages/keel-broker-coinbase" }, + { name = "keel-core", editable = "packages/keel-core" }, +] + +[package.metadata.requires-dev] +dev = [ + { name = "keel-broker-fake", editable = "packages/keel-broker-fake" }, + { name = "mypy", specifier = ">=1.18.0" }, + { name = "pytest", specifier = ">=9.1.1" }, + { name = "ruff", specifier = ">=0.15.21" }, +] + [[package]] name = "librt" version = "0.13.0"