diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c4cb978..02394a3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -28,10 +28,22 @@ 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 }} @@ -39,9 +51,35 @@ jobs: 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 diff --git a/.gitignore b/.gitignore index 0abfc41..0dd8d5a 100644 --- a/.gitignore +++ b/.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 diff --git a/scripts/deploy_fly.py b/scripts/deploy_fly.py new file mode 100644 index 0000000..5faec7b --- /dev/null +++ b/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()) diff --git a/scripts/verify_release.py b/scripts/verify_release.py new file mode 100644 index 0000000..3bfc633 --- /dev/null +++ b/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()) diff --git a/tests/snapshot/test_stage1_shadow.py b/tests/snapshot/test_stage1_shadow.py index e1bdc10..432ed7a 100644 --- a/tests/snapshot/test_stage1_shadow.py +++ b/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() diff --git a/tests/test_deploy_fly.py b/tests/test_deploy_fly.py new file mode 100644 index 0000000..3e52403 --- /dev/null +++ b/tests/test_deploy_fly.py @@ -0,0 +1,153 @@ +"""Tests for the Fly deploy retry ladder. + +Loaded by path and driven through an injected runner, following +`tests/test_fly_observability_snapshot.py` -- the suite never shells out and +never needs a Fly token. + +The point of putting this ladder in a script rather than in workflow YAML is +that it can be tested at all. The `if: failure()` two-step form these tests +would not have caught is the one that reports success as failure. +""" + +from __future__ import annotations + +import importlib.util +import subprocess +import sys +from pathlib import Path +from typing import Any + +import pytest + +_MODULE_PATH = Path(__file__).resolve().parents[1] / "scripts" / "deploy_fly.py" + + +def _load_module() -> Any: + spec = importlib.util.spec_from_file_location("deploy_fly", _MODULE_PATH) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +df = _load_module() + +FAKE_TOKEN = "fm2_lJPECAAAAAAAtesttokenvalue0000000000000xyz" + + +class Runner: + """Records argv per call and returns queued exit codes (or raises).""" + + def __init__(self, outcomes: list[object]): + self.outcomes = list(outcomes) + self.calls: list[list[str]] = [] + + def __call__(self, argv, timeout=None, **kwargs): + self.calls.append(list(argv)) + outcome = self.outcomes.pop(0) + if isinstance(outcome, BaseException): + raise outcome + return subprocess.CompletedProcess(argv, outcome) + + +@pytest.fixture +def sleeps() -> list[float]: + return [] + + +@pytest.fixture +def sleep(sleeps): + return sleeps.append + + +def test_first_attempt_uses_depot_and_no_fallback_runs_on_success(sleep, sleeps): + runner = Runner([0]) + assert df.deploy("property-shared", "fly.toml", runner=runner, sleep=sleep) == 0 + assert len(runner.calls) == 1, "a successful deploy must not be repeated" + assert "--depot=false" not in runner.calls[0], "Depot is the default and faster path" + assert sleeps == [], "no backoff when nothing failed" + + +def test_depot_failure_falls_back_to_depot_false(sleep): + runner = Runner([1, 0]) + assert df.deploy("property-shared", "fly.toml", runner=runner, sleep=sleep) == 0 + assert "--depot=false" not in runner.calls[0] + assert "--depot=false" in runner.calls[1], "the flag that made manual recovery work" + + +def test_exit_status_is_zero_when_the_fallback_succeeds(sleep): + """The defect an `if: failure()` two-step form would have. + + A failed first step fails the job even when a later step succeeds, so that + shape needs continue-on-error and then reports a misleading green. + """ + runner = Runner([1, 0]) + assert df.deploy("propertydata", "fly.app.toml", runner=runner, sleep=sleep) == 0 + + +def test_exit_status_is_one_when_every_attempt_fails(sleep): + runner = Runner([1, 1, 1]) + assert df.deploy("property-shared", "fly.toml", runner=runner, sleep=sleep) == 1 + assert len(runner.calls) == 3 + + +def test_a_hung_attempt_is_bounded_and_the_fallback_still_runs(sleep): + """Both observed failures were timeouts. + + Without a per-attempt bound the first hang eats the job budget and the + fallback never runs, which makes the whole ladder decorative. + """ + runner = Runner([subprocess.TimeoutExpired(cmd="flyctl", timeout=900), 0]) + assert df.deploy("property-shared", "fly.toml", runner=runner, sleep=sleep) == 0 + assert len(runner.calls) == 2 + assert "--depot=false" in runner.calls[1] + + +def test_every_attempt_is_given_a_timeout(sleep): + seen: list[float | None] = [] + + def runner(argv, timeout=None, **kwargs): + seen.append(timeout) + return subprocess.CompletedProcess(argv, 1) + + df.deploy("property-shared", "fly.toml", runner=runner, sleep=sleep) + assert seen and all(t is not None and t > 0 for t in seen), ( + "an attempt with no timeout can hang forever" + ) + + +@pytest.mark.parametrize( + ("app", "config"), + [("property-shared", "fly.toml"), ("propertydata", "fly.app.toml")], +) +def test_app_and_config_are_always_explicit_in_argv(app, config, sleep): + """CI relied on cwd discovery of fly.toml; the command that ran was not the + command anyone had written down.""" + runner = Runner([0]) + df.deploy(app, config, runner=runner, sleep=sleep) + argv = runner.calls[0] + assert argv[:2] == ["flyctl", "deploy"] + assert argv[argv.index("--app") + 1] == app + assert argv[argv.index("--config") + 1] == config + assert "--remote-only" in argv and "--ha=false" in argv + + +def test_a_missing_flyctl_fails_immediately_without_retrying(sleep): + runner = Runner([FileNotFoundError("flyctl")]) + assert df.deploy("property-shared", "fly.toml", runner=runner, sleep=sleep) == 1 + assert len(runner.calls) == 1, "retrying a missing binary cannot help" + + +def test_it_backs_off_between_attempts_but_not_after_the_last(sleep, sleeps): + runner = Runner([1, 1, 1]) + df.deploy("property-shared", "fly.toml", runner=runner, sleep=sleep) + assert len(sleeps) == 2, "three attempts means two gaps" + + +def test_the_token_never_appears_in_output(capsys, sleep, monkeypatch): + monkeypatch.setenv("FLY_API_TOKEN", FAKE_TOKEN) + runner = Runner([1, 1, 1]) + df.deploy("property-shared", "fly.toml", runner=runner, sleep=sleep) + captured = capsys.readouterr() + assert FAKE_TOKEN not in captured.out + captured.err diff --git a/tests/test_verify_release.py b/tests/test_verify_release.py new file mode 100644 index 0000000..5e2f918 --- /dev/null +++ b/tests/test_verify_release.py @@ -0,0 +1,194 @@ +"""Tests for the post-deploy release reconciliation. + +This job exists because of one concrete incident: on v1.15.1 the two Fly deploys +are independent leaf jobs, one failed and one succeeded, and that combination is +a normal terminal state of the workflow graph. The two apps ran different +versions with the broken build still live on the app that needed the fix. + +`test_one_target_behind_fails_and_names_both_versions` replays that exactly. +""" + +from __future__ import annotations + +import importlib.util +import json +import sys +from pathlib import Path +from typing import Any + +import pytest + +_MODULE_PATH = Path(__file__).resolve().parents[1] / "scripts" / "verify_release.py" + + +def _load_module() -> Any: + spec = importlib.util.spec_from_file_location("verify_release", _MODULE_PATH) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +vr = _load_module() + +TARGETS = { + "property-shared": "https://property-shared.fly.dev", + "propertydata": "https://propertydata.fly.dev", +} + + +def card(version: str, name: str = "property-data") -> str: + return json.dumps({"serverInfo": {"name": name, "version": version}}) + + +class Fetcher: + """Serves a queue of per-round responses keyed by host substring.""" + + def __init__(self, rounds: list[dict[str, object]]): + self.rounds = list(rounds) + self.calls: list[str] = [] + self._served = 0 + + def __call__(self, url: str) -> str: + self.calls.append(url) + # One round is consumed per full sweep of the targets. + index = min(self._served // len(TARGETS), len(self.rounds) - 1) + self._served += 1 + for key, value in self.rounds[index].items(): + if key in url: + if isinstance(value, BaseException): + raise value + return str(value) + raise AssertionError(f"unexpected url {url}") + + +@pytest.fixture +def sleeps() -> list[float]: + return [] + + +@pytest.fixture +def sleep(sleeps): + return sleeps.append + + +# --- tag normalisation --- + + +@pytest.mark.parametrize( + ("tag", "expected"), + [("v1.18.2", "1.18.2"), ("1.18.2", "1.18.2"), ("V1.18.2", "1.18.2"), ("vv1.0.0", "v1.0.0")], +) +def test_v_prefix_is_stripped_exactly_once(tag, expected): + """`lstrip("v")` would turn the nonsense tag vv1.0.0 into a plausible 1.0.0.""" + assert vr.normalise_tag(tag) == expected + + +# --- the happy path, and the incident --- + + +def test_both_targets_on_the_released_version_passes(sleep): + fetch = Fetcher([{"property-shared": card("1.18.2"), "propertydata": card("1.18.2")}]) + assert vr.verify("1.18.2", TARGETS, fetch=fetch, sleep=sleep, require_consecutive=2) == 0 + + +def test_one_target_behind_fails_and_names_both_versions(capsys, sleep): + """The v1.15.1 incident, replayed.""" + fetch = Fetcher([{"property-shared": card("1.15.0"), "propertydata": card("1.15.1")}]) + assert vr.verify("1.15.1", TARGETS, attempts=2, fetch=fetch, sleep=sleep) == 1 + out = capsys.readouterr().out + assert "RELEASE DRIFT" in out + assert "1.15.0" in out and "1.15.1" in out, "the operator must see both sides" + + +def test_an_unreachable_target_is_a_failure_not_a_pass(capsys, sleep): + fetch = Fetcher([ + {"property-shared": ConnectionError("refused"), "propertydata": card("1.18.2")} + ]) + assert vr.verify("1.18.2", TARGETS, attempts=2, fetch=fetch, sleep=sleep) == 1 + assert "RELEASE DRIFT" in capsys.readouterr().out + + +def test_an_unreadable_card_is_a_failure_not_a_pass(sleep): + fetch = Fetcher([{"property-shared": "not json", "propertydata": card("1.18.2")}]) + assert vr.verify("1.18.2", TARGETS, attempts=2, fetch=fetch, sleep=sleep) == 1 + + +# --- propagation --- + + +def test_it_waits_for_a_late_target_rather_than_failing_on_the_first_round(sleep): + """A completed deploy does not mean the new version is instantly serving.""" + fetch = Fetcher([ + {"property-shared": card("1.18.1"), "propertydata": card("1.18.2")}, + {"property-shared": card("1.18.2"), "propertydata": card("1.18.2")}, + {"property-shared": card("1.18.2"), "propertydata": card("1.18.2")}, + ]) + assert vr.verify("1.18.2", TARGETS, attempts=5, fetch=fetch, sleep=sleep) == 0 + + +def test_a_version_that_flaps_between_rounds_does_not_pass(sleep): + """Defends against a multi-Machine app round-robining old and new. + + A single 200 cannot distinguish "propagated" from "you happened to hit the + updated Machine". + """ + fetch = Fetcher([ + {"property-shared": card("1.18.2"), "propertydata": card("1.18.2")}, + {"property-shared": card("1.18.1"), "propertydata": card("1.18.2")}, + {"property-shared": card("1.18.2"), "propertydata": card("1.18.2")}, + ]) + assert vr.verify("1.18.2", TARGETS, attempts=3, fetch=fetch, sleep=sleep, + require_consecutive=3) == 1 + + +def test_it_stops_polling_as_soon_as_the_targets_agree(sleeps, sleep): + fetch = Fetcher([{"property-shared": card("1.18.2"), "propertydata": card("1.18.2")}]) + vr.verify("1.18.2", TARGETS, attempts=20, fetch=fetch, sleep=sleep, require_consecutive=1) + assert sleeps == [], "no reason to keep polling once the answer is in" + + +# --- the two failures must stay distinguishable --- + + +def test_a_pyproject_tag_mismatch_fails_immediately_without_polling(tmp_path, capsys): + """No amount of polling can fix a version that was never built. + + Distinct from drift, and reported as such -- otherwise an operator chases a + partial deploy that did not happen. + """ + pyproject = tmp_path / "pyproject.toml" + pyproject.write_text('[project]\nname = "property-shared"\nversion = "1.18.2"\n') + + called: list[str] = [] + original = vr._fetch + vr._fetch = lambda url: called.append(url) or "" # type: ignore[assignment] + try: + code = vr.main([ + "--expect", "v1.18.3", + "--target", "property-shared=https://example.invalid", + "--pyproject", str(pyproject), + ]) + finally: + vr._fetch = original # type: ignore[assignment] + + assert code == 1 + assert called == [], "a mismatch must not spend twenty polling rounds" + out = capsys.readouterr().out + assert "tag/pyproject mismatch" in out + assert "RELEASE DRIFT" not in out, "two different failures must read differently" + + +def test_the_real_pyproject_version_is_readable(): + """Pins the field path this depends on.""" + root = Path(__file__).resolve().parents[1] + assert vr.project_version(root / "pyproject.toml").count(".") >= 2 + + +@pytest.mark.parametrize("raw", ["noequals", "=https://x", "name="]) +def test_a_malformed_target_is_rejected(raw): + import argparse + + with pytest.raises(argparse.ArgumentTypeError): + vr.parse_target(raw)