From 683b9807ff036ccd4aa604b1fd45a44a60595e37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Miralles=20Ranca=C3=B1o?= <68760931+FlagshipDev@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:42:07 +0200 Subject: [PATCH] Two checks that were claiming more than they could know MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found by auditing a live instance and a release that had just gone out, and both are the same mistake in different clothes: a check whose oracle was a story the tree told itself. ## The alarm the architecture guaranteed `hullwork status` on a healthy deployment printed, permanently: ! N item(s) are ready to attempt and HULLWORK_FORGE_CODE_TOKEN is not set anywhere the dispatcher can see it, so nothing will ever pick them up Every clause of it was false, and two independent defects produced it. It asked the wrong process. The flag came from the environment of whichever process ran `status`, which in the shipped deployment is the receiver — and DR-0005 requires the receiver not to hold that credential. So the alarm was produced by the architecture working correctly and could not distinguish the healthy state from the broken one: a dispatcher genuinely missing the token printed the identical line. `doctor` had solved this and reports the same fact as `unknown` with where to ask; this sentence asserted a consequence instead. It also counted a different question from the one the dispatcher answers — two conditions against `eligible`'s six, so it counted an item belonging to an inactive project and then named a credential as the reason nobody had picked it up. The fix is not a fifth condition on a second query: there is no second query now. `readiness_notes` asks `eligible`, which is the dispatcher's own question. With a live dispatcher the note now says the question cannot be answered from here, and where to ask it. With no dispatcher alive the degradation is kept exactly as it was — that is the true positive, it is the shape `hullwork work` itself runs in, and its exit code still has to be 1. ## The surface that could agree with the pins about the wrong release The documentation check compares `docs/published-surface.json` to the pins in the documents. It catches both ordinary mistakes, and it compares the two halves to each other: nothing compared either to the tag that was actually pushed, so the two of them agreeing on the *previous* release was green. That state is reachable by following the release procedure in the order it is written. The recorder, given no argument, reads the pin — which at that step still names the previous release — so it rewrites the file it already had. Forget the pins as well and every check passes while the registry serves something newer. The tree cannot distinguish that from a correct publication whose post-release steps are not done yet. Same bytes. What separates them is a fact outside the tree: whether an image exists for the version this tree claims to be, which is one anonymous request to the registry — public package, no credential. So: surface already records this version, fine. No image published for this version, fine, and that exempts the whole bump-to-release window by a fact rather than by a flag anybody has to clear. Image published and the surface records something else — fail, naming the published tag. This makes `main` red for roughly half an hour after each release, between the image being published and the post-release commit landing. That is accurate rather than unfortunate, and it is the alarm whose absence let the silent state exist. ## Verified Both fixes were checked by reintroducing every defect they cover — eleven in all, eleven caught. The second was also checked against the real registry by putting the tree into the silent state: surface `0.1.0a7`, pins `0.1.0a7`, version `0.1.0a8`, image `0.1.0a8` published. The existing documentation check passed all 23 of its tests. The new one failed, naming the published tag and the commands that fix it. Signed-off-by: Javier Miralles Rancaño <68760931+FlagshipDev@users.noreply.github.com> --- .forgejo/workflows/ci.yml | 16 +++ .github/workflows/ci.yml | 16 +++ docs/releasing.md | 27 +++- hullwork/cli.py | 4 + hullwork/work.py | 81 +++++++++-- ...t_the_alarm_the_architecture_guarantees.py | 131 ++++++++++++++++++ ..._the_surface_is_not_behind_the_registry.py | 119 ++++++++++++++++ tests/the_registry.py | 88 ++++++++++++ 8 files changed, 471 insertions(+), 11 deletions(-) create mode 100644 tests/test_the_alarm_the_architecture_guarantees.py create mode 100644 tests/test_the_surface_is_not_behind_the_registry.py create mode 100644 tests/the_registry.py diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 2d21737..acca729 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -68,7 +68,23 @@ jobs: echo "::warning::This gate does not cover the sandbox. Run 'pytest' on a host with" echo "::warning::Docker before trusting a green tick here — see CONTRIBUTING.md." fi + # **The registry check runs here and nowhere else** (item 192). `test_the_surface_is_not_behind + # _the_registry` asks ghcr.io whether an image exists for the version this tree claims to be, + # and skips without this variable so a developer with no network still gets a green suite. + # + # The question cannot be answered inside the tree: `version=0.1.0a8, surface=0.1.0a7` is a + # correct publication whose post-release steps are not done *yet*, and also post-release steps + # done *wrong*. Same bytes. Only the registry separates them, and the pull is anonymous — a + # public package, no credential, nothing to leak or rotate. + # + # It is expected to be **red on `main` for about half an hour** after each release, between the + # image being published and the post-release commit landing. That is accurate rather than + # unfortunate: the published image really is newer than the documented one, and an alarm that + # stays quiet during the bad state to avoid being inconvenient is the defect this repository + # keeps rediscovering. - run: uv run pytest -ra + env: + ASK_THE_REGISTRY: "1" # DR-0001: the DCO is our contributor agreement, so it gets checked, not just documented. # Deliberately our own five lines of shell: several published DCO actions call the GitHub API to diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d21737..acca729 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,7 +68,23 @@ jobs: echo "::warning::This gate does not cover the sandbox. Run 'pytest' on a host with" echo "::warning::Docker before trusting a green tick here — see CONTRIBUTING.md." fi + # **The registry check runs here and nowhere else** (item 192). `test_the_surface_is_not_behind + # _the_registry` asks ghcr.io whether an image exists for the version this tree claims to be, + # and skips without this variable so a developer with no network still gets a green suite. + # + # The question cannot be answered inside the tree: `version=0.1.0a8, surface=0.1.0a7` is a + # correct publication whose post-release steps are not done *yet*, and also post-release steps + # done *wrong*. Same bytes. Only the registry separates them, and the pull is anonymous — a + # public package, no credential, nothing to leak or rotate. + # + # It is expected to be **red on `main` for about half an hour** after each release, between the + # image being published and the post-release commit landing. That is accurate rather than + # unfortunate: the published image really is newer than the documented one, and an alarm that + # stays quiet during the bad state to avoid being inconvenient is the defect this repository + # keeps rediscovering. - run: uv run pytest -ra + env: + ASK_THE_REGISTRY: "1" # DR-0001: the DCO is our contributor agreement, so it gets checked, not just documented. # Deliberately our own five lines of shell: several published DCO actions call the GitHub API to diff --git a/docs/releasing.md b/docs/releasing.md index 4c749de..f432624 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -163,17 +163,38 @@ anybody pinning either, and the bug report arrives months later from somebody wh **Then, once the image is public**, and in this order, because each step needs the one before it: ```bash -./scripts/record-the-published-surface.py # asks the new image what it accepts +./scripts/record-the-published-surface.py 0.1.0aN # the tag you just released — pass it # move the pins: README.md, docker-compose.yml, docs/install.md, PRIVACY.md ``` +**Pass the tag.** With no argument the script reads the pin from `docker-compose.yml`, and at this +point in the order that pin is still the *previous* release — so the bare command asks the old image +and re-records the surface it already had. It says which tag it asked, and that line is the only +thing that distinguishes the no-op from the work. + That is what lets the documentation describe the new release — including anything held back for it, per [CONTRIBUTING](../CONTRIBUTING.md#documentation-describes-the-released-artefact-not-this-checkout). -Skipping either half is not quiet: re-record without moving the pins and the pins disagree with the -recording; move the pins without re-recording and they disagree the other way. Both fail + +Skipping either half is usually not quiet: re-record without moving the pins and the pins disagree +with the recording; move the pins without re-recording and they disagree the other way. Both fail `tests/test_the_documentation_describes_the_published_artefact.py`, which is the whole point of storing the recording rather than trusting the sequence. +> **One combination used to escape both**, found cutting `0.1.0a8`: run the recorder with no argument +> *and* forget the pins, and everything still names the previous release, consistently. The gate +> compared the two halves to each other, so agreeing on the wrong answer passed, and the release went +> undocumented while every check stayed green. +> +> `tests/test_the_surface_is_not_behind_the_registry.py` closes it by asking the one question the +> tree cannot answer about itself: **is an image published for the version this tree claims to be?** +> If it is, the surface has to have been recorded from it. If it is not, the surface is allowed to +> lag — which exempts the whole bump-to-release window by a fact rather than by a flag somebody has +> to remember to clear. +> +> **So `main` goes red for the half hour between the release and the post-release commit.** That is +> the intended behaviour and it is the to-do list: the published image really is newer than the +> documented one until the two steps above are done. + ## The private forge Development happens on a Forgejo instance, and this repository is a **derivation** of it: the publish diff --git a/hullwork/cli.py b/hullwork/cli.py index ba0351a..7ec926c 100644 --- a/hullwork/cli.py +++ b/hullwork/cli.py @@ -2059,6 +2059,10 @@ def _cmd_status( session, code_token_configured=settings.forge_code_token is not None, forge=_status_forge, + # The dispatcher skips an item whose enrichment has not come back, and only when there + # is a tracker for it to come back from. Counting without this reports work as ready + # that the dispatcher would deliberately leave alone. + tracker_configured=bool(settings.tracker_url and settings.tracker_token), ) # **Asked, not assumed** (item 129). `readiness._forge_state` is module state in whichever # process last spoke to the forge; this is a different process from the receiver, so diff --git a/hullwork/work.py b/hullwork/work.py index 384ddb9..c2cb4b4 100644 --- a/hullwork/work.py +++ b/hullwork/work.py @@ -49,6 +49,14 @@ #: A clone that has not finished by now is a repository too large for one attempt to be worth it. CLONE_TIMEOUT_SECONDS = 600 +#: Counting is not dispatching. `eligible` takes a limit because it hands work out one item at a +#: time; a count that stopped at the first would report `1 item(s) are ready` for ever. +_EVERY_READY_ITEM = 10_000 + +#: Where the credential question can actually be answered, in the deployment shape this ships. +#: Deliberately the same sentence `doctor` gives, because it is the same wrong place to be standing. +WHERE_TO_ASK_THE_DISPATCHER = "`docker compose exec dispatcher hullwork doctor`." + #: Attributed to the tool, not to a person. A plausible human identity on these commits would #: imply somebody reviewed them, which is the claim the whole product exists not to make. #: `.invalid` is reserved by RFC 6761 precisely so an address can be honest about not being one. @@ -793,8 +801,45 @@ def give_up_publishing(session: Session, attempt: Attempt, *, why: str) -> None: ) +def _a_dispatcher_is_alive(session: Session) -> bool: + """Whether some other process is holding the lease and renewing it. + + The same test `doctor.not_from_here` makes, and it has to fail the same way: asking the lease is + asking the database, and this runs where the database may have no tables. Cannot tell → not + alive → nothing is downgraded, because a false silence is worse than a false alarm. + """ + from hullwork import lease + + try: + state, _ = lease.state(session) + except Exception: # any failure here means "cannot tell", whatever its class + return False + return state == "alive" + + +def ready_for_the_dispatcher( + session: Session, *, tracker_configured: bool = False +) -> list[Eligible]: + """Everything the dispatcher would attempt, rather than everything that looks ready. + + **One question, one answer** (item 193). `readiness_notes` used to select on state and lane, + and `eligible` applies six conditions; the two were written apart and drifted by four of them. + On the live instance that made `status` count an item belonging to an **inactive** project, and + then name a missing credential as the reason nobody had picked it up. + + Adding the missing condition to the second query would have re-created the defect with a longer + fuse, so there is no second query. The limit exists only because `eligible` takes one — counting + is not dispatching, and a count that stops at the first item is not a count. + """ + return eligible(session, limit=_EVERY_READY_ITEM, tracker_configured=tracker_configured) + + def readiness_notes( - session: Session, *, code_token_configured: bool, forge: object | None = None + session: Session, + *, + code_token_configured: bool, + forge: object | None = None, + tracker_configured: bool = False, ) -> list[Note]: """What `hullwork status` should say about the dispatcher half. @@ -810,17 +855,37 @@ def readiness_notes( against what the models declare. """ notes: list[Note] = [] - ready = session.execute( - select(Item).where(Item.state == ItemState.READY, Item.lane != Lane.RED) - ).scalars().all() + ready = ready_for_the_dispatcher(session, tracker_configured=tracker_configured) in_progress = session.execute( select(Item).where(Item.state == ItemState.IN_PROGRESS) ).scalars().all() - if ready and not code_token_configured: + if ready and not code_token_configured and _a_dispatcher_is_alive(session): + # **Whose resource is it** (item 193). This process is the receiver, and DR-0005 requires + # the receiver not to hold the code token — so its own environment says nothing about the + # process that does. Measured live: this printed `nothing will ever pick them up` while the + # dispatcher's own doctor printed `ok code token → hullwork`. + # + # Not silence either. The question is real and the operator is the one who can go and ask + # it, so it is reported as unanswerable from here, with where to ask — which is what + # `doctor` has done since item 105 and this sentence never learnt. + notes.append( + Note( + f"{len(ready)} item(s) are ready to attempt. Whether the dispatcher can publish " + f"them cannot be answered from here: HULLWORK_FORGE_CODE_TOKEN is not set in this " + f"process, and a dispatcher is running, so this is a resource it holds rather than " + f"one this process does. Ask it: {WHERE_TO_ASK_THE_DISPATCHER}" + ) + ) + elif ready and not code_token_configured: # A degradation, not a note. Items only reach `ready` when a manifest names an agent, so # this state means somebody configured one and never gave the dispatcher a credential — # work that will sit there for ever while everything reports fine. + # + # **With no dispatcher alive nothing is downgraded**, which is `doctor.py`'s rule word + # for word: the absence of one is exactly when somebody needs to know what is missing. It + # is also the shape `hullwork work` itself runs in, where the credential is genuinely this + # process's business and the exit code has to stay 1. notes.append( Note( f"{len(ready)} item(s) are ready to attempt and HULLWORK_FORGE_CODE_TOKEN is not " @@ -866,9 +931,9 @@ def readiness_notes( # before the dispatcher does, which is the difference between a warning and a post-mortem. if forge is not None and ready: stranded = [ - item.forge_issue_ref - for item in ready - if item.forge_issue_ref and not _issue_resolves(forge, item) + candidate.item.forge_issue_ref + for candidate in ready + if candidate.item.forge_issue_ref and not _issue_resolves(forge, candidate.item) ] if stranded: notes.append( diff --git a/tests/test_the_alarm_the_architecture_guarantees.py b/tests/test_the_alarm_the_architecture_guarantees.py new file mode 100644 index 0000000..1e84341 --- /dev/null +++ b/tests/test_the_alarm_the_architecture_guarantees.py @@ -0,0 +1,131 @@ +"""What `status` may claim about a credential it cannot see. Item 193. + +Found on the live instance: a healthy deployment reporting a permanent degradation, in a sentence +where every clause was false. `hullwork status` runs in the receiver, DR-0005 requires the receiver +not to hold `HULLWORK_FORGE_CODE_TOKEN`, and the note read that absence as *nothing will ever pick +them up* — a claim about a **different process**, which had the credential all along. + +`doctor.py` had solved this and `work.py` never learnt it. Item 073, in `doctor`'s own words: *a +signal that is permanently on is not a signal, and the day the credential really expires that line +is already red.* + +Every test here was verified by reintroducing the defect it covers. +""" + +from __future__ import annotations + +from sqlalchemy.orm import Session +from test_work import _item, _project + +from hullwork import lease, work +from hullwork.models import ItemState + + +def _a_dispatcher_is_alive(session: Session) -> None: + """The state the receiver is in whenever the deployment is working.""" + lease.acquire(session, lease.new_holder()) + + +# --- the claim that could not be true ---------------------------------------------------------- + + +def test_a_live_dispatcher_means_the_missing_credential_is_not_this_process_business( + session: Session, +) -> None: + """The whole finding. The receiver never holds this credential **by design**, so reading its own + environment and announcing a consequence for another process is not a diagnosis. + + Measured on `atlas`, 2026-08-09: this note said nothing would ever pick the item up while + `docker exec hullwork-dispatcher-1 hullwork doctor` said `ok code token → hullwork`. + """ + _item(session, _project(session)) + _a_dispatcher_is_alive(session) + + notes = work.readiness_notes(session, code_token_configured=False) + + assert not any(n.degraded for n in notes), ( + "a healthy instance reported a degradation, which is how this was found" + ) + said = " ".join(n.text for n in notes) + assert "nothing will ever pick them up" not in said + + +def test_it_says_where_the_question_can_be_answered_rather_than_going_quiet( + session: Session, +) -> None: + """Not silence. Downgrading a false alarm into nothing loses a real question — *does the + dispatcher have it?* — and the operator is the one who can go and ask.""" + _item(session, _project(session)) + _a_dispatcher_is_alive(session) + + said = " ".join(n.text for n in work.readiness_notes(session, code_token_configured=False)) + + assert "dispatcher" in said + assert "cannot" in said or "not from here" in said + + +def test_with_no_dispatcher_alive_the_degradation_is_kept_exactly(session: Session) -> None: + """The true positive, and the reason the branch cannot simply be deleted. + + `doctor.py:956`: *with no dispatcher alive, nothing is downgraded — the absence of one is + exactly when somebody needs to know what is missing.* This is also the shape `hullwork work` + runs in, where the credential question is genuinely local and the answer must still exit 1. + """ + _item(session, _project(session)) + + notes = work.readiness_notes(session, code_token_configured=False) + + assert any("nothing will ever pick them up" in n.text for n in notes) + assert any(n.degraded for n in notes) + + +def test_a_stale_lease_is_not_a_live_dispatcher(session: Session) -> None: + """A dispatcher that died holding the lease must not silence the alarm for ever — that would + turn one permanently-on signal into one permanently-off, which is worse.""" + _item(session, _project(session)) + _a_dispatcher_is_alive(session) + lease.release(session, lease.holder_of(session) or "") + + notes = work.readiness_notes(session, code_token_configured=False) + + assert any(n.degraded for n in notes) + + +# --- the second defect in the same sentence ----------------------------------------------------- + + +def test_an_item_of_an_inactive_project_is_not_counted_as_ready(session: Session) -> None: + """The exact shape that produced the alarm: project `simplecheck`, `active = 0`, holding one + item in `ready` that had already opened a pull request on 2026-07-30. + + A count that includes work nobody intends to do turns every other sentence built on it into a + guess. + """ + _item(session, _project(session, active=False, slug="parked")) + + said = " ".join(n.text for n in work.readiness_notes(session, code_token_configured=True)) + + assert "ready for the dispatcher" not in said + + +def test_status_counts_exactly_what_the_dispatcher_would_attempt(session: Session) -> None: + """**Asserted by construction**, because two lists of conditions kept equal by hand is what + produced a four-condition drift in the first place. + + `eligible()` applies six conditions; this note applied two. Any future condition added to the + dispatcher has to reach this sentence without anybody remembering to come here. + + **Two eligible items, not one**, and that is deliberate. `eligible` hands work out one at a time + and defaults to `limit=1`; with a single eligible item this test passes against a count that + stops at the first, which would report `1 item(s) are ready` on an instance with fifty. + """ + live = _project(session, slug="live") + first = _item(session, live) + second = _item(session, live, fingerprint="fp2") + _item(session, _project(session, active=False, slug="parked"), fingerprint="fp3") + _item(session, _project(session, slug="done"), state=ItemState.DONE, fingerprint="fp4") + + counted = work.ready_for_the_dispatcher(session) + + assert [e.item.id for e in counted] == [first.id, second.id] + assert [e.item.id for e in counted] == [e.item.id for e in work.eligible(session, limit=1000)] diff --git a/tests/test_the_surface_is_not_behind_the_registry.py b/tests/test_the_surface_is_not_behind_the_registry.py new file mode 100644 index 0000000..fd919a3 --- /dev/null +++ b/tests/test_the_surface_is_not_behind_the_registry.py @@ -0,0 +1,119 @@ +"""Whether the documented release is the published one. Item 192. + +`test_the_documentation_describes_the_published_artefact` compares the recorded surface to the +pins and catches both of the ordinary mistakes. It compares the two halves **to each other**, and +nothing compares either to the tag that was actually pushed — so the two of them agreeing on the +*previous* release is green, and that state is reachable by following `docs/releasing.md` in the +order it is written, with no step skipped. + +Found cutting `0.1.0a8`: the recorder, given no argument, reads the tag pinned in +`docker-compose.yml` — which at that point in the order is still the previous release. The step is a +no-op, and if the pins are then forgotten too, everything agrees on `0.1.0a7` and the suite is green +while the registry serves `0.1.0a8`. + +**The tree cannot tell the two states apart**, which is the whole reason this file needs a fact from +outside it. `version=0.1.0a8, surface=0.1.0a7` is a correct publication whose post-release steps are +not done *yet*, and it is also post-release steps done *wrong*. Same bytes. The one thing that +separates them is whether an image exists for the version this tree claims to be. + +Every test here was verified by reintroducing the defect it covers. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest +from the_registry import behind_the_registry, published_tags + +ROOT = Path(__file__).resolve().parent.parent +SURFACE = json.loads((ROOT / "docs/published-surface.json").read_text(encoding="utf-8")) + + +def _version() -> str: + """What this tree claims to be, read from the package rather than from a document.""" + from hullwork import __version__ + + return __version__ + + +# --- the rule, decided without a network --------------------------------------------------------- + + +def test_a_published_image_for_this_version_requires_the_surface_to_record_it() -> None: + """The failure this file exists for, and the state that is green in every other test. + + Reported with the published tag in it, because *the surface and the pins disagree* sends a + reader to reconcile two things that are already equal. + """ + said = behind_the_registry("0.1.0a8", surface="0.1.0a7", published=("0.1.0a7", "0.1.0a8")) + + assert said is not None + assert "0.1.0a8" in said and "0.1.0a7" in said + + +def test_no_published_image_for_this_version_means_the_surface_may_lag() -> None: + """**The bump-to-release window, exempted by a fact rather than by a flag.** + + The version is raised before the release exists — the publication pull request carries the bump, + and the image is built from the tag that merge produces. For all of that time the surface + honestly records the previous release, because that is the artefact that exists. + """ + assert behind_the_registry("0.1.0a8", surface="0.1.0a7", published=("0.1.0a7",)) is None + + +def test_a_surface_recorded_from_this_version_is_the_finished_state() -> None: + said = behind_the_registry("0.1.0a8", surface="0.1.0a8", published=("0.1.0a7", "0.1.0a8")) + + assert said is None + + +def test_it_replays_the_sequence_that_produced_the_defect() -> None: + """`0.1.0a8`'s own release, state by state, which is what item 192's gate asks for. + + The third row is the one that is green today and must not be: the image is on the registry and + every document in the tree describes the release before it. + """ + steps = [ + ("before the bump", "0.1.0a7", "0.1.0a7", ("0.1.0a7",), False), + ("version bumped, nothing published", "0.1.0a8", "0.1.0a7", ("0.1.0a7",), False), + ("image public, post-release not done", "0.1.0a8", "0.1.0a7", ("0.1.0a7", "0.1.0a8"), True), + ("surface re-recorded", "0.1.0a8", "0.1.0a8", ("0.1.0a7", "0.1.0a8"), False), + ] + for name, version, surface, published, should_fail in steps: + said = behind_the_registry(version, surface=surface, published=published) + assert (said is not None) is should_fail, name + + +def test_a_registry_that_cannot_be_reached_is_not_a_pass() -> None: + """**`None` means checked and fine**, so an unreachable registry must not borrow that word. + + A check that reports success when it could not run is the failure this repository has now found + three times: the permanently-on signal (item 073), the two halves agreeing wrongly (this item), + and a mutation harness that called six caught defects uncaught (item 193). + """ + with pytest.raises(LookupError): + behind_the_registry("0.1.0a8", surface="0.1.0a7", published=None) + + +# --- the same rule, against the real registry ---------------------------------------------------- + + +@pytest.mark.skipif( + not os.environ.get("ASK_THE_REGISTRY"), + reason="asks ghcr.io; set ASK_THE_REGISTRY=1 to run it (CI does)", +) +def test_this_tree_is_not_describing_a_release_the_registry_has_moved_past() -> None: + """The one that runs for real, and the only one here that can fail because of a person. + + Anonymous: the token endpoint issues a pull scope for a public package with no account. If this + is red, either the surface was recorded from the wrong tag or the post-release commit has not + landed yet — `docs/releasing.md` has both, in order. + """ + said = behind_the_registry( + _version(), surface=SURFACE["version"], published=published_tags() + ) + + assert said is None, said diff --git a/tests/the_registry.py b/tests/the_registry.py new file mode 100644 index 0000000..c9f21e0 --- /dev/null +++ b/tests/the_registry.py @@ -0,0 +1,88 @@ +"""Whether an image exists for the version this tree claims to be. Item 192. + +Split from the test that uses it so the rule can be decided with no network at all: every state of a +release is a tuple of three strings, and `behind_the_registry` is a function of them. Only +`published_tags` touches the wire, and only the one test that asks for it calls it. + +**This lives in `tests/` on purpose.** `scripts/` is withheld from publication, so a check that +lived there would be absent from the public tree — which is the one that gets pulled, and therefore +the one whose documentation being a release behind actually costs somebody something. +""" + +from __future__ import annotations + +import json +import urllib.error +import urllib.request +from collections.abc import Iterable + +#: The published package. Anonymous pulls are allowed on it, which is what makes this check runnable +#: from any checkout and any CI job without a credential to leak or rotate. +IMAGE = "easybytehub/hullwork" + +_TOKEN = f"https://ghcr.io/token?scope=repository:{IMAGE}:pull&service=ghcr.io" +_TAGS = f"https://ghcr.io/v2/{IMAGE}/tags/list" + +TIMEOUT_SECONDS = 20 + + +def behind_the_registry( + version: str, *, surface: str, published: Iterable[str] | None +) -> str | None: + """The failure, or `None` when there is nothing to report. + + Three lines, and the middle one is the whole reason this is not a tree-local rule: + + * the surface already records this version → nothing to say; + * **no image is published for this version** → nothing to say either, and this exempts the + whole window between the bump and the release without a flag anybody has to clear; + * an image is published for this version and the surface records something else → say so, naming + the published tag. + + `published=None` is not "no images", it is "could not ask", and it raises. A check that reports + success when it could not run is worse than no check: `docs/releasing.md` would carry a promise + with nothing behind it, which is precisely the shape item 192 was opened about. + """ + if published is None: + msg = ( + "could not ask ghcr.io which versions are published, so whether this tree documents " + "the current release is unknown — not confirmed" + ) + raise LookupError(msg) + if surface == version: + return None + if version not in set(published): + return None + return ( + f"ghcr.io/{IMAGE}:{version} is published and docs/published-surface.json was recorded from " + f"{surface}, so every document here describes the release before this one. Run " + f"`./scripts/record-the-published-surface.py {version}` and move the pins — " + f"docs/releasing.md has both, in that order." + ) + + +def published_tags() -> tuple[str, ...] | None: + """Every tag on the package, or `None` when the registry could not be asked. + + `None` rather than an exception so a caller can tell *offline* from *nothing published*, which + are the two answers this must never blur. + """ + try: + token = _get(_TOKEN)["token"] + tags = _get(_TAGS, {"Authorization": f"Bearer {token}"})["tags"] + except (urllib.error.URLError, TimeoutError, KeyError, ValueError, OSError): + return None + if not isinstance(tags, list): + # A registry answering something else is a registry that was not asked what we think. + return None + return tuple(str(tag) for tag in tags) + + +def _get(url: str, headers: dict[str, str] | None = None) -> dict[str, object]: + request = urllib.request.Request(url, headers=headers or {}) # noqa: S310 - literal https + with urllib.request.urlopen(request, timeout=TIMEOUT_SECONDS) as response: # noqa: S310 + loaded = json.load(response) + if not isinstance(loaded, dict): + msg = f"{url} did not answer with an object" + raise ValueError(msg) + return loaded