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