From 5078b9427b9d69ea281d7c1958d0c8fad1715a4a Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:08:36 +0200 Subject: [PATCH] Restore the two sections of the parity document that left the branch (#155) This does not finish #155. One path group that commit removed is still missing. What was wrong. `d3edfc95b8526033c79cb26afe48282c2c090e32` took its tree from an older state of the default branch and landed on top of a newer one, under a message describing a change to how one workflow pin is commented. `docs/quality-parity.md` was among the seven paths it replaced, and the file went from 631 lines to 209. Two sections come back. The five declared check names the table gave no verdict to, which is the shape that costs #26 an answer rather than a line of prose, because a name the document leaves unjudged can be read as kept or as dropped with equal justice and the two readings build two different gates. And the whole of `## Which contexts arrive, and on which pull requests`, which is the walk of which contexts reach which commits. Three rows of the table above them come back with the same hunk, and one of the three is wrong on the default branch today rather than merely shorter. The `prettier` row there says the prose half is issue #50 and that nothing in this tree formats Markdown. Both halves have since stopped being true: gh issue view 50 --repo Flowfin/lab --json number,state --jq '"#\(.number) \(.state)"' #50 CLOSED git grep -n 'name: prose format' origin/main -- .github/workflows/ origin/main:.github/workflows/prose.yml:53: name: prose format This is an insertion rather than a blob restore, because #156 added a row to the parameter table well below both sections, so the hashes differ and the diff is what to read. What it says is that the file differs from the state before the removal by that row and the two paragraphs above it, and by nothing else: git diff --stat 90656ba HEAD -- docs/quality-parity.md docs/quality-parity.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) grep -c 'require_extra_approval_for_unattributed_changes' docs/quality-parity.md 1 What failure this prevents is #26 assembling a required set out of a document that declares five names and judges none of them, and a reader taking the table for the whole answer while the question of which contexts arrive on which pull requests is answered nowhere. Every command the restored text pastes was re-run against the current `origin/main` before this change, rather than trusted because it had once been true, and all of them still reproduce. One deserves naming: the paste reading `zizmor.yml` lines 76 to 84 reproduces because the comment removed from that file by the same commit is back, so it would have been stale had this landed first. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- docs/quality-parity.md | 428 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 425 insertions(+), 3 deletions(-) diff --git a/docs/quality-parity.md b/docs/quality-parity.md index 6533cf0..b322916 100644 --- a/docs/quality-parity.md +++ b/docs/quality-parity.md @@ -53,14 +53,14 @@ today no tick does. | `ABI floor build` | Dropped | It exists so a plugin loads against the oldest server it claims to support, and nothing here loads into a host. | | `Package (JPRM) / Build package` | Dropped | This repository ships no plugin, so there is no package to build. | | `Package (JPRM) / Generate SBOM` | Kept in substance, moved out of the gate | A bill of materials is owed for anything downloadable, and this one is produced by the release build rather than by a check on a pull request. Issue #37 builds it and nothing in this tree produces one today. | -| `CodeQL` | Kept, retargeted | Static analysis of the runner's own source, in the language record `0001` chose rather than in C#. | +| `CodeQL` | Kept, retargeted | Static analysis of the runner's own source, in the language record `0001` chose rather than in C#. Two strings arrive here for it, the analysis job's `CodeQL (go)` and the code-scanning upload's `CodeQL`. Which of the two a required set can hold is open rather than decided below: the section on which contexts arrive separates the strings, and says of the upload name that the route which would decide it has not been walked. Issue #62 holds that walk. | | `Analyze (csharp)` | Dropped as a name | The language-specific analysis job is replaced by the equivalent for this language rather than carried across under a name that describes nothing here. | | `DCO sign-off` | Kept unchanged | Already in the tree, asserting the text at `DCO` on every non-merge commit. | | `Deterministic PR-hygiene checks` | Kept, adapted | The class the other checks miss is the pull request itself, and one of the three refusals is this board's own invariant about a record moving with the code it describes. It is in the tree as the `pull request` job, judged in `internal/pullrequest/` and run from `.github/workflows/pull-request.yml`. | | `Enforce greppable invariants` | Kept, different invariants | The invariants are properties of this repository's own tracked text, which is a different set from the target's, and they are in `internal/invariants/`. | | `Reject Trojan Source Unicode` | Kept unchanged | Already in the tree, and the attack it refuses is a property of source rather than of a language. | -| `Audit workflows (zizmor)` | Kept unchanged | Already in the tree. The workflow YAML is the other executable thing here and it runs with write scopes. | -| `prettier` | Kept, split in two | Records here are Markdown, and a whitespace diff on a record hides the sentence that changed. The runner's own source is held to `gofmt` by a job already in the tree; the prose half is issue #50 and nothing in this tree formats Markdown today. | +| `Audit workflows (zizmor)` | Kept unchanged | Already in the tree. The workflow YAML is the other executable thing here and it runs with write scopes. The job reports under this name and the code-scanning upload reports under `zizmor`, which is a different context and does not arrive on every pull request. | +| `prettier` | Kept, split in two, and both halves are in the tree | Records here are Markdown, and a whitespace diff on a record hides the sentence that changed. The runner's own source is held to `gofmt` by the `format` job and the prose half is the `prose format` job. Both are kept and both belong in the required set. Neither rewrites anything, which is where the split departs from the target: `prettier` formats and these two refuse, so a departure is a red tick here and a diff there. | | `dependency-review` | Kept unchanged | Already in the tree, refusing a newly introduced dependency carrying a known advisory. | ## What this board adds that the target does not have @@ -75,6 +75,78 @@ requirement, and the job proves the default suite completes with no graphical session and as an ordinary user. The target does not carry it because a plugin assembly is not run by a contributor on their own machine in the same way. +Three more names this tree declares belong in this section, and until this +paragraph none of them appeared anywhere in this document, in a row or in a +sentence. They are declared here: + +``` +git grep -h -E '\{Name: "(test \(|vet"|required contexts")' origin/main \ + -- internal/contexts/contexts.go | sed 's/^[[:space:]]*//' +{Name: "test (linux/amd64)", Why: theSetIsEmpty, Until: "#26"}, +{Name: "test (windows/amd64)", Why: theSetIsEmpty, Until: "#26"}, +{Name: "test (darwin/arm64)", Why: theSetIsEmpty, Until: "#26"}, +{Name: "vet", Why: theSetIsEmpty, Until: "#26"}, +{Name: "required contexts", Why: theSetIsEmpty, Until: "#26"}, +``` + +The leading indentation is stripped so that this document carries no tab, which +`prose-carries-a-tab` refuses in tracked Markdown. Nothing else about the five +lines is changed. + +A name with no verdict here is the shape that costs issue #26 an answer rather +than a line of prose. The set it assembles is taken from this document, and a +declared name the document gives no verdict to can be read as kept or as +dropped with equal justice, so the two readings produce two different gates. +Each entry below therefore says kept or dropped and why, in the same terms the +table above uses. + +The platform suite. Record `docs/decisions/0012-the-supported-platforms.md` +gives three of its six platforms a suite run as well as a build, and each of +the three reports under its own name, so the entries are `test (linux/amd64)`, +`test (windows/amd64)` and `test (darwin/arm64)`. The target requires a build +and requires nothing that runs a suite: + +``` +gh api repos/Flowfin/jellyfin-plugin-sso/rules/branches/main \ + --jq '.[] | select(.type=="required_status_checks") + | .parameters.required_status_checks[].context' \ + | grep -E '^(test|vet)' ; echo "exit=$?" +exit=1 +``` + +All three are kept and they belong in the required set. The differences record +`0012` picks those three platforms for, whether the filesystem folds case, what +separates a path and what a line ending arrives as, are the defects a runner +that reads a checkout actually has, and a build entry that compiled on a +platform says nothing about whether the runner reads that platform's tree +correctly. The target has no equivalent because the artefact it gates is loaded +by a server rather than run against a working copy. + +`vet` is kept, and it belongs in the set at a smaller cost than the suite: one +job over the whole module rather than three entries on three machines, since +what it reads is the source rather than the filesystem underneath it. It has no +counterpart in the command above for the same reason the language-specific +analysis row does not carry across, which is that the target is written in +another language and holds its own source to that language's tools. + +`required contexts` is kept, and it is the entry a reader is likeliest to find +absent here rather than wrong, because it is younger than the walk this +document is built on. The table above reads a ruleset as it answered on +2026-08-10 and this check landed two days later: + +``` +git log -1 --format='%h %ad %s' --date=short --diff-filter=A \ + -- .github/workflows/contexts.yml +2b954f5 2026-08-12 Refuse a required context and a check name that disagree (#71) +``` + +What it compares is the required set against the check names this tree +declares, so it is the one entry whose subject is the gate rather than the +tree. Requiring it is what refuses a set edited into disagreement with the +workflows, in either direction, and that is the direction no document can cover +because nothing reads a document. The target has no equivalent and the absence +is not an argument against carrying one here. + The supply-chain self-audit stays outside the required set, for the same reason it is outside the target's. It publishes from the default branch and cannot gate a pull request, so requiring it would require a context that never @@ -94,6 +166,356 @@ gh api repos/Flowfin/lab/commits/$(git rev-parse origin/main)/check-runs \ That is the command issue #26 assembles the required set from, and it is the one to run before quoting any context name back at this document. +## Which contexts arrive, and on which pull requests + +A required context that does not arrive is not a red tick. It is a pull request +that cannot merge and says nothing about why, so the first response is to wait +and the second is to make the gate smaller. The names above are therefore +separated by what produces them and by the commit they were read from before +any of them goes into a required set. Issue #62 holds this walk. + +### The command above reads a commit no pull request produced + +`git rev-parse origin/main` resolves a commit on the default branch, which is +reached by a push. Three workflows in this tree carry no push trigger: + +``` +git grep -L 'push:' origin/main -- .github/workflows/ +origin/main:.github/workflows/dco.yml +origin/main:.github/workflows/dependency-review.yml +origin/main:.github/workflows/pull-request.yml +``` + +So the two kinds of commit report different sets, and the difference runs in +both directions. Between the head of the default branch and the head of a pull +request that landed on it, on 2026-08-16: + +``` +gh api repos/Flowfin/lab/commits/82c245f/check-runs?per_page=100 \ + --jq '.check_runs[].name' | sort -u > default-branch.txt +gh api repos/Flowfin/lab/commits/7374b6b/check-runs?per_page=100 \ + --jq '.check_runs[].name' | sort -u > pull-request.txt + +comm -23 default-branch.txt pull-request.txt +Scorecard analysis + +comm -13 default-branch.txt pull-request.txt +CodeQL +DCO sign-off +dependency-review +pull request +zizmor +``` + +Three of those five are the pull-request-only workflows above and two are the +subject of the next section. `Scorecard analysis` is the opposite case and is +already written down as a permanent absence in `internal/contexts/contexts.go`, +for the reason the supply-chain paragraph above gives. + +A set assembled from a default-branch commit therefore leaves out three +contexts that every pull request reports, and it offers one that no pull +request can report. Which commit that command is run against decides what the +gate gets. + +### One name is reported twice on the same commit + +Every command in this document that lists check names deduplicates, and one of +the names they print once was produced by two runs. On `bd861cf`, the head of +pull request #145: + +``` +gh api repos/Flowfin/lab/commits/bd861cf/check-runs?per_page=100 \ + --jq '.check_runs[].name' | sort | uniq -d +Reject Trojan Source Unicode + +gh api repos/Flowfin/lab/commits/bd861cf/check-runs?per_page=100 \ + --jq '.check_runs[] | select(.name=="Reject Trojan Source Unicode") + | "suite \(.check_suite.id), \(.conclusion)"' +suite 86798670354, success +suite 86798547856, success +``` + +The cause is one trigger. Every push trigger in these files names the default +branch except one: + +``` +for f in $(git ls-tree --name-only origin/main .github/workflows/); do + s=$(git show "origin/main:$f" | sed -n '/^ push:/{n;s/^ *//;p;}') + [ -n "$s" ] && printf '%s %s\n' "$f" "$s" +done +.github/workflows/build.yml branches: [main] +.github/workflows/codeql.yml branches: [main] +.github/workflows/contexts.yml branches: [main] +.github/workflows/headless.yml branches: [main] +.github/workflows/invariants.yml branches: [main] +.github/workflows/prose.yml branches: [main] +.github/workflows/records.yml branches: [main] +.github/workflows/scorecard.yml branches: [main] +.github/workflows/unicode-guard.yml branches: ["**"] +.github/workflows/zizmor.yml branches: [ main ] +``` + +A branch pushed for a pull request therefore starts that one workflow twice, +and both runs report under its job name. The file gives the reason its trigger +is wide, and the reason is about which branches the guard covers rather than +about the gate: + +``` +git show origin/main:.github/workflows/unicode-guard.yml | sed -n '3,11p' +on: + # Every branch and every PR: the guard is a cheap read-only scan, so there is no + # reason to narrow it to main, and it covers whatever branches this repository + # grows later without being edited again. + push: + branches: ["**"] + pull_request: + branches: ["**"] +``` + +The two runs do not read the same tree, and that is what makes this more than a +repeated line. Each checkout says what it took, and the answers differ: + +``` +gh run view 32017539721 --log \ + | sed -n 's/.*\(git checkout --progress --force .*\)/\1/p' +git checkout --progress --force -B parity/one-job-asks-the-platform-and-the-fork-half-is-unwalked refs/remotes/origin/parity/one-job-asks-the-platform-and-the-fork-half-is-unwalked + +gh run view 32017585271 --log \ + | sed -n 's/.*\(git checkout --progress --force .*\)/\1/p' +git checkout --progress --force refs/remotes/pull/145/merge +``` + +The first read the branch and the second read the branch merged into the base. +A tree that carries none of these characters on the branch and carries one once +merged separates the two runs, which is the case this particular guard exists +for one merge earlier. + +`Reject Trojan Source Unicode` is a row the table above keeps, so it is a +candidate for the required set. What a merge does when two check runs answer to +one name is platform behaviour that nothing in this tree states, and the set +here is empty, so it cannot be measured on this board today either. Issue #26 +assembles the set and its first clause meets this, because the command it +assembles from prints one line for the two. + +### Two of the names come from an upload rather than from a job + +The names on that same pull request head that no job in this tree produced: + +``` +gh api repos/Flowfin/lab/commits/7374b6b/check-runs?per_page=100 \ + --jq '.check_runs[] | select(.app.slug != "github-actions") + | "\(.name) is created by \(.app.slug)"' | sort -u +CodeQL is created by github-advanced-security +zizmor is created by github-advanced-security +``` + +The two rows in the table above carry the target's strings. Here each of them +is two strings rather than one: the analysis job reports `CodeQL (go)` and the +code-scanning upload reports `CodeQL`, the audit job reports +`Audit workflows (zizmor)` and the upload reports `zizmor`. Two of the four are +written in a workflow file and two are written in no file in this tree, which +is how `internal/contexts/contexts.go` holds them. + +### The upload is conditional and the step that fails on findings is not + +``` +git show origin/main:.github/workflows/zizmor.yml | sed -n '76,84p' + - name: Upload SARIF + # Only upload where the GITHUB_TOKEN can write security events: pushes to main + # and same-repo human PRs. Fork and Dependabot pull requests run with a + # read-only token, so the upload is skipped there - the gate step below still + # runs and blocks on findings. continue-on-error keeps the security gate + # independent of the upload: a transient code-scanning upload failure must not + # skip the "Fail on actionable findings" step below. + if: (github.event_name == 'push' && github.ref == 'refs/heads/main') || (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.user.login != 'dependabot[bot]') + continue-on-error: true +``` + +That condition has a pull request on this board today that it excludes, so the +outcome is measured rather than read off the file. On the head of #135, whose +author the second arm names: + +``` +gh api repos/Flowfin/lab/actions/runs/31929377858/jobs \ + --jq '.jobs[].steps[] | select(.number >= 4 and .number <= 6) + | "step \(.number), \(.name): \(.conclusion)"' +step 4, Audit workflows (SARIF for code scanning): success +step 5, Upload SARIF: skipped +step 6, Fail on actionable findings: failure +``` + +The upload did not run, and only one of the two names reached that commit: + +``` +gh api repos/Flowfin/lab/commits/d18d040/check-runs?per_page=100 \ + --jq '.check_runs[] | select(.name | test("zizmor")) + | "\(.name): \(.conclusion)"' | sort -u +Audit workflows (zizmor): failure +``` + +So the job arrived and reported what the audit found while the upload-derived +context did not arrive at all, which is the separation the comment in the +workflow file argues for. It decides one name: `zizmor` cannot be in the +required set, because requiring it would hold open every pull request the +condition excludes, with nothing on the pull request saying why. That absence is +permanent rather than one issue #26 retires, and the job name beside it is +unaffected. + +### None of these workflows narrows itself to some pull requests + +A context that arrives on some pull requests and not on others is what this +section is written against, and three of the ways to build one are readable in +these files rather than walked. The readings below were made at `1fc6961` and +cover every workflow file in the tree at that commit. + +A path filter is the ordinary way it happens, and no workflow here carries one: + +``` +git grep -n 'paths:\|paths-ignore:' origin/main -- .github/workflows/ ; echo "exit=$?" +exit=1 +``` + +A trigger narrowed to some branches is the same failure by a second route, and +three of these files carry no branch filter that reads every branch: + +``` +git grep -L 'branches: \[ *"\*\*" *\]' origin/main -- .github/workflows/ +origin/main:.github/workflows/dco.yml +origin/main:.github/workflows/dependency-review.yml +origin/main:.github/workflows/scorecard.yml +``` + +The third is the supply-chain self-audit, which declares no pull-request +trigger at all and is outside the required set already. Neither of the other +two narrows anything. `dependency-review.yml` writes no branch filter under any +of its triggers, which is every branch: + +``` +git grep -n 'branches:' origin/main -- .github/workflows/dependency-review.yml ; echo "exit=$?" +exit=1 +``` + +`dco.yml` carries the only type filter in these files: + +``` +git grep -n 'types:' origin/main -- .github/workflows/ +origin/main:.github/workflows/dco.yml:13: types: [opened, synchronize, reopened] +``` + +The claim about the three types it names is that they are the three the +platform runs a pull-request workflow for when a file names none, so writing +them out removes no pull request. That is the platform's documented default +rather than something this tree says, and nothing here measures it. What sits +beside it is that `DCO sign-off` is among the contexts the pull-request head +compared above reported, which shows the workflow runs on an ordinary pull +request and shows nothing about one that arrives another way. + +A job skipped by a condition is the third, and two of these files carry one at +all: + +``` +git grep -c '^\s*if:' origin/main -- .github/workflows/ +origin/main:.github/workflows/scorecard.yml:1 +origin/main:.github/workflows/zizmor.yml:1 +``` + +Neither reaches a name the table above keeps. The first is a job-level +condition on the supply-chain self-audit, which the paragraph above already +places outside the required set for a reason of its own. The second is the +upload condition quoted earlier in this section, indented under a step rather +than a job, and the step that fails on findings sits after it carrying no +condition. + +What these commands do not reach. They read the path filters, the branch and +type filters and the `if:` keys, which is what those three are written with in +these files today, and a fourth route written some other way is not something a +reader can take from them. None of them says anything about +what a job does once it has started, so a context that arrives having done +nothing is a different question and is not answered here. + +### One job can start and still be unable to do its work + +The readings above stop at the moment a job begins. One job here has a second +failure after that moment, because half of what it compares is a live setting +on the platform rather than a file in the checkout, so it has to ask. No other +workflow in this tree asks anything: + +``` +git grep -n 'gh api' origin/main -- .github/workflows/ +origin/main:.github/workflows/contexts.yml:93: if ! gh api "repos/${REPOSITORY}/rules/branches/${DEFAULT_BRANCH}" \ +``` + +That reads the run blocks these files carry, so it says which workflow asks the +platform a question in a script this repository writes. What an action reaches +once it has started is not readable from here and the command says nothing +about it. + +The job reports as `required contexts`, which the section above keeps and puts +in the required set. What it does when the answer does not arrive is written +into the step rather than left to the shell: + +``` +git show origin/main:.github/workflows/contexts.yml | sed -n '92,100p' + set -euo pipefail + if ! gh api "repos/${REPOSITORY}/rules/branches/${DEFAULT_BRANCH}" \ + --jq '.[] | select(.type=="required_status_checks") + | .parameters.required_status_checks[].context' > required.txt; then + echo "::error::The ruleset on ${DEFAULT_BRANCH} could not be read, so this run could not judge whether the gate and the tree agree. That is not the same as them agreeing." + exit 1 + fi + echo "the ruleset on ${DEFAULT_BRANCH} requires $(wc -l < required.txt) context(s):" + cat required.txt +``` + +So this route ends in a red context carrying its reason rather than in a +context that never arrives, which is the better of the two failures and is +still a pull request held open by something other than the change on it. An +empty answer is not that failure. The count is printed and the run carries on, +which is the state this board is in today. + +One of the two things a fork run brings is measurable here already. A +Dependabot pull request runs with a read-only token, which is what the upload +condition quoted earlier in this section names it for, and #135 is one. On its +head the job ran and the fetch answered: + +``` +gh api repos/Flowfin/lab/commits/d18d040/check-runs?per_page=100 \ + --jq '.check_runs[] | select(.name=="required contexts") + | "\(.name): \(.conclusion)"' +required contexts: success + +gh run view 31929377870 --log \ + | sed -n 's/.*\(the ruleset on main requires .*\)/\1/p' +the ruleset on main requires 0 context(s): +``` + +A read-only token reads this board's ruleset, so that is not what would stop a +run from a fork. What is left is the other thing, which is that the token is +issued against a different repository. `github.repository` names this board on +a pull request from a fork, so the fetch asks about this board whichever side +the branch sits on, and whether a token issued that way may read this board's +ruleset is platform behaviour that nothing in this tree states and nothing +above measures. + +`required contexts` therefore belongs in the fork clause of #62, and it is +there for a different reason from the two names already in it. Those two are +created by an upload and turn on a write scope. This one is a job that always +starts, and what is open is whether the answer it needs arrives. + +### What this section does not settle + +No pull request from a fork has been opened here. The condition above has two +arms and only the second was walked: the branch measured above is in this +repository, and a read-only token is what the two arms have in common rather +than what makes them one route. `CodeQL` did arrive on that pull request, so +nothing here says whether it arrives from a fork, and the fork clause of #62 is +open for it. It is open for `required contexts` as well, for the reason the +subsection above gives rather than for this one. + +The required set is empty, so nothing above is a report of a required context +that failed to arrive. Every sentence here is about which names a set could +hold, and none of them is about a gate that bit. + ## The rest of the ruleset A required set is one rule out of the four that stand behind a merge at the