ci: retry a failed Fly deploy, and fail loudly when the two apps disagree

.github/workflows/release.yml

@@ -28,20 +28,58 @@ jobs:
     name: Deploy property-shared to Fly
     runs-on: ubuntu-latest
     needs: publish
+    timeout-minutes: 45
     steps:
       - uses: actions/checkout@v4
-      - uses: superfly/flyctl-actions/setup-flyctl@master
-      - run: flyctl deploy --remote-only --ha=false
+      # Pinned by SHA rather than @master: this job holds a production deploy
+      # token, and @master runs whatever is on that branch at that moment. The
+      # SHA below is tag 1.6 / v1, which is what @master already resolved to
+      # when this was pinned, so the pin changes nothing except reproducibility.
+      # The flyctl BINARY is deliberately unpinned -- it talks to a moving
+      # server API, so pinning it turns a rare flake into certain rot.
+      - uses: superfly/flyctl-actions/setup-flyctl@ed8efb33836e8b2096c7fd3ba1c8afe303ebbff1  # 1.6
+      # One step, not a `run:` plus an `if: failure()` retry step: a failed step
+      # fails the job even when a later one succeeds, so that shape needs
+      # continue-on-error and then reports a misleading green. The ladder lives
+      # in a script so it can be tested -- see tests/test_deploy_fly.py.
+      - name: Deploy (Depot, then --depot=false fallback)
+        run: python3 scripts/deploy_fly.py --app property-shared --config fly.toml
         env:
           FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
 
   deploy-propertydata:
     name: Deploy propertydata to Fly
     runs-on: ubuntu-latest
     needs: publish
+    timeout-minutes: 45
     steps:
       - uses: actions/checkout@v4
-      - uses: superfly/flyctl-actions/setup-flyctl@master
-      - run: flyctl deploy --config fly.app.toml --remote-only --ha=false
+      - uses: superfly/flyctl-actions/setup-flyctl@ed8efb33836e8b2096c7fd3ba1c8afe303ebbff1  # 1.6
+      - name: Deploy (Depot, then --depot=false fallback)
+        run: python3 scripts/deploy_fly.py --app propertydata --config fly.app.toml
         env:
           FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN_PROPERTYDATA }}
+
+  reconcile:
+    name: Both apps must serve the released version
+    runs-on: ubuntu-latest
+    needs: [deploy, deploy-propertydata]
+    # `!cancelled()`, not the default. On v1.15.1 one deploy failed and one
+    # succeeded -- a normal terminal state of this graph -- so the job that
+    # would have noticed must run precisely when a deploy has failed. Not
+    # `always()`, so a cancelled run is not reported as version drift.
+    #
+    # If `publish` fails both deploys skip and this still fails, because neither
+    # app serves the tag. That is correct: a release that shipped nothing must
+    # not show green.
+    if: ${{ !cancelled() }}
+    timeout-minutes: 15
+    steps:
+      - uses: actions/checkout@v4
+      - name: Reconcile deployed versions against the release tag
+        run: |
+          python3 scripts/verify_release.py \
+            --expect "${{ github.event.release.tag_name }}" \
+            --target property-shared=https://property-shared.fly.dev \
+            --target propertydata=https://propertydata.fly.dev \
+            --attempts 20 --interval 15 --require-consecutive 2

.gitignore

@@ -57,6 +57,8 @@ scripts/*
 !scripts/rightmove_address_dump.py
 !scripts/fly_observability_snapshot.py
 !scripts/validate.sh
+!scripts/deploy_fly.py
+!scripts/verify_release.py
 
 # Live git worktrees, not disposable scratch. Untracked and unignored, this
 # directory presented as `?? .claude/worktrees/` in every status, one

scripts/deploy_fly.py

@@ -0,0 +1,127 @@
+#!/usr/bin/env python3
+"""Deploy one Fly app, retrying past a transient builder failure.
+
+On the v1.15.1 release, `publish` and the `propertydata` deploy both succeeded
+while the `property-shared` deploy failed twice, five minutes apart:
+
+    Waiting for depot builder...
+    error releasing builder: deadline_exceeded: context deadline exceeded
+    Error: failed to fetch an image or build from source: error building:
+      timed out connecting to machine: failed to list workers:
+      Unavailable: ... authentication handshake failed: EOF
+
+Fly reported no incident, the same token had built the app five hours earlier,
+and `propertydata` obtained a builder on the same run. It was treated as
+transient Depot unavailability. Recovery was a manual `fly deploy` with
+`--depot=false`, which is the one causally relevant difference from CI.
+
+So: attempt with Depot (the default, and the faster path -- one intermittent
+incident is not a reason to discard it permanently), then fall back to
+`--depot=false`.
+
+**The per-attempt timeout is load-bearing.** Both observed failures were
+timeouts. Without a bound, one hung attempt consumes the whole job budget and
+the fallback never runs, which would make this retry decorative.
+
+Why a script and not YAML: retry logic in a workflow cannot be tested, and the
+`if: failure()` two-step form needs `continue-on-error` (so a failed first
+attempt renders as a misleading green) and writes the flyctl arguments twice --
+and two copies drifting apart is the failure class this change exists to close.
+
+Duplicate-release hazard, stated rather than assumed: if an attempt released the
+app and then timed out waiting, the next attempt releases again. Both apps are
+stateless, run `--ha=false`, and have no migration step, so a duplicate release
+is benign here. That is a property of these two apps, not of `fly deploy`.
+
+Stdlib only: the deploy job installs flyctl, not the project.
+"""
+
+from __future__ import annotations
+
+import argparse
+import subprocess
+import sys
+import time
+from typing import Callable, Sequence
+
+#: Depot first, then two non-Depot attempts.
+ATTEMPTS: tuple[tuple[str, ...], ...] = ((), ("--depot=false",), ("--depot=false",))
+
+ATTEMPT_TIMEOUT_SECONDS = 900.0
+SLEEP_BETWEEN_ATTEMPTS = 30.0
+
+
+def build_argv(app: str, config: str, extra: Sequence[str] = ()) -> list[str]:
+    """The deploy command for one attempt.
+
+    `--app` and `--config` are always explicit. CI previously relied on cwd
+    discovery of `fly.toml` for property-shared, so the command that ran was not
+    the command anyone had written down; the successful manual recovery named
+    both.
+    """
+    return [
+        "flyctl", "deploy",
+        "--app", app,
+        "--config", config,
+        "--remote-only",
+        "--ha=false",
+        *extra,
+    ]
+
+
+def deploy(
+    app: str,
+    config: str,
+    *,
+    runner: Callable[..., subprocess.CompletedProcess] = subprocess.run,
+    sleep: Callable[[float], None] = time.sleep,
+    attempts: Sequence[Sequence[str]] = ATTEMPTS,
+    timeout: float = ATTEMPT_TIMEOUT_SECONDS,
+) -> int:
+    """Run the attempt ladder. Returns a process exit code."""
+    failures: list[str] = []
+
+    for index, extra in enumerate(attempts, start=1):
+        argv = build_argv(app, config, extra)
+        label = "depot" if not extra else "depot=false"
+        print(f"::group::deploy attempt {index}/{len(attempts)} ({label})", flush=True)
+        print(f"$ {' '.join(argv)}", flush=True)
+        try:
+            completed = runner(argv, timeout=timeout)
+            code = completed.returncode
+        except subprocess.TimeoutExpired:
+            code = None
+            failures.append(f"attempt {index} ({label}): timed out after {timeout:.0f}s")
+            print(f"attempt {index} timed out after {timeout:.0f}s", flush=True)
+        except FileNotFoundError as exc:
+            print("::endgroup::", flush=True)
+            print(f"::error::flyctl is not installed: {exc}", flush=True)
+            return 1
+        else:
+            if code == 0:
+                print("::endgroup::", flush=True)
+                print(f"deployed {app} on attempt {index} ({label})", flush=True)
+                return 0
+            failures.append(f"attempt {index} ({label}): exit {code}")
+            print(f"attempt {index} failed with exit {code}", flush=True)
+        print("::endgroup::", flush=True)
+
+        if index < len(attempts):
+            sleep(SLEEP_BETWEEN_ATTEMPTS)
+
+    print(f"::error::every deploy attempt for {app} failed", flush=True)
+    for line in failures:
+        print(f"  {line}", flush=True)
+    return 1
+
+
+def main(argv: Sequence[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--app", required=True)
+    parser.add_argument("--config", required=True)
+    args = parser.parse_args(argv)
+    return deploy(args.app, args.config)
+
+
+if __name__ == "__main__":  # pragma: no cover
+    sys.exit(main())

scripts/verify_release.py

@@ -0,0 +1,173 @@
+#!/usr/bin/env python3
+"""Assert every app actually serves the released version.
+
+`release.yml` fans out to two independent leaf deploy jobs. One failing while
+the other succeeds is a *normal terminal state* of that graph, which is exactly
+what happened on v1.15.1: PyPI and `propertydata` succeeded, `property-shared`
+failed twice, and nothing noticed. The two apps ran different versions for
+roughly an hour, with the app that needed a critical hotfix still on the broken
+build. This script is the job that would have noticed.
+
+Both apps expose the same version field (verified live 2026-09-04):
+
+    property-shared  /.well-known/mcp/server-card.json -> serverInfo.version
+    propertydata     /.well-known/mcp/server-card.json -> serverInfo.version
+
+`serverInfo.name` differs (`property-data` vs `property-app`) but `version`
+comes from the same installed `property-shared` distribution in both, so the
+comparison is meaningful rather than coincidental. `propertydata`'s `/health`
+carries no version and cannot be used for this.
+
+Two distinct failures, deliberately reported differently:
+
+  * **tag/pyproject mismatch** -- the version is baked into the image at build
+    time, so tagging v1.18.3 without bumping pyproject.toml deploys two apps
+    that both honestly report 1.18.2. No amount of polling fixes that, so it is
+    caught up front and reported as its own thing rather than as drift.
+  * **release drift** -- an app is not serving the released version.
+
+Polling, not a single request: a completed deploy does not mean the new version
+is instantly serving. And `--require-consecutive` rounds, because Fly's proxy
+may front more than one Machine; a single 200 cannot distinguish "propagated"
+from "you happened to hit the updated Machine". At the time of writing each app
+runs exactly one Machine, which is a point-in-time observation and not a
+guarantee, so the defence stays.
+
+Stdlib only, so the reconcile job needs no project install.
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import sys
+import time
+import tomllib
+import urllib.error
+import urllib.request
+from pathlib import Path
+from typing import Callable, Sequence
+
+CARD_PATH = "/.well-known/mcp/server-card.json"
+REQUEST_TIMEOUT_SECONDS = 10.0
+
+
+def normalise_tag(tag: str) -> str:
+    """Strip a single leading v.
+
+    Deliberately not `lstrip("v")`, which strips every leading v and turns the
+    nonsense tag `vv1.0.0` into a plausible `1.0.0`.
+    """
+    tag = tag.strip()
+    return tag[1:] if tag[:1] in ("v", "V") else tag
+
+
+def project_version(pyproject: Path) -> str:
+    with pyproject.open("rb") as handle:
+        return tomllib.load(handle)["project"]["version"]
+
+
+def _fetch(url: str) -> str:
+    with urllib.request.urlopen(url, timeout=REQUEST_TIMEOUT_SECONDS) as response:
+        return response.read().decode("utf-8")
+
+
+def observed_version(base_url: str, fetch: Callable[[str], str]) -> tuple[str | None, str | None]:
+    """Return (version, error). Exactly one is None."""
+    try:
+        body = fetch(f"{base_url.rstrip('/')}{CARD_PATH}")
+    except (urllib.error.URLError, OSError, TimeoutError) as exc:
+        return None, f"{type(exc).__name__}: {exc}"
+    try:
+        return json.loads(body)["serverInfo"]["version"], None
+    except (ValueError, KeyError, TypeError) as exc:
+        return None, f"unreadable server card: {type(exc).__name__}: {exc}"
+
+
+def verify(
+    expected: str,
+    targets: dict[str, str],
+    *,
+    attempts: int = 20,
+    interval: float = 15.0,
+    require_consecutive: int = 2,
+    fetch: Callable[[str], str] = _fetch,
+    sleep: Callable[[float], None] = time.sleep,
+) -> int:
+    consecutive = 0
+    last: dict[str, str] = {}
+
+    for attempt in range(1, attempts + 1):
+        round_state: dict[str, str] = {}
+        for name, base_url in targets.items():
+            version, error = observed_version(base_url, fetch)
+            # An unreachable target is a failure, not an unknown. Treating it as
+            # "not yet" would let a permanently dead app pass by timing out.
+            round_state[name] = version if version is not None else f"<{error}>"
+        last = round_state
+
+        agreed = all(value == expected for value in round_state.values())
+        consecutive = consecutive + 1 if agreed else 0
+        summary = "  ".join(f"{name}={value}" for name, value in sorted(round_state.items()))
+        print(
+            f"attempt {attempt}/{attempts}: {summary}"
+            f"  (agreed {consecutive}/{require_consecutive})",
+            flush=True,
+        )
+
+        if consecutive >= require_consecutive:
+            print(f"all targets serve {expected}", flush=True)
+            return 0
+        if attempt < attempts:
+            sleep(interval)
+
+    print(f"::error::RELEASE DRIFT: expected {expected} — " + "  ".join(
+        f"{name}={value}" for name, value in sorted(last.items())
+    ), flush=True)
+    for name, value in sorted(last.items()):
+        print(f"  {name:20} {value}", flush=True)
+    return 1
+
+
+def parse_target(raw: str) -> tuple[str, str]:
+    name, _, url = raw.partition("=")
+    if not name or not url:
+        raise argparse.ArgumentTypeError(f"expected name=url, got {raw!r}")
+    return name, url
+
+
+def main(argv: Sequence[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--expect", required=True, help="release tag, e.g. v1.18.2")
+    parser.add_argument("--target", action="append", required=True, type=parse_target)
+    parser.add_argument("--attempts", type=int, default=20)
+    parser.add_argument("--interval", type=float, default=15.0)
+    parser.add_argument("--require-consecutive", type=int, default=2)
+    parser.add_argument("--pyproject", default=str(Path(__file__).resolve().parents[1] / "pyproject.toml"))
+    args = parser.parse_args(argv)
+
+    expected = normalise_tag(args.expect)
+
+    declared = project_version(Path(args.pyproject))
+    if declared != expected:
+        print(
+            f"::error::tag/pyproject mismatch: tag {args.expect!r} normalises to "
+            f"{expected!r} but pyproject.toml declares {declared!r}. The version is "
+            f"baked into the image at build time, so both apps would report "
+            f"{declared!r} however many times they are polled. This is a release "
+            f"preparation error, not a partial deploy.",
+            flush=True,
+        )
+        return 1
+
+    return verify(
+        expected,
+        dict(args.target),
+        attempts=args.attempts,
+        interval=args.interval,
+        require_consecutive=args.require_consecutive,
+    )
+
+
+if __name__ == "__main__":  # pragma: no cover
+    sys.exit(main())

tests/snapshot/test_stage1_shadow.py

@@ -2010,10 +2010,21 @@ def test_the_release_workflow_still_deploys_both_apps():
     would become misleading in the other direction.
     """
     workflow = (REPO / ".github" / "workflows" / "release.yml").read_text()
-    assert "flyctl deploy --remote-only --ha=false" in workflow
-    assert "--config fly.app.toml" in workflow
     assert "release:" in workflow and "published" in workflow
 
+    # Both apps are named explicitly in the workflow. This previously asserted
+    # the literal `flyctl deploy --remote-only --ha=false`, which lived inline;
+    # the retry ladder moved that invocation into scripts/deploy_fly.py, so the
+    # flags are pinned there instead (below) and the app/config pairing is
+    # pinned here. The fact being guarded is unchanged: every release still
+    # deploys propertydata as well as property-shared.
+    assert "--app property-shared --config fly.toml" in workflow
+    assert "--app propertydata --config fly.app.toml" in workflow
+
+    deploy_script = (REPO / "scripts" / "deploy_fly.py").read_text()
+    assert '"--remote-only"' in deploy_script
+    assert '"--ha=false"' in deploy_script
+
 
 def test_the_runbook_documents_the_search_level_containment_rule():
     body = RUNBOOK.read_text()