diff --git a/.github/workflows/pr-metadata-policy.yml b/.github/workflows/pr-metadata-policy.yml index 4f09617..20985bf 100644 --- a/.github/workflows/pr-metadata-policy.yml +++ b/.github/workflows/pr-metadata-policy.yml @@ -26,11 +26,23 @@ permissions: jobs: pr-metadata-policy: name: pr metadata policy - # Self-hosted where the org provides a label, GitHub-hosted otherwise. The - # fallback matters: this job must be able to run in a repo that has no - # self-hosted capacity, or adopting it would mean adopting a runner - # dependency. Override CI_LABEL_LINUX_X64 at the org/repo level to pin it. - runs-on: ${{ vars.CI_LABEL_LINUX_X64 && fromJSON(vars.CI_LABEL_LINUX_X64) || 'ubuntu-latest' }} + # GitHub-hosted by default, even in an org with a large self-hosted fleet. + # + # This job is a few seconds of stdlib Python with no build, no toolchain and + # no cache to reuse, so a self-hosted queue buys it nothing and costs it the + # thing that matters most for a REQUIRED check: a guaranteed conclusion. On + # the EvoMap fleet, routing it to `[self-hosted, Linux, X64]` produced jobs + # cancelled with "not acquired by Runner of type self-hosted even after + # multiple attempts" — a pool whose autoscaled members can report online and + # still fail to pick up work. A required check that never reaches a + # conclusion blocks the PR exactly as hard as a failing one, and no amount of + # re-running fixes it reliably. + # + # PR_GATE_RUNNER is a dedicated opt-in, deliberately NOT the org's general + # CI label: a repo that wants this job on its own capacity sets it, and + # nobody inherits a runner dependency by having a fleet. Set it to a JSON + # array of labels, e.g. '["self-hosted","Linux","X64"]'. + runs-on: ${{ vars.PR_GATE_RUNNER && fromJSON(vars.PR_GATE_RUNNER) || 'ubuntu-latest' }} steps: - name: Check out base branch (never the PR head) # Pinned to a full commit SHA, not a tag: a tag is mutable, so `@v4` diff --git a/scripts/validate_pr_metadata.py b/scripts/validate_pr_metadata.py index 53a874a..4c1d856 100644 --- a/scripts/validate_pr_metadata.py +++ b/scripts/validate_pr_metadata.py @@ -72,6 +72,11 @@ re.IGNORECASE, ) +# "and so on" written inside a template's choice menu. A menu entry matching this +# is the template trailing off, not an alternative the author may select, so it +# must not become an accepted answer when the menu is parsed. +CONTINUATION_RE = re.compile(r"^(?:\.{2,}|…+|etc\.?|and so on)$", re.IGNORECASE) + TEMPLATE_CANDIDATES = ( ".github/PULL_REQUEST_TEMPLATE.md", ".github/pull_request_template.md", @@ -190,12 +195,39 @@ def _offered_choices(template_default: str) -> set[str]: placeholder. `Data egress impact: none | local-only | network-content` makes `none` the correct answer for a docs change, so the placeholder filter must not reject it — the author picked from the menu the repo wrote. + + Real templates write the menu two ways, and both have to count. EvoX writes + `Builds on: <#PR / #issue / docs/… path / none>` and documents `none` as the + answer for genuinely fresh work, so recognising only the pipe form rejected + the answer the repo's own template told the author to give. + + Two details keep the slash form from over-accepting: + + * The separator must be a SPACED slash. A bare `/` is a path separator, so + splitting on it would shred `docs/… path` into bogus choices and promote + every single path component into an "offered" answer. + * A continuation marker is not a choice. `Phase: ` + offers three phases and an et-cetera; without this filter, stripping the + angle brackets would make the literal `...` a valid answer. Placeholder- + LOOKING choices such as `none` are kept on purpose — a word the template + lists is an answer, which is the whole point of this function. """ default = normalize(template_default) - if "|" not in default: + # Templates bracket the menu as often as not (``). The brackets + # are punctuation around the list, not part of the first and last choice. + if default.startswith("<") and default.endswith(">"): + default = default[1:-1].strip() + if "|" in default: + parts = default.split("|") + elif " / " in default: + parts = default.split(" / ") + else: return set() - choices = default.split("|") - return {c.strip().casefold() for c in choices if c.strip()} + return { + choice.casefold() + for choice in (part.strip() for part in parts) + if choice and not CONTINUATION_RE.fullmatch(choice) + } def is_answered(value: str, template_default: str = "") -> bool: