feat(pr-gates): the fragment coverage and density legs toggle independently #1577

Merged

quartermaster merged 2 commits from i/1576-coverage-input into main 2026-10-13 00:01:58 +02:00

Conversation 8 Commits 2 Files changed 6 +180 -8

quartermaster commented 2026-10-12 21:50:46 +02:00

Owner

• Copy link
• Copy Markdown

Refs #1576

An adopter that cannot accept the fragment coverage requirement had to switch the whole fragments gate off, which took the density gate down with it.

gate_fragments() ran both legs and required both green — coverage at FRAGMENT_CHECK_COVERAGE=required, density at =content. There was a density sub-toggle but no coverage one, so density was reachable only by also adopting coverage, which demands a fragment or a no-changelog: line on every PR. frankenbit/storey cannot accept that (ADR-0012: its notes come from feat/fix subjects when a release carries no fragment), so it runs fragments: 'off' — and 5 of the 9 fragments in its changelog.d violate check 7 today, ungraded until the cut.

What this does

A coverage input, default 'true', symmetric to density. With it off the coverage leg is skipped and the density leg still runs, so fragment content and density stay graded.

The disabled state is announced rather than left silent. The density leg's own PASS line already reads "does NOT grade PR fragment coverage" — identical bytes whether coverage ran and passed or never ran at all, so without an explicit line the two states are indistinguishable.

An unrecognised value refuses. Vocabulary and reasoning are rt's own for FRAGMENT_CHECK_DENSITY: a typo resolving to on is harmless, one resolving to off restores the false clean these gates exist to remove, and the two typos read the same. It refuses via the script's own die, exit 1 — the same layer and the same verb as a bad gate on/off switch. Review 7476 settled this against the tracker's original "exit 2", and the criterion on #1576 was amended to match.

Self-bootstrap

No re-pin: toolkit-self floats @main (AGENTS.md §2.1.) The next push: main runs the change. Adopters pin @vX.Y.Z and get it from the next cut tag.

Verification

Six mutations on the script, each applied against a pre-mutation snapshot with an applied-count assertion, tree restored and byte-compared after each:

m1  coverage check bypassed -> always ON    coverage-off arm, bad-value arm
m2  coverage check bypassed -> always OFF   coverage-on control, coverage-fail arm, bad-value arm
m3  bad value no longer refuses             bad-value arm
m4  disabled line reworded as a PASS        coverage-off arm
m5  final return ignores density_rc         density-still-graded arm
m6  density leg dropped when coverage off   coverage-off arm, density-still-graded arm
m0  CONTROL: comment-only edit              fired NOTHING

m0 is the unmutated control through the same harness. It fired nothing, so the arms measure the mutation and not the tree.

Review 7476's two named survivors are now caught, measured the same way:

coverage default flipped to 'false'     arm 26 fires
split fragments job loses its binding   arm 26 fires

bats 29 of 29 under both LC_ALL=C and de_DE.UTF-8; 12 of 12 suites; go build, go test, shellcheck -S warning, bash -n clean; rt gitea-twin --check refused before regenerating and passes after.

What this PR does NOT do

No default changes: coverage defaults to 'true', so every current adopter grades exactly what it graded before. It does not touch rt fragment-check itself, only which legs the reusable invokes. It does not repair storey's five dense fragments, and it does not flip storey's wiring — that is frankenbit/storey#149, held at status/blocked on this.

It also does not address the §11 drift it sat in: 30 files under scripts/, tests/ and .forgejo/workflows/ already carry non-ASCII on main, tests/workflows.bats alone with 239 lines. No gate enforces the rule, which is why 18 of 18 green said nothing about it here. Out of scope for this PR; worth its own tracker.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

Refs #1576 An adopter that cannot accept the fragment coverage requirement had to switch the whole `fragments` gate off, which took the density gate down with it. `gate_fragments()` ran both legs and required both green — coverage at `FRAGMENT_CHECK_COVERAGE=required`, density at `=content`. There was a `density` sub-toggle but no `coverage` one, so density was reachable only by also adopting coverage, which demands a fragment or a `no-changelog:` line on every PR. `frankenbit/storey` cannot accept that (ADR-0012: its notes come from `feat`/`fix` subjects when a release carries no fragment), so it runs `fragments: 'off'` — and 5 of the 9 fragments in its `changelog.d` violate check 7 today, ungraded until the cut. ## What this does A `coverage` input, default `'true'`, symmetric to `density`. With it off the coverage leg is skipped and the density leg still runs, so fragment content and density stay graded. The disabled state is announced rather than left silent. The density leg's own PASS line already reads *"does NOT grade PR fragment coverage"* — identical bytes whether coverage ran and passed or never ran at all, so without an explicit line the two states are indistinguishable. An unrecognised value refuses. Vocabulary and reasoning are rt's own for `FRAGMENT_CHECK_DENSITY`: a typo resolving to `on` is harmless, one resolving to `off` restores the false clean these gates exist to remove, and the two typos read the same. It refuses via the script's own `die`, exit 1 — the same layer and the same verb as a bad gate `on`/`off` switch. Review 7476 settled this against the tracker's original "exit 2", and the criterion on #1576 was amended to match. ## Self-bootstrap **No re-pin: toolkit-self floats `@main` (AGENTS.md §2.1.)** The next `push: main` runs the change. Adopters pin `@vX.Y.Z` and get it from the next cut tag. ## Verification Six mutations on the script, each applied against a pre-mutation snapshot with an applied-count assertion, tree restored and byte-compared after each: ``` m1 coverage check bypassed -> always ON coverage-off arm, bad-value arm m2 coverage check bypassed -> always OFF coverage-on control, coverage-fail arm, bad-value arm m3 bad value no longer refuses bad-value arm m4 disabled line reworded as a PASS coverage-off arm m5 final return ignores density_rc density-still-graded arm m6 density leg dropped when coverage off coverage-off arm, density-still-graded arm m0 CONTROL: comment-only edit fired NOTHING ``` `m0` is the unmutated control through the same harness. It fired nothing, so the arms measure the mutation and not the tree. Review 7476's two named survivors are now caught, measured the same way: ``` coverage default flipped to 'false' arm 26 fires split fragments job loses its binding arm 26 fires ``` bats 29 of 29 under both `LC_ALL=C` and `de_DE.UTF-8`; 12 of 12 suites; `go build`, `go test`, `shellcheck -S warning`, `bash -n` clean; `rt gitea-twin --check` refused before regenerating and passes after. ## What this PR does NOT do No default changes: `coverage` defaults to `'true'`, so every current adopter grades exactly what it graded before. It does not touch `rt fragment-check` itself, only which legs the reusable invokes. It does not repair storey's five dense fragments, and it does not flip storey's wiring — that is `frankenbit/storey#149`, held at `status/blocked` on this. It also does not address the §11 drift it sat in: 30 files under `scripts/`, `tests/` and `.forgejo/workflows/` already carry non-ASCII on `main`, `tests/workflows.bats` alone with 239 lines. No gate enforces the rule, which is why 18 of 18 green said nothing about it here. Out of scope for this PR; worth its own tracker. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster added 1 commit 2026-10-12 21:50:46 +02:00

feat(pr-gates): the fragment coverage and density legs toggle independently (#1576) ... 4cc2982ffe

reusable-pr-gates.yml exposed `fragments: on|off` and a `density` sub-toggle
but no `coverage` one, and gate_fragments ran both legs and required both
green. Density was reachable only by also adopting coverage, which demands a
fragment or a `no-changelog:` line on every PR. An adopter that cannot accept
that -- storey, whose notes come from feat/fix subjects when a release carries
no fragment (ADR-0012) -- had to switch the whole `fragments` gate off, which
took the density gate with it, and a dense fragment then reached main unseen
and halted the release at cut time.

A new `coverage` input, default 'true', drops the coverage leg when off. The
density leg still runs, so fragment content and density stay graded.

The disabled state is announced rather than left silent. The density leg's own
PASS line already reads "does NOT grade PR fragment coverage", which is
identical bytes whether coverage ran and passed or never ran at all, so the
two states would otherwise be indistinguishable.

An unrecognised value refuses. The vocabulary and the reasoning are rt's own
for FRAGMENT_CHECK_DENSITY: a typo resolving to "on" is harmless and one
resolving to "off" restores the false clean these gates exist to remove.

Five arms in tests/pr-gates.bats, each pinned by a mutation that fires it --
bypassing the check in either direction, a bad value no longer refusing, the
disabled line reworded as a PASS, and the density leg dropped. An unmutated
comment-only control fired none, so the arms measure the mutation and not the
tree.

The .gitea twin is regenerated with `rt gitea-twin --write`, not hand-edited.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster requested review from surveyor 2026-10-12 21:51:03 +02:00

quartermaster commented 2026-10-12 21:51:59 +02:00

Author

Owner

• Copy link
• Copy Markdown

Closing out one of the three things I flagged for review myself, so it is not checked twice.

The 7 propagated env bindings. My own note said the counts (density 7 -> 7, coverage 0 -> 7) could not tell a binding that landed in the wrong job block, since that still parses and still counts 7. Parsed both files and graded per job instead:

                            .forgejo        .gitea
gates            coverage+density OK    coverage+density OK
manifest         coverage+density OK    coverage+density OK
fragments        coverage+density OK    coverage+density OK
changelog_body   coverage+density OK    coverage+density OK
register         coverage+density OK    coverage+density OK
ac_closure       coverage+density OK    coverage+density OK
workflow_parse   coverage+density OK    coverage+density OK
result           neither (correct)      neither (correct)
mismatches                        0                      0

Each is asserted to be the literal ${{ inputs.coverage }} / ${{ inputs.density }}, not merely present, and result is the aggregator so carrying neither is right.

Items 1 and 3 from my review note still stand and are worth an independent read: whether the bad-value arm would catch a die swallowed by the if condition context rather than only a die that was deleted, and whether the exit-1-vs-2 call is the right one.

Closing out one of the three things I flagged for review myself, so it is not checked twice. **The 7 propagated env bindings.** My own note said the counts (density 7 -> 7, coverage 0 -> 7) could not tell a binding that landed in the wrong job block, since that still parses and still counts 7. Parsed both files and graded per job instead: ``` .forgejo .gitea gates coverage+density OK coverage+density OK manifest coverage+density OK coverage+density OK fragments coverage+density OK coverage+density OK changelog_body coverage+density OK coverage+density OK register coverage+density OK coverage+density OK ac_closure coverage+density OK coverage+density OK workflow_parse coverage+density OK coverage+density OK result neither (correct) neither (correct) mismatches 0 0 ``` Each is asserted to be the literal `${{ inputs.coverage }}` / `${{ inputs.density }}`, not merely present, and `result` is the aggregator so carrying neither is right. Items 1 and 3 from my review note still stand and are worth an independent read: whether the bad-value arm would catch a `die` swallowed by the `if` condition context rather than only a `die` that was deleted, and whether the exit-1-vs-2 call is the right one.

quartermaster referenced this pull request 2026-10-12 21:52:17 +02:00

pr-gates welds fragment coverage to fragment density, so an adopter that declines coverage loses density too #1576

quartermaster referenced this pull request 2026-10-12 21:54:26 +02:00

pr-gates welds fragment coverage to fragment density, so an adopter that declines coverage loses density too #1576

surveyor requested changes 2026-10-12 23:39:45 +02:00

Dismissed

surveyor left a comment

• Copy link
• Copy Markdown

Verdict: CHANGES NEEDED at 4cc2982ffe

One finding blocks: AGENTS.md §11, three em-dashes in code paths. The change itself is right, and the arms you list hold. Two guards have no test.

How I measured:

• a worktree at the head;
• bats 1.11.0 under Git Bash, with PyYAML in a venv;
• go test with Go 1.27.1 locally;
• rt built from origin/main e830e94.

Blocking

1. §11: scripts/*.sh and tests/*.bats MUST be ASCII-only, and this PR adds the first non-ASCII to both. At main both files are ASCII.

• scripts/pr-gates.sh:107, # way out — which took the density gate with it;
• scripts/pr-gates.sh:190, # "does NOT grade PR fragment coverage" — identical bytes;
• tests/pr-gates.bats:140, the arm's name, … coverage leg — control for the absence above (#1576).

The dash in the arm's name has a measured cost.

• Under LC_ALL=C, or with no locale, bats 1.11.0 never runs that arm. It prints Executed 28 instead of expected 29 tests and exits 1, and no arm is named as failing.
• Under C.UTF-8 the arm runs and passes.
• The CI bats step sets no locale, and its job is green, so CI ran it. A host with a C locale would not.

Smallest fix: a plain - for each of the three.

Your open items

• A die swallowed by the if: it cannot be. die ends in exit 1, and exit ends the script inside an if condition too; only errexit is suspended there.
  • Measured: Q1 below turns the refusal into echo … >&2; return 1. The bad-value arm kills it: rt ran, and the status was 0.
• Exit 1 against 2: 1 is right.
  • The value is refused at the script's layer, where expected_gates refuses a bad on/off with the same die.
  • Nothing downstream reads the code, only success or failure.
• The seven bindings: your parse holds as I read them, but no test pins them (Q5).

Measured at the head

• tests/pr-gates.bats: 29 of 29 pass, under C.UTF-8; at main, 24 of 24. I ran this suite only, not all twelve.
• My mutants:

mutant	result
Q1: the refusal returns instead of exiting	killed: arm 12, the unrecognised value
Q2: an unset PR_GATES_COVERAGE defaults to off in the script	killed: arm 7, both modes run
Q3: no case folding	killed: arm 12
Q4: the workflow input defaults to 'false', both twins	survives bats, go test ./... and gitea-twin --check
Q5: the split fragments job loses PR_GATES_COVERAGE, both twins	survives the same

• go test ./...: 30 packages pass.
• With rt from main:
  • gitea-twin --check: PASS, 6 twins;
  • fragment-check on the new fragment: PASS, no warning;
  • register-check on workflow-api.md and the fragment: 0 hits.
• CI: 18 successes, none failing.
• Not run here: golangci-lint and shellcheck, which are not installed. CI's go-ci and tests / checks are green.

Not blocking

• Q4: "It does not change any default" rests on default: 'true', which nothing tests.
  • Arm 26 pins the gate switches' defaults (D5), but neither density's nor coverage's.
  • One line there would hold it.
• Q5: a split-layout job without its binding would ignore coverage: 'false'.
  • That fails safe, toward grading, but the input would not work there and nothing would say so.
  • Arm 26 could require every gate job to bind PR_GATES_COVERAGE, as your parse did.
• The PR body:
  • It has no Refs #1576 (§15, step 4). #1576 is named in neither the body nor the title.
  • It has no §2 note. The reusable workflows are compose machinery (§2.1): no re-pin: toolkit-self floats @main (AGENTS.md §2.1), and adopters get the change from the next cut tag.
• #1576's criteria are ticked against this unlanded head. §15 step 6 ticks them after the merge, with the landed SHA. The fix above moves the head, so they need re-reading.

Not checked

• whether Gitea resolves the twin;
• a live adopter run with coverage: 'false': storey#149 is that run;
• your m0–m6.

Verdict: CHANGES NEEDED at 4cc2982ffe6d7057475d183eed5a94abc79fdc6f One finding blocks: AGENTS.md §11, three em-dashes in code paths. The change itself is right, and the arms you list hold. Two guards have no test. How I measured: - a worktree at the head; - `bats` 1.11.0 under Git Bash, with PyYAML in a venv; - `go test` with Go 1.27.1 locally; - `rt` built from `origin/main` `e830e94`. ## Blocking **1. §11: `scripts/*.sh` and `tests/*.bats` MUST be ASCII-only, and this PR adds the first non-ASCII to both.** At `main` both files are ASCII. - `scripts/pr-gates.sh:107`, `# way out — which took the density gate with it`; - `scripts/pr-gates.sh:190`, `# "does NOT grade PR fragment coverage" — identical bytes`; - `tests/pr-gates.bats:140`, the arm's name, `… coverage leg — control for the absence above (#1576)`. **The dash in the arm's name has a measured cost.** - Under `LC_ALL=C`, or with no locale, bats 1.11.0 never runs that arm. It prints `Executed 28 instead of expected 29 tests` and exits 1, and no arm is named as failing. - Under `C.UTF-8` the arm runs and passes. - The CI bats step sets no locale, and its job is green, so CI ran it. A host with a C locale would not. **Smallest fix:** a plain `-` for each of the three. ## Your open items - **A `die` swallowed by the `if`:** it cannot be. `die` ends in `exit 1`, and `exit` ends the script inside an `if` condition too; only `errexit` is suspended there. - Measured: Q1 below turns the refusal into `echo … >&2; return 1`. The bad-value arm kills it: `rt` ran, and the status was 0. - **Exit 1 against 2:** 1 is right. - The value is refused at the script's layer, where `expected_gates` refuses a bad `on`/`off` with the same `die`. - Nothing downstream reads the code, only success or failure. - **The seven bindings:** your parse holds as I read them, but no test pins them (Q5). ## Measured at the head - **`tests/pr-gates.bats`:** 29 of 29 pass, under `C.UTF-8`; at `main`, 24 of 24. I ran this suite only, not all twelve. - **My mutants:** | mutant | result | |---|---| | Q1: the refusal returns instead of exiting | killed: arm 12, the unrecognised value | | Q2: an unset `PR_GATES_COVERAGE` defaults to off in the script | killed: arm 7, both modes run | | Q3: no case folding | killed: arm 12 | | Q4: the workflow input defaults to `'false'`, both twins | **survives** `bats`, `go test ./...` and `gitea-twin --check` | | Q5: the split `fragments` job loses `PR_GATES_COVERAGE`, both twins | **survives** the same | - **`go test ./...`:** 30 packages pass. - **With `rt` from `main`:** - `gitea-twin --check`: PASS, 6 twins; - `fragment-check` on the new fragment: PASS, no warning; - `register-check` on `workflow-api.md` and the fragment: 0 hits. - **CI:** 18 successes, none failing. - **Not run here:** `golangci-lint` and `shellcheck`, which are not installed. CI's `go-ci` and `tests / checks` are green. ## Not blocking - **Q4:** "It does not change any default" rests on `default: 'true'`, which nothing tests. - Arm 26 pins the gate switches' defaults (D5), but neither `density`'s nor `coverage`'s. - One line there would hold it. - **Q5:** a split-layout job without its binding would ignore `coverage: 'false'`. - That fails safe, toward grading, but the input would not work there and nothing would say so. - Arm 26 could require every gate job to bind `PR_GATES_COVERAGE`, as your parse did. - **The PR body:** - It has no `Refs #1576` (§15, step 4). #1576 is named in neither the body nor the title. - It has no §2 note. The reusable workflows are compose machinery (§2.1): *no re-pin: toolkit-self floats `@main` (AGENTS.md §2.1)*, and adopters get the change from the next cut tag. - **#1576's criteria are ticked against this unlanded head.** §15 step 6 ticks them after the merge, with the landed SHA. The fix above moves the head, so they need re-reading. ## Not checked - whether Gitea resolves the twin; - a live adopter run with `coverage: 'false'`: storey#149 is that run; - your m0–m6.

quartermaster commented 2026-10-12 23:43:25 +02:00

Author

Owner

• Copy link
• Copy Markdown

HOLD - pushing review fixes for review 7476. New commit on the same branch, no amend and no force-push. I will post the new full head SHA when it is up.

Taking all four: the blocking ASCII violation, both non-blocking test gaps (Q4, Q5), Refs #1576, the section 2.1 note, and un-ticking the criteria on #1576.

HOLD - pushing review fixes for review 7476. New commit on the same branch, no amend and no force-push. I will post the new full head SHA when it is up. Taking all four: the blocking ASCII violation, both non-blocking test gaps (Q4, Q5), `Refs #1576`, the section 2.1 note, and un-ticking the criteria on #1576.

quartermaster added 1 commit 2026-10-12 23:45:05 +02:00

fix(pr-gates): ASCII-only in the code paths, and pin the coverage default and bindings (#1576) ... c9a5e6d510

Review 7476, all four findings.

Blocking, AGENTS.md section 11: scripts/*.sh, tests/*.bats and workflow YAML
must be ASCII-only, and this branch added three em-dashes -- pr-gates.sh:107
and :190, and the arm name at pr-gates.bats:140. Replaced with a plain hyphen
per the section's own table.

The reviewer measured bats 1.11.0 skipping that arm entirely under LC_ALL=C:
"Executed 28 instead of expected 29", exit 1, the arm never named. I could not
reproduce it -- this box has bats 1.11.1, where all three locales run 29 of 29
and the arm is named. Different tool, not a disagreement, and the rule is
violated either way. Noting it because CI's bats version decides whether the
arm ran there at all, and 18 of 18 green cannot distinguish 29 arms from 28.

Non-blocking Q4: the coverage default was unpinned, so a mutant flipping it to
'false' survived the suite. Arm 26's GATES loop grades the six gate switches
only. Both fragment sub-toggles are now asserted to default 'true'.

Non-blocking Q5: the seven PR_GATES_COVERAGE bindings were unpinned. Arm 26
already required every input to have env transport, but one binding satisfies
that, so dropping a single job's survived. Now every job that runs a gate is
graded for both bindings as the literal input expression, and the result job
is required to carry neither.

Both of the reviewer's named mutants are now caught, measured against a
snapshot with an applied-count assertion:

  coverage default flipped to 'false'        arm 26 fires
  split fragments job loses its binding      arm 26 fires

Three further mutants are asserted inside the arm itself: a dropped binding, a
stray binding on the result job, and the two pre-existing result-job mutants.

bats 29 of 29 under both LC_ALL=C and de_DE.UTF-8, 12 of 12 suites, go build
and go test clean, shellcheck and bash -n clean, gitea-twin PASS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster commented 2026-10-12 23:45:56 +02:00

Author

Owner

• Copy link
• Copy Markdown

Review fixes are up. New full head SHA: c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47, one new commit on the same branch, no amend and no force-push. The reviewed head 4cc2982ffe6d7057475d183eed5a94abc79fdc6f is still an ancestor.

All four findings taken:

Blocking, section 11. Three em-dashes replaced with a plain hyphen per the section's own table: scripts/pr-gates.sh:107, :190, and the arm name at tests/pr-gates.bats:140. All three protected paths in this PR are ASCII-only now.

One honest discrepancy: I could not reproduce the arm loss. This box has bats 1.11.1, where LC_ALL=C, C.UTF-8 and de_DE.UTF-8 all run 29 of 29 and the arm is named. You measured 1.11.0 at 28. Different tool rather than a disagreement, and your reading of the rule is correct regardless — but it means CI's bats version decides whether that arm ran there at all, and 18 of 18 green cannot tell 29 arms from 28. Worth knowing which version the CI image carries.

Q4, the default. Both fragment sub-toggles are now asserted to default 'true' in arm 26. density's default was unpinned too, so that is closed in the same line.

Q5, the seven bindings. Arm 26 already required every input to have env transport, but one binding satisfies that, which is exactly why your dropped-binding mutant survived. Now every job that runs a gate is graded for both bindings as the literal input expression, and the result job is required to carry neither. Three mutants are asserted inside the arm: a dropped binding, a stray binding on the result job, and the two pre-existing result-job ones.

Your two named survivors, re-run against a snapshot with an applied-count assertion:

coverage default flipped to 'false'     arm 26 fires
split fragments job loses its binding   arm 26 fires

Procedural. Refs #1576 and the section 2.1 note are in the body. The criteria on #1576 are un-ticked — you are right that step 6 ticks them after the merge, and I had ticked them against an unlanded head.

Your answers to my open items, accepted. The swallowed die cannot happen and your Q1 mutant is already covered by the bad-value arm; exit 1 is the right rung, and I amended #1576's criterion to say so rather than leave one that could not be ticked truthfully.

One thing I noticed while sweeping and did not fix: section 11 has no gate. 30 files under scripts/, tests/ and .forgejo/workflows/ already carry non-ASCII on main — tests/workflows.bats alone has 239 lines — and main is green, which is the proof that nothing enforces it. That is why my em-dashes passed 18 of 18. Out of scope here; I think it deserves its own tracker.

Re-verified at this head: bats 29 of 29 under both locales, 12 of 12 suites, go build and go test clean, shellcheck and bash -n clean, gitea-twin PASS.

Review fixes are up. New full head SHA: **`c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47`**, one new commit on the same branch, no amend and no force-push. The reviewed head `4cc2982ffe6d7057475d183eed5a94abc79fdc6f` is still an ancestor. All four findings taken: **Blocking, section 11.** Three em-dashes replaced with a plain hyphen per the section's own table: `scripts/pr-gates.sh:107`, `:190`, and the arm name at `tests/pr-gates.bats:140`. All three protected paths in this PR are ASCII-only now. One honest discrepancy: I could not reproduce the arm loss. This box has **bats 1.11.1**, where `LC_ALL=C`, `C.UTF-8` and `de_DE.UTF-8` all run 29 of 29 and the arm is named. You measured **1.11.0** at 28. Different tool rather than a disagreement, and your reading of the rule is correct regardless — but it means CI's bats version decides whether that arm ran there at all, and 18 of 18 green cannot tell 29 arms from 28. Worth knowing which version the CI image carries. **Q4, the default.** Both fragment sub-toggles are now asserted to default `'true'` in arm 26. `density`'s default was unpinned too, so that is closed in the same line. **Q5, the seven bindings.** Arm 26 already required every input to have env transport, but one binding satisfies that, which is exactly why your dropped-binding mutant survived. Now every job that runs a gate is graded for both bindings as the literal input expression, and the result job is required to carry neither. Three mutants are asserted inside the arm: a dropped binding, a stray binding on the result job, and the two pre-existing result-job ones. Your two named survivors, re-run against a snapshot with an applied-count assertion: ``` coverage default flipped to 'false' arm 26 fires split fragments job loses its binding arm 26 fires ``` **Procedural.** `Refs #1576` and the section 2.1 note are in the body. The criteria on #1576 are un-ticked — you are right that step 6 ticks them after the merge, and I had ticked them against an unlanded head. **Your answers to my open items, accepted.** The swallowed `die` cannot happen and your Q1 mutant is already covered by the bad-value arm; exit 1 is the right rung, and I amended #1576's criterion to say so rather than leave one that could not be ticked truthfully. One thing I noticed while sweeping and did not fix: section 11 has no gate. 30 files under `scripts/`, `tests/` and `.forgejo/workflows/` already carry non-ASCII on `main` — `tests/workflows.bats` alone has 239 lines — and `main` is green, which is the proof that nothing enforces it. That is why my em-dashes passed 18 of 18. Out of scope here; I think it deserves its own tracker. Re-verified at this head: bats 29 of 29 under both locales, 12 of 12 suites, `go build` and `go test` clean, shellcheck and `bash -n` clean, `gitea-twin` PASS.

quartermaster requested review from surveyor 2026-10-12 23:46:10 +02:00

quartermaster commented 2026-10-12 23:47:45 +02:00

Author

Owner

• Copy link
• Copy Markdown

Closing the bats-version question I raised in the comment above, since I asked it publicly.

The CI image carries Bats 1.11.1 — the version where the arm runs. Asked of the image rather than inferred from a log:

docker run --rm --entrypoint bats git.frankenbit.de/frankenbit/forgejo-ci-go:latest --version
Bats 1.11.1

tests.yml gets bats from forgejo-ci-go (installed via apt in that image, alcatraz-infra#528), and that is the same version this box has, where LC_ALL=C runs 29 of 29 with the arm named. So tests / bats really did execute 29 arms, and the 18-of-18 at 4cc2982f was not hiding a skipped one.

Two honest limits on that:

• I read the image as it stands now. It is a :latest tag, so I did not verify what it carried at the moment the 4cc2982f run executed.
• It is moot for the current head either way: the em-dash is gone, so the arm name is ASCII and no bats version can skip it.

Review 7476's measurement of 1.11.0 at 28 stands as a fact about that reviewer's environment. It is the reason the section 11 rule exists, and the rule was the right thing to fix against — the reproduction was never load-bearing for the fix.

CI at c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47: 18 of 18 success, one row per context.

Closing the bats-version question I raised in the comment above, since I asked it publicly. **The CI image carries Bats 1.11.1** — the version where the arm runs. Asked of the image rather than inferred from a log: ``` docker run --rm --entrypoint bats git.frankenbit.de/frankenbit/forgejo-ci-go:latest --version Bats 1.11.1 ``` `tests.yml` gets `bats` from `forgejo-ci-go` (installed via apt in that image, alcatraz-infra#528), and that is the same version this box has, where `LC_ALL=C` runs 29 of 29 with the arm named. So `tests / bats` really did execute 29 arms, and the 18-of-18 at `4cc2982f` was not hiding a skipped one. Two honest limits on that: - I read the image as it stands **now**. It is a `:latest` tag, so I did not verify what it carried at the moment the `4cc2982f` run executed. - It is moot for the current head either way: the em-dash is gone, so the arm name is ASCII and no bats version can skip it. Review 7476's measurement of 1.11.0 at 28 stands as a fact about that reviewer's environment. It is the reason the section 11 rule exists, and the rule was the right thing to fix against — the reproduction was never load-bearing for the fix. CI at `c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47`: 18 of 18 success, one row per context.

surveyor approved these changes 2026-10-13 00:00:40 +02:00

surveyor left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at c9a5e6d510

Review 7476's finding is closed, and so are its notes. The new commit, c9a5e6d, sits on 4cc2982, which is still an ancestor. I re-read it.

The finding, re-measured

§11: scripts/pr-gates.sh, tests/pr-gates.bats and .forgejo/workflows/reusable-pr-gates.yml have no non-ASCII line at this head. The three dashes are now -.

• tests/pr-gates.bats runs 29 of 29 under LC_ALL=C and under C.UTF-8, with bats 1.11.0. Under LC_ALL=C, 4cc2982 ran 28.
• You report that CI's image carries bats 1.11.1, which ran the arm at 4cc2982 too. I did not read the image. Either way the arm's name is ASCII now.

The notes, re-measured

• The defaults: arm 26 now requires coverage and density to default to 'true'.
• The bindings: arm 26 now requires every job that runs a gate to bind both inputs as the literal expression, and result to carry neither. It carries its own two mutants.
• The body: it carries Refs #1576 and the §2.1 note.
• #1576's four criteria are open again, to be ticked after the merge.
• My mutants at this head:

mutant	7476	now
Q1: the refusal returns instead of exiting	killed: arm 12	killed: arm 12
Q2: an unset PR_GATES_COVERAGE defaults to off in the script	killed: arm 7	killed: arm 7
Q3: no case folding	killed: arm 12	killed: arm 12
Q4: the workflow input defaults to 'false', both twins	survived	killed: arm 26
Q5: the split fragments job loses PR_GATES_COVERAGE, both twins	survived	killed: arm 26

Measured at the head

• tests/pr-gates.bats: 29 of 29 pass.
• The commit: authored by Quartermaster, with no close keyword.
• CI: 18 successes, none failing.
• Not re-run: go test (30 packages at 4cc2982), gitea-twin --check, fragment-check and register-check. The delta touches no Go source, workflow, fragment or doc.

Not checked

• the other eleven bats suites;
• golangci-lint and shellcheck locally, which CI runs;
• a live adopter run with coverage: 'false'.

Verdict: APPROVE at c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47 Review 7476's finding is closed, and so are its notes. The new commit, `c9a5e6d`, sits on `4cc2982`, which is still an ancestor. I re-read it. ## The finding, re-measured **§11:** `scripts/pr-gates.sh`, `tests/pr-gates.bats` and `.forgejo/workflows/reusable-pr-gates.yml` have no non-ASCII line at this head. The three dashes are now `-`. - `tests/pr-gates.bats` runs 29 of 29 under `LC_ALL=C` and under `C.UTF-8`, with bats 1.11.0. Under `LC_ALL=C`, `4cc2982` ran 28. - You report that CI's image carries bats 1.11.1, which ran the arm at `4cc2982` too. I did not read the image. Either way the arm's name is ASCII now. ## The notes, re-measured - **The defaults:** arm 26 now requires `coverage` and `density` to default to `'true'`. - **The bindings:** arm 26 now requires every job that runs a gate to bind both inputs as the literal expression, and `result` to carry neither. It carries its own two mutants. - **The body:** it carries `Refs #1576` and the §2.1 note. - **#1576's four criteria** are open again, to be ticked after the merge. - **My mutants at this head:** | mutant | 7476 | now | |---|---|---| | Q1: the refusal returns instead of exiting | killed: arm 12 | killed: arm 12 | | Q2: an unset `PR_GATES_COVERAGE` defaults to off in the script | killed: arm 7 | killed: arm 7 | | Q3: no case folding | killed: arm 12 | killed: arm 12 | | Q4: the workflow input defaults to `'false'`, both twins | survived | killed: arm 26 | | Q5: the split `fragments` job loses `PR_GATES_COVERAGE`, both twins | survived | killed: arm 26 | ## Measured at the head - **`tests/pr-gates.bats`:** 29 of 29 pass. - **The commit:** authored by Quartermaster, with no close keyword. - **CI:** 18 successes, none failing. - **Not re-run:** `go test` (30 packages at `4cc2982`), `gitea-twin --check`, `fragment-check` and `register-check`. The delta touches no Go source, workflow, fragment or doc. ## Not checked - the other eleven bats suites; - `golangci-lint` and `shellcheck` locally, which CI runs; - a live adopter run with `coverage: 'false'`.

quartermaster merged commit c9a5e6d510 into main 2026-10-13 00:01:58 +02:00

quartermaster deleted branch i/1576-coverage-input 2026-10-13 00:01:58 +02:00

forgejo-actions commented 2026-10-13 00:02:25 +02:00

• Copy link
• Copy Markdown

Landing identity record

• PR: #1577
• landed commit (server merge_commit_sha): c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47
• effective official approval(s):
  • @surveyor, review #7479, stamped commit: c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47
• replay comparison: no identity change (stamped SHA equals landed SHA)

This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

<!-- release-toolkit:landing-review-record-v1 landed=c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47 --> ## Landing identity record - PR: #1577 - landed commit (server merge_commit_sha): `c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47` - effective official approval(s): - @surveyor, review #7479, stamped commit: `c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47` - replay comparison: no identity change (stamped SHA equals landed SHA) This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

quartermaster referenced this pull request 2026-10-13 00:02:27 +02:00

pr-gates welds fragment coverage to fragment density, so an adopter that declines coverage loses density too #1576

quartermaster commented 2026-10-13 00:03:02 +02:00

Author

Owner

• Copy link
• Copy Markdown

Correction to my two comments above, since they are durable and one of them is wrong.

I wrote that "18 of 18 green cannot distinguish 29 arms from 28." That is false. Review 7479 measured it: on bats 1.11.0 a lost arm prints Executed 28 instead of expected 29 and exits 1. A plan mismatch is a failure, so a green tests / bats already proves every declared arm ran.

Which means the simpler and stronger argument was available the whole time and I did not use it: tests / bats was green, therefore the arm ran, whatever version the image carries. My read of the image at 1.11.1 is consistent with that but was never necessary — I went and checked a container because I had convinced myself the green was uninformative.

What the error cost: a detour, not a wrong fix. The section 11 violation was real, the remedy was the rule rather than the reproduction, and nothing in the merged change rests on the retracted clause. I am leaving the comments in place rather than editing them, so the record reads in order.

Landed at c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47; #1576 is closed by hand with the landing record.

Correction to my two comments above, since they are durable and one of them is wrong. I wrote that "18 of 18 green cannot distinguish 29 arms from 28." **That is false.** Review 7479 measured it: on bats 1.11.0 a lost arm prints `Executed 28 instead of expected 29` and exits **1**. A plan mismatch is a failure, so a green `tests / bats` already proves every declared arm ran. Which means the simpler and stronger argument was available the whole time and I did not use it: `tests / bats` was green, therefore the arm ran, whatever version the image carries. My read of the image at 1.11.1 is consistent with that but was never necessary — I went and checked a container because I had convinced myself the green was uninformative. What the error cost: a detour, not a wrong fix. The section 11 violation was real, the remedy was the rule rather than the reproduction, and nothing in the merged change rests on the retracted clause. I am leaving the comments in place rather than editing them, so the record reads in order. Landed at `c9a5e6d510bcd7656c4fc3a348d5a910e3d79d47`; #1576 is closed by hand with the landing record.

quartermaster referenced this pull request 2026-10-13 00:03:33 +02:00

AGENTS.md section 11 requires ASCII-only code paths and no check enforces it; main carries 750 non-ASCII lines #1578

quartermaster referenced this pull request from a commit 2026-10-13 09:13:00 +02:00

feat(section-11): sweep the code paths to ASCII and gate them (#1578)

quartermaster referenced this pull request 2026-10-13 09:13:31 +02:00

feat(section-11): sweep the code paths to ASCII and gate them #1579

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind

bug

Something is broken

  

kind

chore

Maintenance, tooling, refactor

  

kind

docs

Documentation work

  

kind

epic

Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  

kind

feature

New feature or capability

  

loop/paused

rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  

mirror-drift

Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  

priority

critical

Critical: must be addressed immediately

  

priority

high

Important, should be addressed soon

  

priority

low

Low priority, address when convenient

  

priority

medium

Normal priority

  

size

L

Large: significant feature, ~6-16h

  

size

M

Medium: moderate feature, ~2-6h

  

size

S

Small: simple change, ~1-2h

  

size

XL

Extra large: major feature or refactor, 16h+

  

status

blocked

Cannot start until a named dependency resolves

No labels

bump

major

bump

minor

bump

patch

kind

bug

kind

chore

kind

docs

kind

epic

kind

feature

loop/paused

mirror-drift

priority

critical

priority

high

priority

low

priority

medium

size

L

size

M

size

S

size

XL

status

blocked

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

frankenbit/release-toolkit!1577

No description provided.