Add the deterministic pull-request hygiene check

[iderex]

The issue this belongs to

Closes #83.

What changed

Two files, both new, and nothing existing is touched.

.github/pr-hygiene/hygiene.sh holds three rules as shell functions. The body
names an issue. The body carries at least one line the template did not put
there. Every changed path is inside a Scope: that one of the named issues
declares, where any of them declares one. It also holds the fixtures for those
rules and runs them under selftest.

.github/workflows/pr-hygiene.yml runs it on every pull request, under the
check-run name Deterministic PR-hygiene checks. The run executes the fixtures
first and refuses to judge the pull request at all if one of them did not hold,
so a green verdict is one whose rules were proven in the same minute.

The rules are functions in a file rather than steps in the workflow for one
reason. Each rule owes a fixture proving it bites, and a fixture run against a
second copy of the logic proves the copy rather than the rule. selftest and
check call the same functions.

The run prints what it did not compare. On this repository that is the scope
comparison every time, because no issue here declares a Scope: line at column
zero, so the changed paths are compared against nothing and the run says so in
those words. It also prints that it read no commits, because the sign-off is
.github/workflows/dco.yml's and is not reimplemented here.

The means is shell and awk inside the workflow apparatus this repository already
carries in unicode-guard.yml and dco.yml. It fits because the check reads a
pull request and the issues it names rather than any of the core's own source,
so it needs nothing from the language and toolchain question in #11, and it adds
no language, runtime or dependency the tree does not already have. awk rather
than grep throughout, because grep exits 1 when it selects nothing, which is the
ordinary answer for most of these rules, and a pipeline that has to tell "nothing
matched" from "the scanner broke" one set -o pipefail at a time is how a gate
ends up passing on everything.

What failure it prevents

Nothing on this repository reads a pull request today. Whether a change names the
issue it belongs to, and whether its body says anything a reader can check the
change against, is caught by whoever happens to look and missed when nobody does.

The narrower failure is the change that arrives as the template with the number
filled in and nothing written. It looks complete in a list of pull requests, it
carries no statement of what changed or what failure it prevents, and the reader
who needs that is the one reading it a year later.

Evidence

Every command below was run at 0e1dc63b73fd16d2235438d96eafe99ace9e8d38, the
head of this branch.

The fixtures, which are also the proof each rule bites:

bash .github/pr-hygiene/hygiene.sh selftest

== names-an-issue ==
ok    bites: the template's own reference, with no number after the hash
ok    passes: the same body one character longer
ok    passes: named without a closing keyword, which is this board's practice
ok    bites: a reference only inside an HTML comment
ok    bites: a heading is not a reference
ok    passes: several issues, deduplicated and in order
== body-not-empty ==
ok    bites: the template with the issue number filled in and nothing written
ok    passes: the same body with one sentence added
ok    bites: a body that is only whitespace
ok    bites: a body that is only a list of issue numbers
ok    passes: a sentence about those issues rather than a list of them
== changed-paths-inside-scope ==
ok    bites: a neighbouring directory whose name starts the same way
ok    passes: every path under the declared prefix
ok    bites: one path outside two declared prefixes
== scope-declaration ==
ok    reads: a Scope line at column zero, split on commas
ok    does not read: the same words inside a sentence
ok    does not read: a body declaring no scope at all

Every fixture held. The rules the gate applies are the rules these fixtures ran.

The size of what landed:

wc -l .github/pr-hygiene/hygiene.sh .github/workflows/pr-hygiene.yml
  356 .github/pr-hygiene/hygiene.sh
   71 .github/workflows/pr-hygiene.yml
  427 total

That is over the 400 line figure this organisation inherits as a readable cap,
by 27 lines, and it is stated rather than rounded down. One property holds across
every line of it, which is that each rule is decided by reading the pull request
and carries a fixture that refuses a body one character away from a good one, so
a reader checks that property rather than the diff. Splitting it into a rules
pull request and a fixtures pull request would produce two changes that only make
sense together and neither of which is reviewable alone.

Neither file carries a carriage return, checked against the committed blob rather
than the working copy, because this clone checks out with line-ending translation
on:

for f in .github/pr-hygiene/hygiene.sh .github/workflows/pr-hygiene.yml; do
  printf '%s\t' "$f"; git cat-file -p "HEAD:$f" | tr -cd '\r' | wc -c; done
.github/pr-hygiene/hygiene.sh	0
.github/workflows/pr-hygiene.yml	0

That is a fact about this commit and not a guard. .gitattributes does not exist
in this tree and #99 is where the guard belongs.

What a guard here refuses, and the proof it bites

Each rule was deleted in turn and the fixtures were watched going red. Three
runs, each with the deletion described and the fixture that caught it:

issue_refs stops removing HTML comments before it looks for a reference:

FAIL  bites: a reference only inside an HTML comment
::error::1 hygiene fixture(s) did not hold.

body_prose stops dropping lines that are nothing but issue references:

FAIL  bites: the template with the issue number filled in and nothing written
FAIL  passes: the same body with one sentence added
FAIL  bites: a body that is only a list of issue numbers
::error::3 hygiene fixture(s) did not hold.

paths_outside stops requiring a prefix to match a whole path segment, which is
the one-character version of that rule that admits a neighbouring directory:

FAIL  bites: a neighbouring directory whose name starts the same way
::error::1 hygiene fixture(s) did not hold.

The first rule was written the wrong way round and the measurement is what found
it. It required a closing keyword, and run against every pull request already
merged here it refused twelve of the seventeen:

for n in $(gh pr list --repo Flowfin/core --state merged --limit 60 --json number --jq '.[].number' | sort -n); do
  gh pr view "$n" --repo Flowfin/core --json body --jq '.body' > /tmp/b.md
  gh api "repos/Flowfin/core/pulls/$n/files" --paginate --jq '.[].filename' > /tmp/p.txt
  if GH_REPO=Flowfin/core PR_NUMBER=$n PR_BODY_FILE=/tmp/b.md CHANGED_PATHS_FILE=/tmp/p.txt \
     bash .github/pr-hygiene/hygiene.sh check >/dev/null 2>&1
  then echo "pass    #$n"; else echo "REFUSED #$n"; fi
done

pass    #118
pass    #119
pass    #120
pass    #121
pass    #122
REFUSED #123
REFUSED #124
REFUSED #125
REFUSED #126
REFUSED #127
REFUSED #128
REFUSED #129
REFUSED #130
REFUSED #131
REFUSED #132
REFUSED #133
REFUSED #134

The practice on this repository is to name the issue and deliberately not close
it while its definition of done is unmet, and each of those twelve says so in its
own words. A rule refusing the honest half of a practice is a rule that gets
worked around in its first week, which is the deviation #83 asks this check to
keep. Named rather than closed is what landed, and the same loop at this commit
refuses none of them:

(same loop, at 0e1dc63)

pass    #118
pass    #119
pass    #120
pass    #121
pass    #122
pass    #123
pass    #124
pass    #125
pass    #126
pass    #127
pass    #128
pass    #129
pass    #130
pass    #131
pass    #132
pass    #133
pass    #134

What this does not cover

The sign-off is not read here. .github/workflows/dco.yml refuses a commit whose
Signed-off-by trailer does not match its author, which is stricter than asking
whether one is present, and a second implementation of one rule is two things
that agree until they drift. #83 lists the sign-off among what this check
produces, and delegating it rather than writing it twice is a departure from that
list. The run prints that it read no commits, so a green verdict cannot be read
as this check having looked.

The scope comparison is never made on this repository today, because no issue
here declares a Scope: line at column zero. Every run prints that in those
words. The rule and its fixtures exist so that the comparison starts working on
the day an issue declares one, rather than being written then.

Where several named issues declare scopes, the comparison is against their union,
so naming a further issue can only widen it. It is a floor rather than a fence,
and the run says so.

Nothing here judges whether a body says something true, whether its evidence is
evidence, or whether the change belongs to the issue it names. Those need a
person and they are what a review is.

A body naming an issue number that resolves to nothing reddens this check rather
than being ignored, because a lookup that failed is not an issue that declares no
scope, and reading it as one turns an outage into a pass.

Nothing here makes this check required. The ruleset on main carries no required
status check today, and which names become required is #26 taking its answer from
#113.

The rules were written and run under gawk, and the awk on the runner is mawk. No
POSIX character class and no interval expression appears in any pattern, which is
where those two implementations disagree, but that was an argument rather than a
measurement when this was opened. The first run of this workflow, on this pull
request, is the first execution under mawk, and it is now measured rather than
argued:

gh run view 31567686059 --log

Every fixture held. The rules the gate applies are the rules these fixtures ran.

Pull request #135 on Flowfin/core.
Examined: the body as sent, and 2 changed path(s).

-- names-an-issue
ok    names: 11 26 83 99 113 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134

-- body-not-empty
ok    the body carries 177 line(s) the template did not

-- changed-paths-inside-scope
NOT MADE: no issue this pull request names declares a 'Scope:' line at column zero, so the changed paths were compared against nothing. This is a pass with no comparison behind it, and it is not a pass with one.

-- what this run did not read
NOT MADE HERE: the DCO sign-off. .github/workflows/dco.yml refuses a commit whose Signed-off-by does not match its author; this run read no commits.
NOT MADE HERE: anything needing judgement. Whether the body says something true, whether the evidence is evidence, and whether the change belongs to the issue it names are read by a person.

Every rule this check owns passed.

That run judged its own pull request, which is the narrowest useful test of it:
the rules a change adds are the rules that change is measured by.

Who has read it

Nobody other than the author has read this change. The evidence above stands in
place of a review rather than beside one.