diff --git a/.gitea/workflows/ci-image.yml b/.gitea/workflows/ci-image.yml index 68f123684..841a54c81 100644 --- a/.gitea/workflows/ci-image.yml +++ b/.gitea/workflows/ci-image.yml @@ -60,8 +60,13 @@ on: # this file no longer republishes on its own, so pair it with a `docker/ci/**` edit. # # `paths:` here and `ci-image-pin`'s `expected` pathspec in pr-checks.yml MUST name the same - # sources, and nothing mechanically enforces that since the shared self-reference went — - # ersatztv#855. + # sources. Since the shared self-reference went, `scripts/tests/test_ci_image_paths_pin_agreement.py` + # is what holds them together: it derives BOTH lists from these two workflows and compares them for + # set equality (ersatztv#855). The two are written in different glob dialects, so it models exactly + # one pair of spellings — `/**` here against the pathspec `` — and REFUSES anything else + # rather than canonicalising a pattern space whose spellings the two consumers treat differently. + # Change this list and that guard goes red until the pathspec follows; write it any other way and + # it goes red asking for the new shape to be modelled. workflow_dispatch: push: branches: [main] diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml index 373d58f58..f1fbd671b 100644 --- a/.gitea/workflows/pr-checks.yml +++ b/.gitea/workflows/pr-checks.yml @@ -106,9 +106,14 @@ jobs: # runner placement than for build recipe. Make a recipe change alongside a `docker/ci/**` # edit — a comment bump suffices, and it is the ONLY remedy: pinning the workflow-only # commit is rejected here, because `expected` is the last `docker/ci` commit. - # Nothing MECHANICALLY couples this pathspec to `ci-image.yml`'s `on.push.paths`; before + # This pathspec and `ci-image.yml`'s `on.push.paths` MUST name the same sources; before # #744 the shared self-reference kept them in step. Divergence is silent and green in the - # dangerous direction — tracked in ersatztv#855. + # dangerous direction, so it is enforced rather than asserted: + # `scripts/tests/test_ci_image_paths_pin_agreement.py` derives BOTH lists from the two + # workflows and compares them for set equality (ersatztv#855). It takes this pathspec from + # the ASSIGNMENT below rather than from any `git log` in the job, and models only a plain + # `` against `/**` there — any other spelling is refused rather than compared. + # Change this pathspec and that guard goes red until `on.push.paths` follows. # See docs/ci-cd.md -> "Publishing from a branch is a dispatch, not a push". # # Compare RESOLVED FULL shas, never the abbreviations: git auto-scales abbreviation length diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 053de5bd0..13624f68c 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -2307,7 +2307,14 @@ through a rebase. > recovery — are #854, deferred on exactly these grounds. **A change that lives only in `ci-image.yml` publishes nothing.** That file is no longer one of its -own trigger paths, nor one of `ci-image-pin`'s `expected` paths. That pairing is a **decided +own trigger paths, nor one of `ci-image-pin`'s `expected` paths. Those two lists must otherwise name +the same sources, and `scripts/tests/test_ci_image_paths_pin_agreement.py` holds them to it — it +derives both from the workflow documents and compares them for set equality, so editing one alone +reddens `script-tests`. That job is not a required context, so the red does not block server-side; it +denies the merge-consent auto-grant through the combined status. **Read that guard before taking the +reversal described next**: it models a `/**`-against-`` pair only, so putting +`ci-image.yml` back into both lists is a file-against-file pair it refuses, and would need a file arm +added there first (ersatztv#855). That pairing is a **decided tradeoff, not a necessity** — keeping the file in `expected` still works, since the branch dispatch can publish the ci-image.yml commit itself and the pin then matches. What it costs is the reason it went: that route charges a full ~2 GB publish and a five-pin bump for *every* edit to the file, a diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index d5d033583..e1ad4f9aa 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -242,6 +242,7 @@ recorded as unexamined rather than as cleared. | `scripts/tests/test_check_doc_narrative.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_ci_dropped_step_guard.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_complete_annotation_dispositions.py` | the `script-tests` job, when the SPA guard's `DISPOSITIONS` table disagrees with `test_optional_request_members.py` about a schema both rule on | GUARD | MUTATION | `test_complete_annotation_dispositions.py::test_the_two_dispositions_AGREE` | +| `scripts/tests/test_ci_image_paths_pin_agreement.py` | the `script-tests` job, when `ci-image.yml`'s `on.push.paths` and `ci-image-pin`'s `git log` pathspec stop naming the same image sources — the live comparison is `test_the_publish_paths_and_the_pin_pathspec_name_the_same_sources`; the ref below is its mutation proof | GUARD | MUTATION | `test_ci_image_paths_pin_agreement.py::test_a_diverging_list_is_DETECTED` | | `scripts/tests/test_ci_image_pin_population.py` | the `script-tests` job, when a container job loses its pin | GUARD | MUTATION | `test_ci_image_pin_population.py::test_a_single_job_losing_its_pin_is_DETECTED` | | `scripts/tests/test_ci_release_path_scan_job.py` | the `script-tests` job, on a weakened release-path scan job | GUARD | NONE | — | | `scripts/tests/test_ci_status_context_uniqueness.py` | the `script-tests` job, when two CI jobs synthesize the same status-check context (which branch protection cannot tell apart) | GUARD | MUTATION | `test_ci_status_context_uniqueness.py::test_no_two_jobs_synthesize_the_SAME_status_context` | @@ -330,7 +331,7 @@ job it covers, because a proof reference that covers a fraction must not read as | `docker-build.yml::scan` | a merge and the release path, when a banned expression delimiter appears in the `run:` body of a delimiter-ban job (`DELIMITER_BAN_JOBS` = the required contexts **plus `build`**, which is wider than the marked set). It carries no `if:`, so it runs on PRs too and a red blocks through the combined status | GUARD | the ban assertion is `test_ci_dropped_step_guard.py::test_the_delimiter_banned_jobs_have_NO_expression_delimiter_in_any_run_body` — note that file is graded `PROOF` above for its `ci-step-ran.sh` role, and also carries this GUARD-shaped assertion; `test_ci_release_path_scan_job.py` (`GUARD`) protects the job that runs it, and `scripts/ci-prove-ban-detects.sh` proves the ban still detects | `scripts/ci-prove-ban-detects.sh`'s row above — which itself carries `NONE`, as do the two pytest files, so this row claims no proof at all | | `docker-build.yml::api-docs` | a PR whose committed OpenAPI spec, endpoint index or SPA client types are stale | GUARD | inline — `git diff --exit-code` after re-running `scripts/update-openapi.sh` | NONE | | `docker-build.yml::format` | a PR whose changed `.cs` files do not conform to `.editorconfig` | GUARD | inline — `dotnet format --verify-no-changes` | NONE | -| `pr-checks.yml::ci-image-pin` | a PR whose `ersatztv-ci` pin is not the 7-char sha of the last commit to touch `docker/ci` | GUARD | inline | `test_ci_image_pin_population.py::test_a_single_job_losing_its_pin_is_DETECTED` covers the POPULATION half only; the git-history half has no test | +| `pr-checks.yml::ci-image-pin` | a PR whose `ersatztv-ci` pin is not the 7-char sha of the last commit to touch `docker/ci` | GUARD | inline | `test_ci_image_pin_population.py::test_a_single_job_losing_its_pin_is_DETECTED` covers the POPULATION half; `test_ci_image_paths_pin_agreement.py::test_a_diverging_list_is_DETECTED` covers the PATHSPEC half, bounded: it compares the pathspec this job's `git log` assignment WRITES against `ci-image.yml`'s publish trigger, and does not establish that the staleness comparison consumes that variable. Neither test runs the git-history comparison itself (that `expected` really is the last `docker/ci` commit), which still has no test | | `pr-checks.yml::docs-reminder` | **nothing.** Both of its check steps — and the `Set up Python` between them — carry `continue-on-error: true`, so no finding it makes can fail the job. (`Checkout` does not, so the job can still go red on infrastructure — never on a finding.) | REPORT-ONLY | inline + `scripts/check-doc-narrative.py` | n/a — it cannot go red | | `pr-checks.yml::decisions-guard` | a PR whose decision records fail lifecycle validation, whose catalog is stale, or whose kickoff file drifted | GUARD | `scripts/decisions_validate.py`, `scripts/build_decisions_catalog.py`, `scripts/check-kickoff-guard.sh` | those scripts' rows above | | `pr-checks.yml::prove-fix` | a PR whose `Proves:` trailer names a test that passes without the fix | GUARD | inline + `scripts/prove-fix.sh` | that script's row above | @@ -379,7 +380,7 @@ clause-provable and the entry is regraded. ## What the numbers say -46 guards, 6 tooling scripts, 23 proof files. **26 guards carry a mutation proof; 5 are +47 guards, 6 tooling scripts, 23 proof files. **27 guards carry a mutation proof; 5 are behaviour-only; 15 have none.** These figures are asserted against the table by `test_the_summary_counts_match_the_table`, because a hand-maintained summary of a table is a second copy of it — the duplication family this change argues against, and one that has already drifted diff --git a/scripts/tests/mutation_manifest.py b/scripts/tests/mutation_manifest.py index 68f305534..84f2e19e4 100644 --- a/scripts/tests/mutation_manifest.py +++ b/scripts/tests/mutation_manifest.py @@ -161,6 +161,22 @@ MUTATIONS: tuple[Mutation, ...] = ( why="The UNPROVEN branch: a named test that passes WITHOUT the fix must be refused. Same clause " "the proof test names.", ), + Mutation( + guard="scripts/tests/test_ci_image_paths_pin_agreement.py", + target="scripts/tests/test_ci_image_paths_pin_agreement.py", + clause="for path in sorted(published - required):", + replacement="for path in sorted(set()):", + proof="test_ci_image_paths_pin_agreement.py::test_a_diverging_list_is_DETECTED", + granularity=CLAUSE, + expect="the agreement check accepted a workflow pair whose publish paths and pin pathspec", + why="The two set-difference clauses fail differently and neither implies the other, so each " + "is separately load-bearing: this one catches a path that PUBLISHES an image the pin never " + "tracks (silent and green), while `required - published` catches a pathspec entry that " + "never publishes (a red on a blocking job until an image for it is published). Disarming " + "this clause alone leaves " + "the `publish-path-added` mutant undetected while the other four cases stay green, which " + "is why the grade is CLAUSE rather than DETECTOR.", + ), Mutation( guard="scripts/tests/test_ci_image_pin_population.py", target="scripts/tests/test_ci_image_pin_population.py", diff --git a/scripts/tests/test_ci_image_paths_pin_agreement.py b/scripts/tests/test_ci_image_paths_pin_agreement.py new file mode 100644 index 000000000..c73cd7a5e --- /dev/null +++ b/scripts/tests/test_ci_image_paths_pin_agreement.py @@ -0,0 +1,594 @@ +"""`ci-image.yml`'s publish paths and `ci-image-pin`'s pathspec are held to the same set (ersatztv#855). + +`.gitea/workflows/ci-image.yml`'s `on.push.paths` decides which pushes to `main` publish a toolchain +image; the weekly `schedule` and a `workflow_dispatch` build the selected ref's HEAD whatever it +touched, so this list governs the push route only. `.gitea/workflows/pr-checks.yml`'s `ci-image-pin` +job passes a pathspec to `git log` to decide what the image pin is REQUIRED to name. Divergence is +silent and green in the dangerous direction: a path that publishes but is not in the pathspec produces +images the pin never tracks, while a path in the pathspec that does not publish reddens a blocking +job until an image for it is published (which a `workflow_dispatch` can do, at the cost that decided +ersatztv#744). `ci.toolchain-image-publish-is-a-dispatch` removed the shared self-reference +that used to keep the two in step. + +Both lists are DERIVED from the workflow documents and compared for SET EQUALITY in both directions +(`testing.guard-derives-population-from-source`). Neither LIST is restated as a literal: the live +values are read from the parsed documents, so no second copy of either list stands as authority here. +(The divergence fixtures below do name synthetic paths of their own — `docker/extra`, `docker/other`, +`docker/ci/sub`, `docker` — which is what a fixture is for, and which is why renaming the real source +directory to one of THOSE would need an edit here.) + +ACCEPTED: a publish entry spelled exactly `/**`, against a pathspec entry spelled exactly +``, where every `/`-segment matches `[A-Za-z0-9._-]+` and is neither `.` nor `..`. Every other +spelling RAISES rather than comparing, and this file does not assert what any refused spelling would +have selected. A canonicaliser mapping the two dialects onto a common string form is a +tested-and-rejected design here, not an untried one. + +MEASURED against Gitea 1.27.1's own compiler — `modules/actions/workflowpattern`, whose +`CompilePatterns` calls `modules/glob.CompileWorkflow` — and real git. That is in-tree and is NOT +`nektos/act`'s `PatternToRegex`; the two disagree on brace, `**/x` and character-class forms, so +anyone extending the accepted shape must read the Gitea one. +This is why the accepted pair is the one modelled: + + * `docker/ci/**` compiles to `^docker/ci/.*\\Z`, and the pathspec `docker/ci` selects that subtree. + * a bare `docker/ci` in `paths:` compiles to `^docker/ci\\Z` and selects NONE of the directory's + contents, while the pathspec `docker/ci` selects all of them. + * `/**` matches nothing, while the pathspec `` tracks the file. The two spell the same + string, so `test_every_named_directory_is_a_tracked_DIRECTORY` decides it from the git index. + * a leading `/` is literal to Gitea, while git refuses it outright (`fatal: Invalid path`). + +RESIDUAL, measured: Gitea compiles `/**` to a Go regex whose `.` excludes newline, so any descendant +whose path below `` CONTAINS A NEWLINE — in its own name or in that of any directory above it — +is selected by the git pathspec and not by the publish pattern. The accepted pair still selects +different sets for such a file, and no refusal here closes that. + +BOUNDARY: this compares the pathspec the pin job WRITES. It does not establish that the job's +staleness comparison CONSUMES that pathspec — a job could assign it, read it in a diagnostic, and +compare something else, and only `ci-image-pin` itself going red would show that. + +THE `<<` REFUSAL IS TEXTUAL: a `<<` token on a code line of the pin job raises, because a heredoc body +is data that this reader would otherwise treat as code. A herestring `<<<` is excluded, since +`pr-checks.yml` already uses one in a sibling job; an arithmetic shift is not excluded and raises. +""" + +from __future__ import annotations + +import copy +import re +import shlex +from pathlib import Path + +import pytest +import yaml + +from scripts.tests.tracked_files import tracked_under + +REPO_ROOT = Path(__file__).resolve().parents[2] + +IMAGE_WORKFLOW = REPO_ROOT / ".gitea" / "workflows" / "ci-image.yml" +CHECKS_WORKFLOW = REPO_ROOT / ".gitea" / "workflows" / "pr-checks.yml" + +# The job whose `git log` pathspec is the second of the two lists. +PIN_JOB = "ci-image-pin" + +# `paths:` patterns and git pathspecs share no glob dialect. Only the recursive-directory suffix is +# modelled, because only it is known to denote the same set on both sides. +RECURSIVE_SUFFIX = "/**" + + +class ExtractionError(AssertionError): + """A workflow no longer has the shape this guard reads its lists out of. + + Raised rather than returned: an extraction that quietly yields an empty list would make the set + comparison below trivially true, which is the vacuity this guard exists to avoid reporting. + """ + + +class Unnormalisable(AssertionError): + """A path shape `_directory` does not model, so agreement cannot be decided for it.""" + + +def _triggers(doc: dict) -> dict: + """`on:` — YAML 1.1 resolves the bare key to the boolean True, so accept either spelling.""" + for key in ("on", True): + section = doc.get(key) + if isinstance(section, dict): + return section + raise ExtractionError(f"{IMAGE_WORKFLOW.name}: no usable `on:` mapping (tried 'on' and True)") + + +def publish_paths(image_doc: dict) -> list[str]: + """The `on.push.paths` list — what causes ci-image.yml to publish a toolchain image.""" + push = _triggers(image_doc).get("push") + if not isinstance(push, dict): + raise ExtractionError(f"{IMAGE_WORKFLOW.name}: `on.push` is not a mapping") + if "paths-ignore" in push: + raise ExtractionError( + f"{IMAGE_WORKFLOW.name}: `on.push` declares `paths-ignore`, which SUBTRACTS from the set " + "`paths:` selects. This guard models no subtraction, so it would compare the unreduced " + "list and report an agreement that does not hold. Model it here deliberately or drop the " + "key." + ) + paths = push.get("paths") + if not isinstance(paths, list) and paths is not None: + raise ExtractionError(f"{IMAGE_WORKFLOW.name}: `on.push.paths` is not a list") + if not paths: + # An ABSENT key and an EMPTY list are the same thing to the matcher, and both are the + # dangerous direction. Gitea compiles the list and treats "no patterns" as "do not skip" + # (`workflowpattern.Skip` returns false on an empty sequence), so either spelling means + # EVERY push to a matched branch publishes — never "publishes nothing". Diagnosing them + # apart, or diagnosing the empty list as an absence of publishing, would name the safe + # failure for the dangerous one. + raise ExtractionError( + f"{IMAGE_WORKFLOW.name}: `on.push` has no effective `paths:` filter " + f"({paths!r}), so EVERY push to a matched branch publishes. No pathspec can agree with " + "that — either restore the filter or retire this guard deliberately." + ) + if not all(isinstance(p, str) for p in paths): + raise ExtractionError(f"{IMAGE_WORKFLOW.name}: `on.push.paths` is not a list of strings") + return list(paths) + + +def _pin_step_code(checks_doc: dict) -> list[str]: + """Every non-comment line of every `run:` body in the pin job. + + Comments are dropped before the search below: this job's `run:` body carries far more prose than + code, and a `git log` written inside a comment is not an invocation. + """ + jobs = checks_doc.get("jobs") + if not isinstance(jobs, dict) or PIN_JOB not in jobs: + raise ExtractionError(f"{CHECKS_WORKFLOW.name}: no `{PIN_JOB}` job") + steps = jobs[PIN_JOB].get("steps") + if not isinstance(steps, list): + raise ExtractionError(f"{CHECKS_WORKFLOW.name}: `{PIN_JOB}` has no `steps:` list") + lines: list[str] = [] + for step in steps: + if not isinstance(step, dict) or not isinstance(step.get("run"), str): + continue + for ln in step["run"].splitlines(): + if ln.lstrip().startswith("#"): + continue + # A heredoc body is DATA, not executable lines, so an assignment written inside one is + # inert, and this reader cannot tell body from code without tracking the delimiter. The + # test is textual and the message says so. A herestring is excluded because it opens no + # body and `pr-checks.yml` already uses one in a sibling job; an arithmetic shift is not + # excluded and is a disclosed false red. + if "<<" in ln.replace("<<<", ""): + raise ExtractionError( + f"{CHECKS_WORKFLOW.name}: `{PIN_JOB}` has a `<<` token on a code line " + f"({ln.strip()!r}). If it opens a heredoc, this reader would treat the body as " + "executable lines, so the shape is refused rather than parsed by guesswork." + ) + lines.append(ln) + return lines + + +# The pathspec lives in an assignment whose whole right-hand side is a command substitution: +# `expected="$(git log -1 --format=%H -- docker/ci)"`. Binding to the ASSIGNMENT rather than to "the +# only `git log` in the job" is what stops an UNBOUND one being adopted: a `git log` in an inline +# comment or a heredoc body, left behind after the real assignment was rewired to something else. +# It does NOT establish which variable the job acts on — a diagnostic that is itself an assignment +# and is read IS adopted — and the docstring's boundary section says so. +_ASSIGNED_SUBSTITUTION = re.compile(r"^\s*(?P[A-Za-z_][A-Za-z0-9_]*)=\"?\$\(\s*(?P[^()]*?)\s*\)\"?\s*$") +_GIT_LOG = re.compile(r"\bgit\s+log\b") + + +def pin_pathspec(checks_doc: dict) -> list[str]: + """The pathspec `ci-image-pin` passes to `git log` — what the pin is required to name.""" + code = _pin_step_code(checks_doc) + assignments = [ + (m.group("name"), m.group("cmd")) + for line in code + if (m := _ASSIGNED_SUBSTITUTION.match(line)) and _GIT_LOG.search(m.group("cmd")) + ] + loose = [line.strip() for line in code if _GIT_LOG.search(line) and not _ASSIGNED_SUBSTITUTION.match(line)] + if loose: + raise ExtractionError( + f"{CHECKS_WORKFLOW.name}: `{PIN_JOB}` runs `git log` outside an assignment: {loose!r}. " + "The pathspec this guard compares must be the one the job ACTS on, so a `git log` whose " + "result is not bound to a variable is refused rather than silently adopted." + ) + if len(assignments) != 1: + raise ExtractionError( + f"{CHECKS_WORKFLOW.name}: expected exactly one `git log` assignment in `{PIN_JOB}`, " + f"found {len(assignments)}: {assignments!r}. With none there is nothing to read the " + "pathspec from; with several, choosing by position would be a guess. Extend this " + "extractor and say which assignment carries the pathspec." + ) + name, invocation = assignments[0] + if not any(re.search(r"\$\{?" + re.escape(name) + r"\b", line) for line in code): + raise ExtractionError( + f"{CHECKS_WORKFLOW.name}: `{name}` is assigned from `git log` in `{PIN_JOB}` but never " + "read, so its pathspec is not what the job checks against. Comparing it would report " + "agreement about a value nothing consumes." + ) + tokens = shlex.split(invocation) + separators = tokens.count("--") + if separators == 0: + raise ExtractionError( + f"{CHECKS_WORKFLOW.name}: `{invocation}` has no `--` separator, so its path arguments " + "cannot be told from its revision arguments." + ) + if separators > 1: + # A bare `--` is not necessarily THE separator: it can be the argument of a preceding + # option (`--decorate-refs "--"`), and telling those apart needs git's option arity table. + # Refusing more than one is the answer that needs no such table. + raise ExtractionError( + f"{CHECKS_WORKFLOW.name}: `{invocation}` contains {separators} bare `--` tokens. Only " + "one can be the path separator and the others are option arguments, which this guard " + "cannot tell apart without git's option arity. Refused rather than resolved by position." + ) + paths = tokens[tokens.index("--") + 1 :] + if not paths: + raise ExtractionError( + f"{CHECKS_WORKFLOW.name}: `{invocation}` names no path after `--`, so it matches the " + "whole tree and the pin tracks every commit." + ) + return paths + + +PUBLISH_ORIGIN = "ci-image.yml on.push.paths" +PIN_ORIGIN = "pr-checks.yml ci-image-pin's git-log pathspec" + + +# ONE SHAPE IS MODELLED AND EVERYTHING ELSE IS REFUSED. An open-ended canonicaliser over these two +# dialects is a tested-and-rejected design: among the spellings it mapped onto the modelled stem were +# pairs the two consumers select differently — a bare directory, a leading `/`, and a path that is a +# FILE rather than a directory (all three measured; see the module docstring). The comparison +# therefore does not canonicalise at all. A publish entry must be `/**` and a pathspec entry +# must be ``, where `` is built only from segments that carry no special meaning in EITHER +# dialect. That is the single pair of spellings whose selected set is unambiguous on both sides; +# every other spelling raises and asks to be modelled deliberately. The refusal set is wide on +# purpose — a red asks a question, while a false GREEN is the silent divergence this guard exists +# to stop. +_SAFE_SEGMENT = re.compile(r"[A-Za-z0-9._-]+\Z") +_RESERVED_SEGMENTS = frozenset({".", ".."}) + + +def _directory(raw: str, origin: str) -> str: + segments = raw.split("/") + bad = [s for s in segments if not _SAFE_SEGMENT.match(s) or s in _RESERVED_SEGMENTS] + if bad: + raise Unnormalisable( + f"{origin}: {raw!r} has segment(s) {bad} outside the plain-path form this guard models. " + "A `paths:` pattern and a git pathspec agree on the set selected by a plain directory " + "path; outside it they either select differently (a leading `/` or `./`) or cannot be " + "compared as written (a wildcard, a brace, an empty segment). A bare `` on the " + "publish side is refused earlier, by `_publish_directory`. Model the new shape here " + "deliberately rather than letting it compare as a literal." + ) + return raw + + +def _publish_directory(raw: str) -> str: + """A `paths:` entry -> the directory it selects recursively. + + Gitea compiles a `paths:` entry as an ANCHORED match, so `docker/ci/**` selects everything under + `docker/ci` while a bare `docker/ci` selects the literal path and none of its contents. Only the + explicit recursive form is accepted, because it is the only one that denotes what the git + pathspec `docker/ci` denotes. + """ + if not raw.endswith(RECURSIVE_SUFFIX): + raise Unnormalisable( + f"{PUBLISH_ORIGIN}: {raw!r} is not `{RECURSIVE_SUFFIX}`, the only publish spelling " + "this guard models. That form is modelled because its selected set matches the git " + "pathspec ``; no claim is made here about what any other spelling selects. Model " + "the new shape deliberately rather than widening this check to admit it." + ) + return _directory(raw[: -len(RECURSIVE_SUFFIX)], PUBLISH_ORIGIN) + + +def _pin_directory(raw: str) -> str: + """A git pathspec entry -> the directory it selects recursively.""" + return _directory(raw, PIN_ORIGIN) + + +def agreement_faults(image_doc: dict, checks_doc: dict) -> list[str]: + """Set equality in BOTH directions between the two derived lists. + + The two clauses fail differently and neither implies the other, which is why they are separate + and why each is separately proven below. + """ + published = {_publish_directory(p) for p in publish_paths(image_doc)} + required = {_pin_directory(p) for p in pin_pathspec(checks_doc)} + faults = [] + for path in sorted(published - required): + faults.append( + f"{path!r} is named by ci-image.yml's on.push.paths and NOT by ci-image-pin's git-log " + "pathspec, so the two lists no longer name the same image sources" + ) + for path in sorted(required - published): + faults.append( + f"{path!r} is named by ci-image-pin's git-log pathspec and NOT by ci-image.yml's " + "on.push.paths, so the two lists no longer name the same image sources" + ) + return faults + + +_IMAGE_DOC = yaml.safe_load(IMAGE_WORKFLOW.read_text()) +_CHECKS_DOC = yaml.safe_load(CHECKS_WORKFLOW.read_text()) + + +def test_both_lists_extract_and_are_NOT_empty(): + """Anti-vacuity. Two empty sets compare equal, so a broken extractor would make the agreement + assertion below pass while establishing nothing — the characteristic failure of a completeness + check (`testing.guard-ships-with-mutation-proof`).""" + published = publish_paths(_IMAGE_DOC) + required = pin_pathspec(_CHECKS_DOC) + assert published, f"{PUBLISH_ORIGIN} extracted as empty; the agreement check would be vacuous" + assert required, f"{PIN_ORIGIN} extracted as empty; the agreement check would be vacuous" + + +def test_every_extracted_path_is_one_the_MODELLED_shape_admits(): + """A shape `_directory` does not model must be a red asking for an extension, never a silent + literal comparison between two different glob dialects.""" + {_publish_directory(p) for p in publish_paths(_IMAGE_DOC)} + {_pin_directory(p) for p in pin_pathspec(_CHECKS_DOC)} + + +def test_the_publish_paths_and_the_pin_pathspec_name_the_same_sources(): + """The live assertion — the invariant ersatztv#855 was opened to stop carrying in prose.""" + faults = agreement_faults(_IMAGE_DOC, _CHECKS_DOC) + assert not faults, ( + "ci-image.yml's publish paths and ci-image-pin's git-log pathspec name DIFFERENT image " + "sources:\n " + "\n ".join(faults) + ) + + +def _live_assignment() -> str: + """The pin job's `git log` assignment line, DERIVED from the document. + + A literal copy here would be a hand-maintained mirror of the very line this guard exists to stop + anyone maintaining by hand, and it strands every fixture below on a stale anchor the moment the + pathspec is edited — which is exactly the co-ordinated edit the workflow comments promise will + clear a red. + """ + for line in _pin_step_code(_CHECKS_DOC): + match = _ASSIGNED_SUBSTITUTION.match(line) + if match and _GIT_LOG.search(match.group("cmd")): + return line.strip() + raise ExtractionError(f"{CHECKS_WORKFLOW.name}: no `git log` assignment in `{PIN_JOB}` for fixtures to anchor on") + + +def _rewire_pin_job(replacement: str) -> dict: + """Replace the pin job's `git log` assignment, asserting the edit LANDED. + + A fixture whose anchor stops matching silently arranges nothing, and the test then reports on the + unmodified document — so the substitution count is checked rather than assumed. + """ + anchor = _live_assignment() + doc = copy.deepcopy(_CHECKS_DOC) + applied = 0 + for step in doc["jobs"][PIN_JOB]["steps"]: + if isinstance(step, dict) and isinstance(step.get("run"), str): + # CODE lines only. A comment quoting the assignment — documenting the very line this + # guard reads is a plausible thing to write — would otherwise be counted, and the + # `applied == 1` assertion below would redden every fixture while the invariant holds. + rebuilt = [] + for ln in step["run"].splitlines(keepends=True): + if not ln.lstrip().startswith("#") and anchor in ln: + applied += ln.count(anchor) + ln = ln.replace(anchor, replacement) + rebuilt.append(ln) + step["run"] = "".join(rebuilt) + assert applied == 1, ( + f"the pin-job fixture derived the anchor {anchor!r} and matched it {applied} times, so the " + "document it is about to test is not the one it arranged." + ) + return doc + + +@pytest.mark.parametrize("spelling", [None, []], ids=["absent", "empty-list"]) +def test_NO_EFFECTIVE_paths_filter_is_refused_in_the_DANGEROUS_direction(spelling): + """An absent `paths:` and an empty one are the same thing to the matcher, and both mean EVERY + push to a matched branch publishes. Gitea compiles the list and its `Skip` returns false on an + empty sequence, so `paths: []` filters nothing. Reporting it as "publishes nothing" would name + the safe failure for the dangerous one, and would invite dropping the pathspec to match.""" + doc = copy.deepcopy(_IMAGE_DOC) + if spelling is None: + _triggers(doc)["push"].pop("paths") + else: + _triggers(doc)["push"]["paths"] = spelling + with pytest.raises(ExtractionError, match="EVERY push"): + publish_paths(doc) + + +def test_a_paths_ignore_key_is_REFUSED(): + """`paths-ignore` subtracts from what `paths:` selects. Comparing the unreduced list would report + an agreement that does not hold, so the shape is refused rather than approximated.""" + doc = copy.deepcopy(_IMAGE_DOC) + _triggers(doc)["push"]["paths-ignore"] = ["docker/ci/Dockerfile"] + with pytest.raises(ExtractionError, match="paths-ignore"): + publish_paths(doc) + + +def test_a_git_log_OUTSIDE_an_assignment_is_REFUSED(): + """The pathspec must be the one the job acts on. A `git log` whose result is not bound to a + variable — a diagnostic line left behind after `expected` was rewired — must not be adopted as + the subject of the comparison.""" + doc = _rewire_pin_job('expected="$(git rev-parse HEAD)"\n echo "$(git log -1 --format=%H -- docker/ci)"') + with pytest.raises(ExtractionError, match="outside an assignment"): + pin_pathspec(doc) + + +def test_a_git_log_assignment_that_is_never_READ_is_REFUSED(): + """A variable nothing consumes is not what the job checks against.""" + doc = copy.deepcopy(_CHECKS_DOC) + for step in doc["jobs"][PIN_JOB]["steps"]: + if isinstance(step, dict) and isinstance(step.get("run"), str) and "git log" in step["run"]: + step["run"] = ( + step["run"] + .replace("expected=", "unused_audit=") + .replace("${expected}", "X") + .replace('"$expected"', '"X"') + .replace("${expected:0:7}", "X") + ) + with pytest.raises(ExtractionError, match="never"): + pin_pathspec(doc) + + +def test_an_UNMODELLED_publish_spelling_is_REFUSED(): + """Spellings a canonicaliser would have admitted, none of which may compare here. + + Some canonicalise onto the modelled stem and select a DIFFERENT set from it — `docker/ci` is an + anchored match selecting the literal path, and a leading `/` is literal to Gitea while git + REFUSES it outright (`fatal: Invalid path`). Others do not reduce to the stem at all, a brace or + wildcard form among them. The guard does not distinguish those cases: every spelling outside the + modelled one raises.""" + for spelling in ( + "docker/ci", + "docker/ci/", + "docker/ci/**/*", + "./docker/ci/**", + "docker/{ci,extra}/**", + "/docker/ci/**", + "docker/*/**", + "docker/../docker/ci/**", + ): + doc = copy.deepcopy(_IMAGE_DOC) + _triggers(doc)["push"]["paths"] = [spelling] + with pytest.raises(Unnormalisable): + agreement_faults(doc, _CHECKS_DOC) + + +@pytest.mark.parametrize("spelling", [" docker/ci/**", "docker/ci/** ", "docker/ci /**"]) +def test_a_whitespace_padded_publish_pattern_is_REFUSED(spelling): + """The matcher takes padding literally, so trimming it would compare a pattern Gitea never + applies. Trailing padding fails the recursive-form check and the others fail the segment check — + what matters is that none of them compares.""" + doc = copy.deepcopy(_IMAGE_DOC) + _triggers(doc)["push"]["paths"] = [spelling] + with pytest.raises(Unnormalisable): + agreement_faults(doc, _CHECKS_DOC) + + +def test_a_git_log_in_an_INLINE_comment_is_REFUSED(): + """`_pin_step_code` drops whole-line comments only, so an inline `#` leaves a `git log` in the + token stream that never executes. Binding to the assignment is what refuses it.""" + doc = _rewire_pin_job('expected="$(git rev-parse HEAD)"\n : # $(git log -1 --format=%H -- docker/ci)') + with pytest.raises(ExtractionError, match="outside an assignment"): + pin_pathspec(doc) + + +def test_every_named_directory_is_a_tracked_DIRECTORY(): + """`/**` matches nothing while the pathspec `` tracks the file, so the two sides can + name the same string and select different sets. The index is what settles it: a path with no + tracked file beneath it is not a directory.""" + for origin, names in ( + (PUBLISH_ORIGIN, {_publish_directory(p) for p in publish_paths(_IMAGE_DOC)}), + (PIN_ORIGIN, {_pin_directory(p) for p in pin_pathspec(_CHECKS_DOC)}), + ): + for name in sorted(names): + assert tracked_under(name), ( + f"{origin}: {name!r} has no tracked file beneath it, so it is a file or absent " + "rather than a directory. The publish pattern and the pathspec select different " + "sets for such a path even when they spell it identically." + ) + + +def test_a_SECOND_bare_separator_is_REFUSED(): + """A bare `--` can be an option's argument (`--decorate-refs "--"`), so the first one is not + necessarily the path separator. Telling them apart needs git's option arity table; refusing more + than one needs nothing.""" + doc = _rewire_pin_job("expected=\"$(git log -1 --decorate-refs '--' -- docker/ci)\"") + with pytest.raises(ExtractionError, match="bare `--` tokens"): + pin_pathspec(doc) + + +def test_a_HERESTRING_is_NOT_treated_as_a_heredoc(): + """A herestring opens no body, so nothing in it is mistaken for code. Excluded explicitly because + `pr-checks.yml` already uses one in a sibling job, which makes `mapfile -t pins <<< "$(...)"` an + ordinary refactor of this job rather than a hypothetical.""" + doc = _rewire_pin_job(_live_assignment() + '\n mapfile -t x <<< "$(grep -oE y f)"') + assert pin_pathspec(doc) == pin_pathspec(_CHECKS_DOC), ( + "adding a herestring changed what the extractor returned. Expected is DERIVED from the live " + "document, not a literal, so a co-ordinated rename of both lists leaves this test green." + ) + + +def test_a_HEREDOC_in_the_pin_job_is_REFUSED(): + """A heredoc body is data. An assignment written inside one is inert, and a reader that treats + it as code adopts a pathspec the job never uses.""" + doc = _rewire_pin_job( + 'expected="$(git rev-list -1 HEAD -- docs)"\n' + " cat <<'STOP'\n" + ' audit="$(git log -1 --format=%H -- docker/ci)"\n' + ' echo "$audit"\n' + " STOP" + ) + with pytest.raises(ExtractionError, match="heredoc"): + pin_pathspec(doc) + + +def _with_publish_paths(paths: list[str]) -> dict: + doc = copy.deepcopy(_IMAGE_DOC) + _triggers(doc)["push"]["paths"] = paths + return doc + + +def _with_pin_pathspec(replacement: str) -> dict: + """Rewrite the pathspec inside the pin job's real `run:` body, so the mutant exercises the same + extraction path the live document does rather than a hand-built stand-in.""" + doc = copy.deepcopy(_CHECKS_DOC) + current = " ".join(pin_pathspec(_CHECKS_DOC)) + for step in doc["jobs"][PIN_JOB]["steps"]: + if isinstance(step, dict) and isinstance(step.get("run"), str) and "git log" in step["run"]: + step["run"] = step["run"].replace(f"-- {current})", f"-- {replacement})") + assert pin_pathspec(doc) == shlex.split(replacement), ( + f"the pathspec mutant did not take: wanted {replacement!r}, extraction returned {pin_pathspec(doc)!r}" + ) + return doc + + +def _mutants(): + """One divergence per case, each isolating a single direction of the comparison where it can. + + Every mutant keeps both lists NON-EMPTY and inside the modelled shape, so a red here is the + agreement clause speaking rather than an extractor or a shape refusal. + """ + live = publish_paths(_IMAGE_DOC) + + # published > required: a new source publishes images the pin will never name. + yield "publish-path-added", (_with_publish_paths([*live, "docker/extra/**"]), _CHECKS_DOC) + + # published < required: the publish trigger narrows while the pathspec stays wide. + yield "publish-path-narrowed", (_with_publish_paths(["docker/ci/sub/**"]), _CHECKS_DOC) + + # published disjoint from required: the publish trigger is retargeted wholesale. + yield "publish-path-retargeted", (_with_publish_paths(["docker/other/**"]), _CHECKS_DOC) + + # required > published: the pathspec widens, so an unpublished path reddens a blocking job. + yield "pathspec-widened", (_IMAGE_DOC, _with_pin_pathspec("docker")) + + # required grows a second entry the publish trigger does not carry. + yield "pathspec-extra-entry", (_IMAGE_DOC, _with_pin_pathspec("docker/ci docker/extra")) + + +_MUTANTS = list(_mutants()) + + +@pytest.mark.parametrize("pair", [m for _, m in _MUTANTS], ids=[i for i, _ in _MUTANTS]) +def test_a_diverging_list_is_DETECTED(pair): + """The proof this guard can go red. Without it, `agreement_faults` returning a constant empty + list would satisfy the live assertion above and prove nothing.""" + image_doc, checks_doc = pair + assert agreement_faults(image_doc, checks_doc), ( + "the agreement check accepted a workflow pair whose publish paths and pin pathspec name different image sources" + ) + + +def test_the_mutation_set_is_not_empty(): + """The positive control for the parametrisation itself: if `_mutants` yielded nothing, pytest + would collect zero cases above and the file would report all-green having proved nothing.""" + assert len(_MUTANTS) == 5, ( + f"expected 5 divergence mutants, got {len(_MUTANTS)}. An equality rather than a floor, so a " + "generator that degraded to fewer cases cannot pass while silently testing less." + ) + faults = [f for _, pair in _MUTANTS for f in agreement_faults(*pair)] + assert any("named by ci-image.yml's on.push.paths and NOT" in f for f in faults), ( + "no mutant produces a published-side fault, so disarming the `published - required` clause " + "would leave this file green" + ) + assert any("named by ci-image-pin's git-log pathspec and NOT" in f for f in faults), ( + "no mutant produces a pathspec-side fault, so disarming the `required - published` clause " + "would leave this file green" + ) diff --git a/scripts/tests/test_guard_populations_derive_from_git.py b/scripts/tests/test_guard_populations_derive_from_git.py index ecc437bb5..001ac4c2c 100644 --- a/scripts/tests/test_guard_populations_derive_from_git.py +++ b/scripts/tests/test_guard_populations_derive_from_git.py @@ -129,6 +129,12 @@ _PAIRS = [(label, derive) for label, derive, _ in DERIVATIONS] POPULATION_EXEMPT = { # Uses the index to assemble a HERMETIC tmp fixture copy; nothing in it asserts membership. "test_ci_release_path_scan_job.py": "index-derived fixture copy, not a population", + # Its population is the two path lists parsed out of ci-image.yml and pr-checks.yml, derived + # from those documents rather than from the index. It consults the index only to answer a + # per-member question — does this named path have tracked files beneath it, i.e. is it a + # DIRECTORY — because `/**` and the pathspec `` select different sets while spelling + # the same string. Reviewed 2026-08-30. + "test_ci_image_paths_pin_agreement.py": "index used for a per-member directory test, not a population", } _HELPER = "scripts.tests.tracked_files" @@ -216,6 +222,47 @@ def test_the_primitive_does_not_recurse_into_an_untracked_subdirectory(tmp_path, ) +def test_the_nested_primitive_separates_a_DIRECTORY_from_a_FILE_and_a_PREFIX(tmp_path, monkeypatch): + """`tracked_under` decides directory-ness, and two neighbours are what make that non-trivial. + + Its caller (`test_ci_image_paths_pin_agreement.py`) uses an empty result to mean "this path is a + file, not a directory", because `/**` matches nothing while the git pathspec `` tracks + the file — two spellings that look equal and select differently. A prefix sibling is the other + trap: `docker/ci` must not collect `docker/cix/...`, or a file-only path would look like a + directory because of its neighbour's name. + """ + repo = tmp_path / "repo" + (repo / "docker" / "ci").mkdir(parents=True) + (repo / "docker" / "cix").mkdir(parents=True) + (repo / "docker" / "ci" / "Dockerfile").write_text("FROM scratch\n") + (repo / "docker" / "cix" / "Dockerfile").write_text("FROM scratch\n") + (repo / "docker" / "standalone.txt").write_text("a file, not a directory\n") + + def git(*args): + subprocess.run(["git", "-C", str(repo), *args], check=True, capture_output=True) + + git("init", "-q") + git("config", "user.email", "guard@example.invalid") + git("config", "user.name", "guard") + git("add", "-A") + git("commit", "-qm", "a directory, a prefix sibling, and a plain file") + + monkeypatch.setattr(tracked_files, "REPO_ROOT", repo) + + assert tracked_files.tracked_under("docker/ci") == {"docker/ci/Dockerfile"}, ( + "the prefix sibling docker/cix/ leaked into docker/ci's subtree, so a path would be judged a " + "directory because of what its NEIGHBOUR is named" + ) + assert tracked_files.tracked_under("docker/ci/") == {"docker/ci/Dockerfile"}, ( + "a trailing slash changed the answer; it is the same directory" + ) + assert tracked_files.tracked_under("docker/standalone.txt") == set(), ( + "a tracked FILE reported a subtree, which is the emptiness its caller reads as 'this is a " + "directory' — the `/**` vs `` false green" + ) + assert tracked_files.tracked_under("docker/absent") == set(), "an absent path reported a subtree" + + def test_an_empty_index_FAILS_LOUDLY_rather_than_reporting_an_empty_population(tmp_path, monkeypatch): """The floor under every floor. A silent empty population is how a completeness guard reports total coverage having examined nothing, which is the failure mode this repo has shipped twice @@ -577,8 +624,9 @@ def test_every_index_derived_module_is_registered(): WHAT NO ASSERTION CAN DECIDE, dated so it is re-examined rather than assumed: whether an exemption is still WARRANTED. A stale key and an unexplained one are both caught below, but an exempt module that later grows a real derived population stays uncovered and silent. Reviewed - 2026-08-22 — the single entry uses the helper only to assemble a tmp fixture copy and asserts - nothing about membership. + 2026-08-30 — two entries. One uses the helper only to assemble a tmp fixture copy; the other + consults the index per-member, to decide whether a named path is a directory, and derives its + population from parsed workflow documents instead. Neither asserts a file-population claim. """ exempt = {Path(__file__).name} | set(POPULATION_EXEMPT) importers = _modules_importing_the_helper() diff --git a/scripts/tests/tracked_files.py b/scripts/tests/tracked_files.py index 2233ba9b9..727c14352 100644 --- a/scripts/tests/tracked_files.py +++ b/scripts/tests/tracked_files.py @@ -102,6 +102,18 @@ def tracked_children(directory: str, patterns: tuple[str, ...]) -> set[str]: return found +def tracked_under(directory: str) -> set[str]: + """Tracked files anywhere BENEATH `directory`, recursively. + + This is the explicit nested ask `tracked_children` directs callers to when direct children are + not enough. It decides whether a path names a directory at all: a path with nothing beneath it + is a file or absent, and a caller that assumed a directory would otherwise compare two spellings + that select different sets. + """ + prefix = directory.rstrip("/") + "/" + return {path for path in _git_ls_files() if path.startswith(prefix)} + + def tracked_paths(directory: str, patterns: tuple[str, ...]) -> list[Path]: """`tracked_children` as absolute `Path`s, sorted — for guards that read member contents.