fix(release): stop cutting a release when only the dev tools moved

.github/workflows/release.yml

@@ -64,6 +64,28 @@ jobs:
         with:
           fetch-depth: 0
 
+      # The change detector asks uv what the no-dev resolution was at the last
+      # tag. Without uv on PATH the script says "changed" and is fail-safe but
+      # useless, so install it before anything reads its answer.
+      #
+      # The SAME uv the images build with, read out of the Dockerfile rather
+      # than restated here: uv guarantees lockfile compatibility within a minor
+      # release, and a detector on a different one would be judging a
+      # resolution nobody ships. Warns rather than fails if it cannot read the
+      # version -- an unpinned uv still compares both refs with one binary, so
+      # the answer stays sound; tests/test_release_triggers.py is what keeps the
+      # Dockerfile readable. (CodeRabbit, PR #354.)
+      - name: Install uv
+        run: |
+          UV_VERSION=$(grep -oE 'astral-sh/uv:[0-9]+\.[0-9]+\.[0-9]+' Dockerfile | head -1 | cut -d: -f2)
+          if [ -z "$UV_VERSION" ]; then
+            echo "::warning::could not read the uv version from Dockerfile; installing the newest"
+            pip install uv --quiet
+          else
+            echo "Installing uv $UV_VERSION, the version Dockerfile builds with"
+            pip install "uv==$UV_VERSION" --quiet
+          fi
+
       - name: Detect what changed
         id: changes
         run: |
@@ -94,9 +116,33 @@ jobs:
           # lan_isolation, notify, ...) produced a GREEN release run that
           # published nothing at all: no tag, no GitHub Release, no images.
           # Skipped steps do not fail a run, so it looked like a success.
-          if echo "$CHANGED" | grep -qE '^(Dockerfile|entrypoint\.sh|pyproject\.toml|uv\.lock|app/)'; then
+          # uv.lock and pyproject.toml hold the DEV group too, and both images
+          # build with `uv sync --frozen --no-dev`. Matching on the filename
+          # meant a pytest or ruff bump -- the most frequent dependency PR there
+          # is -- cut a full release. v1.36.4 was exactly that: 78 entries in
+          # site-packages, not one of them different from v1.36.3, and three
+          # containers restarted on the fleet for a version label.
+          #
+          # So the manifests do not trigger by name. The script exports the
+          # no-dev resolution at both refs with the real resolver and compares,
+          # and every way of not knowing answers "changed".
+          RUNTIME_CHANGED=false
+          if echo "$CHANGED" | grep -qE '^(pyproject\.toml|uv\.lock)$'; then
+            if [ -n "$LAST_TAG" ]; then
+              RUNTIME_CHANGED=$(python3 scripts/runtime_deps_changed.py "$LAST_TAG" HEAD || echo true)
+            else
+              RUNTIME_CHANGED=true
+            fi
+          fi
+          echo "Runtime dependencies changed: $RUNTIME_CHANGED"
+
+          if echo "$CHANGED" | grep -qE '^(Dockerfile|entrypoint\.sh|app/)'; then
             BUILD_UI=true
           fi
+          if [ "$RUNTIME_CHANGED" = true ]; then
+            BUILD_UI=true
+            BUILD_WORKER=true
+          fi
           if echo "$CHANGED" | grep -qE '^services/'; then
             BUILD_UI=true
             BUILD_WORKER=true
@@ -111,7 +157,7 @@ jobs:
           WORKER_MODULES=$(grep -oE '^COPY[^#]*app/([a-z_]+)\.py' Dockerfile.worker \
             | grep -oE 'app/[a-z_]+\.py' | sort -u)
           echo "Worker modules (from Dockerfile.worker):"; echo "$WORKER_MODULES"
-          if echo "$CHANGED" | grep -qE '^(Dockerfile\.worker|entrypoint\.sh|pyproject\.toml|uv\.lock)'; then
+          if echo "$CHANGED" | grep -qE '^(Dockerfile\.worker|entrypoint\.sh)'; then
             BUILD_WORKER=true
           fi
           while IFS= read -r mod; do

scripts/runtime_deps_changed.py

@@ -0,0 +1,150 @@
+#!/usr/bin/env python3
+"""Did the dependencies that SHIP change between two refs?
+
+``uv.lock`` holds the dev group as well as the runtime one, and release.yml
+treated any change to it as a reason to build. So a bump of pytest or ruff --
+the most frequent kind of dependency PR there is -- cut a full release whose
+images were identical to the previous one. v1.36.4 was exactly that: 78 entries
+in site-packages, zero difference from v1.36.3, three containers restarted on
+the fleet for a version label.
+
+Both images build with ``uv sync --frozen --no-dev``, so the question that
+decides a release is not "did uv.lock change" but "did the no-dev resolution
+change". This answers that by exporting it at both refs with the real resolver
+and comparing.
+
+    python scripts/runtime_deps_changed.py v1.36.3 HEAD    # -> false
+    python scripts/runtime_deps_changed.py v1.36.2 v1.36.3 # -> true
+
+FAIL-SAFE, and this is the whole design. Every way of not knowing -- uv is
+missing, a ref does not exist, an export fails, the lock disagrees with
+pyproject -- prints ``true`` and explains itself on stderr. A release that
+should not have happened costs a pointless image. A release that silently did
+not happen ships nothing while the run reports success, which is the failure
+this repo has been bitten by before.
+"""
+
+from __future__ import annotations
+
+import argparse
+import pathlib
+import subprocess
+import sys
+import tempfile
+
+#: What both Dockerfiles copy before `uv sync`. Nothing else feeds the resolution.
+MANIFESTS = ("pyproject.toml", "uv.lock")
+
+
+def _warn(message: str) -> None:
+    print(f"runtime_deps_changed: {message}", file=sys.stderr)
+
+
+def _run(args: list[str], **kwargs) -> subprocess.CompletedProcess[str]:
+    """Never raise. A missing binary is an answer, not a crash.
+
+    ``subprocess.run`` raises FileNotFoundError when the executable is absent,
+    which is precisely the case this script has to survive: no uv on PATH must
+    mean "assume it changed", not a traceback that fails the release job.
+    """
+    try:
+        return subprocess.run(args, capture_output=True, text=True, timeout=180, check=False, **kwargs)
+    except (OSError, subprocess.SubprocessError) as exc:
+        return subprocess.CompletedProcess(args, returncode=127, stdout="", stderr=str(exc))
+
+
+def _materialise(repo: pathlib.Path, ref: str, into: pathlib.Path) -> bool:
+    """Write the manifests as they were at ``ref``. False if any is unreadable."""
+    for name in MANIFESTS:
+        result = _run(["git", "-C", str(repo), "show", f"{ref}:{name}"])
+        if result.returncode != 0:
+            _warn(f"cannot read {name} at {ref}: {result.stderr.strip()}")
+            return False
+        (into / name).write_text(result.stdout, encoding="utf-8")
+    return True
+
+
+def _runtime_requirements(directory: pathlib.Path) -> set[str] | None:
+    """Everything the no-dev resolution pins, or None when uv cannot say.
+
+    ``--frozen`` so uv reports a lock that disagrees with its pyproject instead
+    of quietly re-resolving it, which would need the network and would answer a
+    different question from the one the Dockerfile asks.
+
+    HASHES ARE INCLUDED. Exporting with ``--no-hashes`` and keeping only the
+    ``name==version`` lines compares less than the build consumes: a lock can
+    gain or change an artifact for a version that already exists -- a new wheel
+    for a platform, a re-resolved sdist -- and every pin still reads the same
+    while ``uv sync --frozen`` installs something different. That returns
+    "unchanged" for a change that ships, which is the one direction this script
+    must never get wrong. (CodeRabbit, PR #354.)
+
+    Comment lines go, and only those. uv writes the command it was run with into
+    the header, and that names the temp directory, so it differs on every call
+    by construction. The ``# via ...`` provenance notes are dropped with it;
+    they restate the graph the pins already describe.
+    """
+    result = _run(
+        [
+            "uv",
+            "export",
+            "--directory",
+            str(directory),
+            "--frozen",
+            "--no-dev",
+            "--format",
+            "requirements-txt",
+        ]
+    )
+    if result.returncode != 0:
+        _warn(f"uv export failed in {directory}: {result.stderr.strip()[:400]}")
+        return None
+    return {line.strip() for line in result.stdout.splitlines() if line.strip() and not line.strip().startswith("#")}
+
+
+def runtime_deps_changed(repo: pathlib.Path, base: str, head: str) -> bool:
+    with tempfile.TemporaryDirectory() as tmp:
+        root = pathlib.Path(tmp)
+        exported = []
+        for ref in (base, head):
+            into = root / ref.replace("/", "_")
+            into.mkdir(parents=True, exist_ok=True)
+            if not _materialise(repo, ref, into):
+                _warn("assuming the runtime dependencies changed")
+                return True
+            requirements = _runtime_requirements(into)
+            if requirements is None:
+                _warn("assuming the runtime dependencies changed")
+                return True
+            exported.append(requirements)
+
+    before, after = exported
+    if before == after:
+        _warn(f"{len(before)} runtime requirements, identical between {base} and {head}")
+        return False
+
+    for pin in sorted(after - before):
+        _warn(f"  + {pin}")
+    for pin in sorted(before - after):
+        _warn(f"  - {pin}")
+    return True
+
+
+def main(argv: list[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("base", help="the ref to compare from, usually the last release tag")
+    parser.add_argument("head", nargs="?", default="HEAD")
+    parser.add_argument("--repo", default=".", help="repository root (default: cwd)")
+    args = parser.parse_args(argv)
+
+    if _run(["uv", "--version"]).returncode != 0:
+        _warn("uv is not on PATH; assuming the runtime dependencies changed")
+        print("true")
+        return 0
+
+    print("true" if runtime_deps_changed(pathlib.Path(args.repo), args.base, args.head) else "false")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/test_compose_image_pins.py

@@ -147,10 +147,17 @@ def _runs_the_drift_test(command: str) -> bool:
         requiring tags there would be a rule about a problem that workflow
         cannot have. The first version of this guard flagged it, which is how
         the distinction got noticed.
+
+        Comments are stripped first. release.yml's change detector explains that
+        a pytest bump used to cut a release, and on the raw text that prose read
+        as a job that runs the suite — the guard matching someone's writing
+        rather than a command, which is the same way the skip-marker guard in
+        test_beads_batch_65.py once fooled itself.
         """
-        if "pytest" not in command:
+        commands = "\n".join(line for line in command.splitlines() if not line.strip().startswith("#"))
+        if "pytest" not in commands:
             return False
-        return " -m " not in command and " -k " not in command
+        return " -m " not in commands and " -k " not in commands
 
     def _workflows_running_pytest(self):
         import yaml
@@ -174,6 +181,11 @@ def test_a_marker_filtered_run_is_not_required_to_fetch_tags(self):
         assert self._runs_the_drift_test("uv run pytest")
         assert self._runs_the_drift_test("pytest tests/ -v --tb=short")
 
+    def test_prose_about_pytest_is_not_a_pytest_run(self):
+        """The control for the comment-stripping, which is the whole point."""
+        assert not self._runs_the_drift_test("# a pytest bump used to cut a release\npip install uv --quiet")
+        assert self._runs_the_drift_test("# install first\nuv run pytest tests/")
+
     def test_each_such_job_checks_out_with_tags(self):
         offenders = []
         for name, doc in self._workflows_running_pytest():