Ci9 gateproof #102

Merged
admin merged 3 commits from ci9-gateproof into release/1.0 2026-09-24 14:37:16 +02:00
Owner
No description provided.
admin changed target branch from master to release/1.0 2026-09-19 16:54:24 +02:00
admin added 1 commit 2026-09-19 16:54:24 +02:00
The residual-risk paragraph at the top of checks.yml claims the `guard the
trigger` step "keeps it that way mechanically rather than by asking the next
editor to read this comment". For blind spots 1 and 3 that is not true: the
guard lives inside the workflow a filter makes Gitea skip, so for those cases
the defence IS the comment.

That paragraph is the preamble someone reads at the exact moment they are
editing the trigger, so a summary running ahead of its detail is worse here
than elsewhere. It now points at the blind spots and states spot 3 plainly:
a filter naming a branch nothing targets is itself skipped, lands green and
untested, and then skips every subsequent pull request - the gate off for
everything, silently.

Flagged by the reviewer on PR #95 (issuecomment-1424) as non-blocking.

This PR is also the first real test of CI-9's fix on release/1.0 itself. Run
806 was a workflow_dispatch and posts no commit status at all, so it verified
content but never the gate. This one has to post `checks / unit
(pull_request)` as success rather than skipped.

Verified before push: the guard passes against this edited file (whose new
prose quotes `branches: [main]` and `master`) and still refuses a genuine
filter added to the `on:` block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
admin closed this pull request 2026-09-19 17:14:28 +02:00
admin reopened this pull request 2026-09-19 17:14:31 +02:00
admin closed this pull request 2026-09-23 20:08:12 +02:00
admin reopened this pull request 2026-09-23 20:08:15 +02:00
Author
Owner

Review 1/5 — VERIFIED, no defect: the new prose does not blind the guard

This was the one way these 12 lines could be worse than useless, so I re-derived it from the branch's bytes rather than trusting the PR description.

Method. Extracted the awk program from the guard the trigger step verbatim into a standalone file, then ran it against git show origin/ci9-gateproof:.gitea/workflows/checks.yml and against origin/release/1.0's copy.

  • base file → no output (guard passes)
  • head file, with the new clause quoting `branches: [main]` and `master` → no output (guard passes)

Negative controls, on the head file — i.e. with the new prose present. Injected a real filter directly under pull_request: inside the on: block, four variants, one at a time:

branches: [main]        -> GUARD BITES  "140:     branches: [main]"
paths: / - 'goodwe_controller/**'
                        -> GUARD BITES  "140:     paths:"
branches-ignore: [foo]  -> GUARD BITES  "140:     branches-ignore: [foo]"
types: [opened]         -> GUARD BITES  "140:     types: [opened]"

Why it holds, structurally. /^[[:space:]]*#/ { next } sits ahead of the on: rule in program order, so a full-line comment is discarded before anything can set inon. The new block is 12 full-line comments at column 0, all of them above on: (lines 113–124; on: is line 138), so it is doubly out of reach.

Also confirmed mechanically, not by reading (scope):

  • added lines that are neither a comment nor blank: 0
  • removed lines: 0
  • files touched: .gitea/workflows/checks.yml only
  • non-comment lines of the file, base vs head: identical, 139 == 139
  • yaml.safe_load of the head file → on: {pull_request: None, workflow_dispatch: None} — no filter
  • checks.yml in the merged tree (git merge-tree of origin/release/1.0 + origin/ci9-gateproof) is byte-identical to the head's: sha256 ead9ed32…84484a both sides

And the live run agrees: run 821 job 1946, the genuine PR-102 run against base 239e120, prints OK: the on: block of the merged checks.yml carries no event filter.

Nothing to fix here. The rest of this review is about the content of the sentence, not its safety.

## Review 1/5 — VERIFIED, no defect: the new prose does **not** blind the guard This was the one way these 12 lines could be worse than useless, so I re-derived it from the branch's bytes rather than trusting the PR description. **Method.** Extracted the `awk` program from the `guard the trigger` step verbatim into a standalone file, then ran it against `git show origin/ci9-gateproof:.gitea/workflows/checks.yml` and against `origin/release/1.0`'s copy. * base file → no output (guard passes) * head file, with the new clause quoting `` `branches: [main]` `` and `` `master` `` → no output (guard passes) **Negative controls, on the head file — i.e. with the new prose present.** Injected a real filter directly under `pull_request:` inside the `on:` block, four variants, one at a time: ``` branches: [main] -> GUARD BITES "140: branches: [main]" paths: / - 'goodwe_controller/**' -> GUARD BITES "140: paths:" branches-ignore: [foo] -> GUARD BITES "140: branches-ignore: [foo]" types: [opened] -> GUARD BITES "140: types: [opened]" ``` **Why it holds, structurally.** `/^[[:space:]]*#/ { next }` sits *ahead of* the `on:` rule in program order, so a full-line comment is discarded before anything can set `inon`. The new block is 12 full-line comments at column 0, all of them above `on:` (lines 113–124; `on:` is line 138), so it is doubly out of reach. **Also confirmed mechanically, not by reading (scope):** * added lines that are neither a comment nor blank: **0** * removed lines: **0** * files touched: `.gitea/workflows/checks.yml` only * non-comment lines of the file, base vs head: **identical, 139 == 139** * `yaml.safe_load` of the head file → `on: {pull_request: None, workflow_dispatch: None}` — no filter * `checks.yml` in the *merged* tree (`git merge-tree` of `origin/release/1.0` + `origin/ci9-gateproof`) is byte-identical to the head's: sha256 `ead9ed32…84484a` both sides And the live run agrees: run 821 job 1946, the genuine PR-102 run against base `239e120`, prints `OK: the on: block of the merged checks.yml carries no event filter.` Nothing to fix here. The rest of this review is about the *content* of the sentence, not its safety.
Author
Owner

Review 2/5 — BLOCKING: "no pull request in 99 has ever targeted master" is false, and this pull request is the counterexample

The clause reads:

a filter naming a branch nothing targets (branches: [main] from a template - this repo has no main, and no pull request in 99 has ever targeted master)

this repo has no main is true — I listed the remote refs, there is no main. The master half is false, twice over.

(a) It was already false when it was written — by the very pull request that writes it

PR #102 was opened against master, at 2026-09-19T16:53:11+02:00. The CI run it triggered is still in the forge, and its own environment block says so:

run 808 / job 1932, 2026-09-19T14:53:17Z
  PRNUM: 102
  BASEREF: master
  base  master              5184a2cfc25cdf7437fff775d58fd677a31ada4e
  head  refs/pull/102/head  33e17dba0ee2160c0b711c5a47e4b091d34021a4

(5184a2c was master's tip on that date; PKG-15 did not bring release/1.0 to master until the following day.) It was retargeted to release/1.0 ~20 minutes later, which produced run 809 / job 1933 with BASEREF: release/1.0.

The number therefore comes from the /pulls census, which reports a pull request's current base — and this file already contains the paragraph explaining exactly why that census cannot support this claim:

⚠️ THAT 92 IS AN UPPER BOUND, NOT A CENSUS … The API reports a pull request's CURRENT base, so one that was opened stacked and later retargeted is recorded against its final base and is invisible here.

The clause reuses the census result without reapplying the caveat written 25 lines above it. That is the same defect the whole header exists to warn about: "CI-01 WROTE THE REASON DOWN WRONG, AND THE WRONG REASON WAS THE SAFE-SOUNDING ONE."

(b) It is now false prospectively, and this is the part that matters

SEC-2 (#108) merged into this PR's own base on 2026-09-23 04:08, four days after this head was cut. docs/RELEASE.md §1 now makes release/1.0 → master the mandated release path — the pull request that puts a version on the battery:

  1. Open a pull request release/1.0 → master. Do not push to master …
  2. Wait for checks / unit (pull_request) to go green on that pull request. The workflow in .gitea/workflows/checks.yml carries no branches: filter, so a master base matches and a real run happens — verified on throwaway PR #107 (base master), which queued a genuine run.

And master is now protected with the same single required context as release/1.0 (enable_status_check: true, status_check_contexts: ["checks / unit (pull_request)"], required_approvals: 0) — so master's gate inherits the identical fail-open-on-skipped property this file documents. I confirmed PR #107 (base master) carries a real success from run 814, 7m56s, with a job URL.

What a reader would wrongly do

A reader lands on the top preamble — which is exactly where docs/RELEASE.md sends them ("see the long note at the top of .gitea/workflows/checks.yml") — and takes away that master is a dead branch nothing targets. From that they conclude that the master half of any branches: list is inert, and they do not connect this guard to the release pull request at all. The truth today is the inverse: master is the one base where a skipped check ships untested code to a live 5 kW inverter.

Suggested fix

Drop the census-derived clause entirely; it adds nothing the main example does not already carry, and it is the only falsifiable sentence in the block. Something like:

(branches: [main] copied out of a GitHub template — this repo has no main, so such a filter matches nothing at all.)

If you want to keep a master example, it has to be re-dated against SEC-2 rather than against the pre-SEC-2 census. See review 3/5 for what I think should replace it.

## Review 2/5 — BLOCKING: "no pull request in 99 has ever targeted `master`" is false, and **this pull request is the counterexample** The clause reads: > a filter naming a branch nothing targets (`branches: [main]` from a template - this repo has no `main`, and **no pull request in 99 has ever targeted `master`**) `this repo has no main` is true — I listed the remote refs, there is no `main`. The `master` half is false, twice over. ### (a) It was already false when it was written — by the very pull request that writes it PR #102 was **opened against `master`**, at `2026-09-19T16:53:11+02:00`. The CI run it triggered is still in the forge, and its own environment block says so: ``` run 808 / job 1932, 2026-09-19T14:53:17Z PRNUM: 102 BASEREF: master base master 5184a2cfc25cdf7437fff775d58fd677a31ada4e head refs/pull/102/head 33e17dba0ee2160c0b711c5a47e4b091d34021a4 ``` (`5184a2c` was `master`'s tip on that date; PKG-15 did not bring `release/1.0` to `master` until the following day.) It was retargeted to `release/1.0` ~20 minutes later, which produced run 809 / job 1933 with `BASEREF: release/1.0`. The number therefore comes from the `/pulls` census, which reports a pull request's **current** base — and this file already contains the paragraph explaining exactly why that census cannot support this claim: > ⚠️ THAT 92 IS AN UPPER BOUND, NOT A CENSUS … The API reports a pull request's CURRENT base, so one that was opened stacked and later retargeted is recorded against its final base and is **invisible here**. The clause reuses the census result without reapplying the caveat written 25 lines above it. That is the same defect the whole header exists to warn about: *"CI-01 WROTE THE REASON DOWN WRONG, AND THE WRONG REASON WAS THE SAFE-SOUNDING ONE."* ### (b) It is now false prospectively, and this is the part that matters SEC-2 (#108) merged into this PR's own base on **2026-09-23 04:08**, four days after this head was cut. `docs/RELEASE.md` §1 now makes `release/1.0 → master` **the mandated release path** — the pull request that puts a version on the battery: > 2. Open a pull request **`release/1.0` → `master`**. Do not push to `master` … > 3. Wait for `checks / unit (pull_request)` to go green on that pull request. The workflow in `.gitea/workflows/checks.yml` carries **no `branches:` filter**, so a `master` base matches and a real run happens — verified on throwaway PR #107 (base `master`), which queued a genuine run. And `master` is now protected with the *same single required context* as `release/1.0` (`enable_status_check: true`, `status_check_contexts: ["checks / unit (pull_request)"]`, `required_approvals: 0`) — so `master`'s gate inherits the identical fail-open-on-`skipped` property this file documents. I confirmed PR #107 (base `master`) carries a real `success` from run 814, 7m56s, with a job URL. ### What a reader would wrongly do A reader lands on the top preamble — which is exactly where `docs/RELEASE.md` sends them ("see the long note at the top of `.gitea/workflows/checks.yml`") — and takes away that `master` is a dead branch nothing targets. From that they conclude that the `master` half of any `branches:` list is inert, and they do **not** connect this guard to the release pull request at all. The truth today is the inverse: `master` is the one base where a skipped check ships untested code to a live 5 kW inverter. ### Suggested fix Drop the census-derived clause entirely; it adds nothing the `main` example does not already carry, and it is the only falsifiable sentence in the block. Something like: > (`branches: [main]` copied out of a GitHub template — this repo has no `main`, so such a filter matches nothing at all.) If you want to keep a `master` example, it has to be re-dated against SEC-2 rather than against the pre-SEC-2 census. See review 3/5 for what I think should replace it.
Author
Owner

Review 3/5 — BLOCKING (gap): since SEC-2 the worst instance of blind spot 3 is branches: [release/1.0], and the clause does not mention it

This is the consequence of 2/5, stated as its own finding because it is not merely a stale fact — it is a case the clause's examples now actively steer a reader away from.

The clause names two candidate "innocuous tidy-up" filters: [main] and (by implication, via the pre-existing blind-spot-3 block below it) [master]. After SEC-2 there are two live pull request bases, release/1.0 and master, and the failure modes are no longer symmetric:

reintroduced filter release/1.0 PRs (development) release/1.0 → master PR (the release)
branches: [main] skipped → laundered green skipped → laundered green
branches: [master] skipped → laundered green matches, runs for real
branches: [release/1.0] matches, runs for real skipped → laundered green

The third row is the new one and it is the worst thing on this table. branches: [release/1.0] is the single most plausible edit anyone would make here — it looks like tightening the file to the branch the gate protects, it is what the filter said before CI-9 minus the dead half, and every day-to-day pull request would keep going green for real, so nothing would ever look wrong. The only pull request it silently switches off is the release/1.0 → master one — the deployment, the one docs/RELEASE.md §1 step 3 depends on running, the one whose payload reaches a real battery via ha store reload.

It is also invisible to the guard the trigger step for the ordinary blind-spot-3 reason, now doubled: the pull request introducing it targets release/1.0, which the filter names, so that pull request runs and the guard bites — good. But a filter that lands on release/1.0 some other way (blind spot 2: through a stacked pull request whose head already carries it), or simply a hand-edit on the release branch under a different gate, reaches master unchallenged, because by then the guard is inside a workflow that the master-based pull request skips.

What a reader would wrongly do

Reads the clause, internalises "the dangerous filter is one naming a branch nothing targets", concludes that branches: [release/1.0] is the safe narrowing — it names the branch that is obviously targeted — and writes it. Development pull requests keep going green, so the change looks validated. The next release pull request is skipped, laundered to success, cleared by master's brand-new protection rule, and merged untested onto the default branch that the Supervisor clones.

Suggested addition

Replace the census parenthetical from 2/5 with the shape of the risk rather than a list of branch names, e.g.:

Since SEC-2 there are TWO live bases — release/1.0 and master — so ANY branches: list skips one of them. [release/1.0] is the trap: it looks like tightening the guard onto the protected branch, every day-to-day pull request keeps running for real so nothing looks wrong, and the only thing it switches off is the release/1.0 -> master release pull request. That is the one that ships to the inverter. See docs/RELEASE.md §1.

I am not asking for the table; one sentence naming [release/1.0] as the trap is enough. But the clause should not ship naming only the two filters that are no longer the dangerous ones.

## Review 3/5 — BLOCKING (gap): since SEC-2 the worst instance of blind spot 3 is `branches: [release/1.0]`, and the clause does not mention it This is the consequence of 2/5, stated as its own finding because it is not merely a stale fact — it is a case the clause's examples now actively steer a reader away from. The clause names two candidate "innocuous tidy-up" filters: `[main]` and (by implication, via the pre-existing blind-spot-3 block below it) `[master]`. After SEC-2 there are **two live pull request bases**, `release/1.0` and `master`, and the failure modes are no longer symmetric: | reintroduced filter | `release/1.0` PRs (development) | `release/1.0 → master` PR (the release) | |---|---|---| | `branches: [main]` | skipped → laundered green | skipped → laundered green | | `branches: [master]` | **skipped → laundered green** | matches, runs for real | | `branches: [release/1.0]` | matches, runs for real | **skipped → laundered green** | The third row is the new one and it is the worst thing on this table. `branches: [release/1.0]` is the single most plausible edit anyone would make here — it *looks* like tightening the file to the branch the gate protects, it is what the filter said before CI-9 minus the dead half, and **every day-to-day pull request would keep going green for real**, so nothing would ever look wrong. The only pull request it silently switches off is the `release/1.0 → master` one — the deployment, the one `docs/RELEASE.md` §1 step 3 depends on running, the one whose payload reaches a real battery via `ha store reload`. It is also invisible to the `guard the trigger` step for the ordinary blind-spot-3 reason, now doubled: the pull request *introducing* it targets `release/1.0`, which the filter names, so that pull request runs and the guard bites — **good**. But a filter that lands on `release/1.0` some other way (blind spot 2: through a stacked pull request whose head already carries it), or simply a hand-edit on the release branch under a different gate, reaches `master` unchallenged, because by then the guard is inside a workflow that the `master`-based pull request skips. ### What a reader would wrongly do Reads the clause, internalises "the dangerous filter is one naming a branch nothing targets", concludes that `branches: [release/1.0]` is the *safe* narrowing — it names the branch that is obviously targeted — and writes it. Development pull requests keep going green, so the change looks validated. The next release pull request is skipped, laundered to `success`, cleared by `master`'s brand-new protection rule, and merged untested onto the default branch that the Supervisor clones. ### Suggested addition Replace the census parenthetical from 2/5 with the shape of the risk rather than a list of branch names, e.g.: > Since SEC-2 there are TWO live bases — `release/1.0` and `master` — so ANY `branches:` list skips one of them. `[release/1.0]` is the trap: it looks like tightening the guard onto the protected branch, every day-to-day pull request keeps running for real so nothing looks wrong, and the only thing it switches off is the `release/1.0 -> master` release pull request. That is the one that ships to the inverter. See `docs/RELEASE.md` §1. I am not asking for the table; one sentence naming `[release/1.0]` as the trap is enough. But the clause should not ship naming only the two filters that are no longer the dangerous ones.
Author
Owner

Review 4/5 — the green this pull request was showing was earned by a different pull request, on a base that no longer exists

Not a defect in the diff. Reporting it because it is load-bearing for the merge decision, because I disturbed it, and because it is this file's own documented mechanism appearing in a new costume.

What the status actually was

Before I touched anything, 33e17db carried success, Successful in 7m55s, timestamped 2026-09-23 04:11:42 — this morning. That looks like a fresh verdict on this pull request. It is not. Its target_url is run 819 / job 1943, and that job's environment block says:

run 819 / job 1943, 2026-09-23T02:03:48Z
  PRNUM: 111
  BASEREF: sec2rev-base
  base  sec2rev-base        33e17dba0ee2160c0b711c5a47e4b091d34021a4
  head  refs/pull/111/head  33e17dba0ee2160c0b711c5a47e4b091d34021a4

That is PR #111, one of SEC-2's throwaway probes, graded against the throwaway branch sec2rev-base — which is now deleted (GET /branches/sec2rev-base → 404). It was never a statement about ci9-gateproof merging into release/1.0. It landed on this pull request only because a commit status is keyed by head sha, and #111 momentarily shared this one.

The last genuine verdict on PR #102 before today was run 809 / job 1933, 2026-09-19 17:14, PRNUM: 102, BASEREF: release/1.0, base release/1.0 16b1d6c — i.e. four days and three merges stale, exactly as suspected.

This is the same sentence the file already contains, pointed the other way:

The skip is a COMMIT STATUS ON THE HEAD SHA, not a property of the pull request

CI-9 measured that for a skipped surviving a retarget. The dual is that a success from an unrelated pull request on an unrelated base lands on yours and reads as a passing gate. required_approvals is 0 on both protected branches, so on a busy day nothing else stands between that and a merge. Worth a sentence in this file at some point; I am not asking for it in this pull request, which should stay 12 lines.

Disclosure — what I did, and what it cost

I tried to refresh the stale green with POST /actions/runs/819/rerun (201). The rerun replayed run 819's stored payload — PR #111 / sec2rev-base — the fetch of that deleted branch failed, the checkout step failed at 0s, and Gitea posted failure on 33e17db. So for about two minutes this pull request was red, through my action and not through anything in the diff.

I then closed and reopened PR #102, which is the trigger this file documents (run 799). That produced run 821 / job 1946 with the correct payload:

  PRNUM: 102
  BASEREF: release/1.0
  base  release/1.0         239e120bf626bb69bc76d98af5296715578ebb55
  head  refs/pull/102/head  33e17dba0ee2160c0b711c5a47e4b091d34021a4
  checked out 239e120 SEC-2: write down the release path ... (#108)
  OK: the `on:` block of the merged checks.yml carries no event filter.

This is the first time this pull request has ever been graded against the current base. Its result is reported in 5/5.

Incidental note for whoever owns the runner: POST /actions/runs/<id>/rerun on this Gitea 1.27.3 replays the run's stored event payload, so rerunning any run whose base branch has since been deleted posts a spurious failure on whatever head sha that run carried. That is a foot-gun worth knowing about, and it is why the throwaway probe branches should be cleaned up rather than left around with live runs pointing at them.

## Review 4/5 — the green this pull request was showing was **earned by a different pull request**, on a base that no longer exists Not a defect in the diff. Reporting it because it is load-bearing for the merge decision, because I disturbed it, and because it is this file's own documented mechanism appearing in a new costume. ### What the status actually was Before I touched anything, `33e17db` carried `success`, `Successful in 7m55s`, timestamped **2026-09-23 04:11:42** — this morning. That looks like a fresh verdict on this pull request. It is not. Its `target_url` is run 819 / job 1943, and that job's environment block says: ``` run 819 / job 1943, 2026-09-23T02:03:48Z PRNUM: 111 BASEREF: sec2rev-base base sec2rev-base 33e17dba0ee2160c0b711c5a47e4b091d34021a4 head refs/pull/111/head 33e17dba0ee2160c0b711c5a47e4b091d34021a4 ``` That is **PR #111**, one of SEC-2's throwaway probes, graded against the throwaway branch `sec2rev-base` — which is now deleted (`GET /branches/sec2rev-base` → 404). It was never a statement about `ci9-gateproof` merging into `release/1.0`. It landed on this pull request only because a commit status is keyed by **head sha**, and #111 momentarily shared this one. The last *genuine* verdict on PR #102 before today was run 809 / job 1933, `2026-09-19 17:14`, `PRNUM: 102`, `BASEREF: release/1.0`, `base release/1.0 16b1d6c` — i.e. four days and three merges stale, exactly as suspected. This is the same sentence the file already contains, pointed the other way: > The skip is a COMMIT STATUS ON THE HEAD SHA, not a property of the pull request CI-9 measured that for a `skipped` surviving a retarget. The dual is that a **`success` from an unrelated pull request on an unrelated base** lands on yours and reads as a passing gate. `required_approvals` is 0 on both protected branches, so on a busy day nothing else stands between that and a merge. Worth a sentence in this file at some point; I am **not** asking for it in this pull request, which should stay 12 lines. ### Disclosure — what I did, and what it cost I tried to refresh the stale green with `POST /actions/runs/819/rerun` (201). The rerun replayed run 819's stored payload — PR #111 / `sec2rev-base` — the fetch of that deleted branch failed, the checkout step failed at 0s, and Gitea posted **`failure`** on `33e17db`. So for about two minutes this pull request was red, through my action and not through anything in the diff. I then closed and reopened PR #102, which is the trigger this file documents (run 799). That produced **run 821 / job 1946** with the correct payload: ``` PRNUM: 102 BASEREF: release/1.0 base release/1.0 239e120bf626bb69bc76d98af5296715578ebb55 head refs/pull/102/head 33e17dba0ee2160c0b711c5a47e4b091d34021a4 checked out 239e120 SEC-2: write down the release path ... (#108) OK: the `on:` block of the merged checks.yml carries no event filter. ``` This is the first time this pull request has ever been graded against the current base. Its result is reported in 5/5. Incidental note for whoever owns the runner: `POST /actions/runs/<id>/rerun` on this Gitea 1.27.3 replays the run's **stored event payload**, so rerunning any run whose base branch has since been deleted posts a spurious `failure` on whatever head sha that run carried. That is a foot-gun worth knowing about, and it is why the throwaway probe branches should be cleaned up rather than left around with live runs pointing at them.
Author
Owner

Review 5/6 — minor but actionable: the prescribed fix now under-specifies the forge change

(Renumbering: the verdict is 6/6, not 5/5 as 4/5 said.)

The clause's last sentence is the only instruction in the new block, and it is the one a future reader would act on:

Closing it needs a SECOND permanently unfiltered workflow posting its own context, with that context added to release/1.0's status_check_contexts - a forge change, with the owner.

That was complete on 2026-09-19, when release/1.0 was the only protected branch. It is not complete now. I read the rules back from the forge:

GET /api/v1/repos/admin/goodwe-addon/branch_protections
  release/1.0  status_check: True  ctx: ['checks / unit (pull_request)']  approvals: 0
  master       status_check: True  ctx: ['checks / unit (pull_request)']  approvals: 0

SEC-2 gave master a rule with the same single required context, so master's gate has the identical fail-open-on-skipped behaviour this whole file is about — and master is the branch the Supervisor clones.

What a reader would fail to do

Follows the instruction literally, adds the second workflow's context to release/1.0 only, and reports blind spot 3 closed. release/1.0 is then genuinely protected; master — the deployment gate — is left exactly as fail-open as it was, and now looks fixed. A half-applied fix that is believed complete is worse than an open ticket.

The pre-existing blind-spots block at the guard the trigger step (line ~388 on release/1.0 today) carries the same release/1.0-only wording, so this is not a defect introduced here — but this pull request is the one re-stating the instruction at the top of the file, where it will be read, and it should not re-state it stale. One word: "to both protected branches' status_check_contexts", or name them.

## Review 5/6 — minor but actionable: the prescribed fix now under-specifies the forge change (Renumbering: the verdict is 6/6, not 5/5 as 4/5 said.) The clause's last sentence is the only *instruction* in the new block, and it is the one a future reader would act on: > Closing it needs a SECOND permanently unfiltered workflow posting its own context, with that context added to **`release/1.0`'s `status_check_contexts`** - a forge change, with the owner. That was complete on 2026-09-19, when `release/1.0` was the only protected branch. It is not complete now. I read the rules back from the forge: ``` GET /api/v1/repos/admin/goodwe-addon/branch_protections release/1.0 status_check: True ctx: ['checks / unit (pull_request)'] approvals: 0 master status_check: True ctx: ['checks / unit (pull_request)'] approvals: 0 ``` SEC-2 gave `master` a rule with the **same single required context**, so `master`'s gate has the identical fail-open-on-`skipped` behaviour this whole file is about — and `master` is the branch the Supervisor clones. ### What a reader would fail to do Follows the instruction literally, adds the second workflow's context to `release/1.0` only, and reports blind spot 3 closed. `release/1.0` is then genuinely protected; `master` — the deployment gate — is left exactly as fail-open as it was, and now *looks* fixed. A half-applied fix that is believed complete is worse than an open ticket. The pre-existing blind-spots block at the `guard the trigger` step (line ~388 on `release/1.0` today) carries the same `release/1.0`-only wording, so this is not a defect introduced here — but this pull request is the one re-stating the instruction at the top of the file, where it will be read, and it should not re-state it stale. One word: **"to both protected branches' `status_check_contexts`"**, or name them.
Author
Owner

Amendment to 2/5 — narrowing my own claim, because I overstated part (a)

Correcting myself before you act on it. In 2/5 I headlined part (a) as "it was already false when it was written — by the very pull request that writes it." That is stronger than the evidence supports, and I should not leave it standing.

no pull request in 99 has ever targeted master states a claim over a population of 99 pull requests. PR #102 is outside any reading of "99" — whether that denominator means #1..#99 or a round count. So on the literal sentence, PR #102 having been opened against master is not a counterexample, and the sentence may well be literally true as written. I was wrong to call it false on that ground.

What the run-808 evidence actually establishes, and what I stand behind:

  1. The premise the parenthetical exists to support is unsupported. The construction is "a filter naming a branch nothing targets (… and no pull request in 99 has ever targeted master)". The parenthetical is offered as evidence for "master is a branch nothing targets." A census of 99 historical pull requests cannot establish that, and this file already says why — the /pulls API reports a pull request's current base, so a retarget erases the evidence. PR #102 is a live demonstration of the erasure, not a refutation of the count: it was opened on master (run 808 / job 1932: PRNUM: 102, BASEREF: master, base master 5184a2c), retargeted ~20 minutes later, and now reports as a release/1.0 pull request in exactly the API the number came from. The file's own words: "THAT 92 IS AN UPPER BOUND, NOT A CENSUS." The clause reuses the method without the caveat.

  2. Part (b) is untouched and is the blocking half. SEC-2 (#108) landed on this PR's base on 2026-09-23 04:08 and makes release/1.0 → master the mandated release path. Whatever was true of the first 99 pull requests, master is a live pull request base now, and the premise "a branch nothing targets" is simply wrong going forward. PR #107 (base master) ran for real — run 814, Successful in 7m56s.

So: 2/5's conclusion and its suggested fix stand unchanged — drop the census clause, it is the only falsifiable sentence in the block and it carries no weight the main example does not already carry. Its part (a) should be read as "the evidence is the wrong kind", not "the count is wrong". My apologies for the imprecision in a review whose whole subject is imprecise claims in this file.

## Amendment to 2/5 — narrowing my own claim, because I overstated part (a) Correcting myself before you act on it. In 2/5 I headlined part (a) as *"it was already false when it was written — by the very pull request that writes it."* That is stronger than the evidence supports, and I should not leave it standing. `no pull request in 99 has ever targeted master` states a claim over a population of 99 pull requests. PR #102 is outside any reading of "99" — whether that denominator means `#1..#99` or a round count. So on the literal sentence, PR #102 having been opened against `master` is **not** a counterexample, and the sentence may well be literally true as written. I was wrong to call it false on that ground. What the run-808 evidence actually establishes, and what I stand behind: 1. **The premise the parenthetical exists to support is unsupported.** The construction is "a filter naming a branch nothing targets (… *and no pull request in 99 has ever targeted `master`*)". The parenthetical is offered as evidence for *"`master` is a branch nothing targets."* A census of 99 historical pull requests cannot establish that, and this file already says why — the `/pulls` API reports a pull request's **current** base, so a retarget erases the evidence. PR #102 is a live demonstration of the erasure, not a refutation of the count: it was opened on `master` (run 808 / job 1932: `PRNUM: 102`, `BASEREF: master`, `base master 5184a2c`), retargeted ~20 minutes later, and now reports as a `release/1.0` pull request in exactly the API the number came from. The file's own words: *"THAT 92 IS AN UPPER BOUND, NOT A CENSUS."* The clause reuses the method without the caveat. 2. **Part (b) is untouched and is the blocking half.** SEC-2 (#108) landed on this PR's base on 2026-09-23 04:08 and makes `release/1.0 → master` the mandated release path. Whatever was true of the first 99 pull requests, `master` is a live pull request base now, and the premise "a branch nothing targets" is simply wrong going forward. PR #107 (base `master`) ran for real — run 814, `Successful in 7m56s`. So: 2/5's *conclusion* and its suggested fix stand unchanged — drop the census clause, it is the only falsifiable sentence in the block and it carries no weight the `main` example does not already carry. Its part (a) should be read as "the evidence is the wrong kind", not "the count is wrong". My apologies for the imprecision in a review whose whole subject is imprecise claims in this file.
Author
Owner

Review 6/6 — VERDICT: not merged. Blocking on 2/5 + 3/5. Worth merging once those are fixed — it is not redundant.

Not merged, and why

Three of the twelve lines make claims that were true on 2026-09-19 and are not true against the base this pull request now targets. In a file whose entire subject is "CI-01 WROTE THE REASON DOWN WRONG, AND THE WRONG REASON WAS THE SAFE-SOUNDING ONE", landing a freshly-stale claim in the most-read paragraph is the defect the file exists to prevent. All three are one-line edits:

  1. 2/5 + amendment — drop the no pull request in 99 has ever targeted master census clause. Its premise ("master is a branch nothing targets") is false as of SEC-2, and the method behind it is one this file already disclaims 25 lines above.
  2. 3/5 — name branches: [release/1.0] as the trap. It is now the filter that silently switches off the release/1.0 → master release pull request while every development pull request keeps going green.
  3. 5/6 — the prescribed forge fix says release/1.0's status_check_contexts; master now carries the same rule and needs the same context.

I have not pushed a fix. These are your twelve lines and the wording is the whole deliverable.

What I verified, and how — so you can disagree with the verdict on evidence

  • The new prose does not blind the guard. Guard's awk extracted verbatim and run against the branch's actual bytes: clean on base, clean on head, and it still bites on branches:, paths:, branches-ignore: and types: injected into the on: block of the head file. Full method in 1/5. Your pre-push check was correct.
  • Scope is comment-only, mechanically. 0 non-comment non-blank added lines, 0 removals, 1 file, non-comment lines identical 139==139, yaml.safe_load shows on: {pull_request, workflow_dispatch} with no filter, and the merged tree's checks.yml is byte-identical to the head's (sha256 ead9ed32…84484a).
  • The thesis is true. The guard genuinely cannot catch blind spots 1 and 3, because a guard inside a skipped workflow does not run. Confirmed against the forge, not just the comment: PR #101's head sha carries a single skipped / Skipped with an empty target_url, and /status rolls it up success, total_count 1. PR #96 is merged: true on nothing but that same laundered skipped.
  • Blind spot 3's "every subsequent pull request is skipped too" is sound, with one honest caveat. It is entailed by the measured premise — Gitea grades the workflow at refs/pull/<n>/head, measured on PR #101 — rather than independently re-measured by me: once the filtered file is on release/1.0, every branch cut from it carries the filter in its own head. I did not construct a fresh throwaway to watch the second pull request skip. If you want that measured rather than entailed, say so and I will do it on throwaway branches.
  • CI re-run against the current base. The green this pull request was displaying was not its own — see 4/5. Run 821 / job 1946 is the first genuine grading of ci9-gateproof merged into 239e120: success, Successful in 7m57s, all 30 test files PASS, and the guard step printing OK: the \on:` block of the merged checks.yml carries no event filter.` So the twelve lines are clean against the current base; the block is on the prose, not on the gate.

Why it is not redundant, and should not be closed

I considered "the situation changed, this no longer earns its place" and rejected it. The opposite happened:

  • The sentence it corrects — "The guard the trigger step below then keeps it that way mechanically rather than by asking the next editor to read this comment" — is still there on release/1.0 today, unqualified, and it is still false for two of the three blind spots. Nothing since 2026-09-19 has fixed it.
  • The correction it points at lives at line ~383, 270 lines below the false sentence. A reader who stops after the summary gets the false version. Closing that gap is the whole point of these twelve lines and no other change has closed it.
  • docs/RELEASE.md, which landed four days after this head, does not duplicate or supersede this clause — it delegates to it: "A skipped status is laundered into a success rollup by this Gitea and the merge gate accepts it — see the long note at the top of .gitea/workflows/checks.yml. Never add an event filter there." SEC-2 made the top of this file load-bearing for the release procedure. That raises the value of an accurate summary there; it does not make it redundant.

So: fix the three sentences and I will merge it. It is a good edit aimed at a real falsehood, which is why it needs to be accurate itself.

## Review 6/6 — VERDICT: **not merged.** Blocking on 2/5 + 3/5. Worth merging once those are fixed — it is not redundant. ### Not merged, and why Three of the twelve lines make claims that were true on 2026-09-19 and are not true against the base this pull request now targets. In a file whose entire subject is *"CI-01 WROTE THE REASON DOWN WRONG, AND THE WRONG REASON WAS THE SAFE-SOUNDING ONE"*, landing a freshly-stale claim in the most-read paragraph is the defect the file exists to prevent. All three are one-line edits: 1. **2/5 + amendment** — drop the `no pull request in 99 has ever targeted master` census clause. Its premise ("`master` is a branch nothing targets") is false as of SEC-2, and the method behind it is one this file already disclaims 25 lines above. 2. **3/5** — name `branches: [release/1.0]` as the trap. It is now the filter that silently switches off the `release/1.0 → master` release pull request while every development pull request keeps going green. 3. **5/6** — the prescribed forge fix says `release/1.0`'s `status_check_contexts`; `master` now carries the same rule and needs the same context. I have not pushed a fix. These are your twelve lines and the wording is the whole deliverable. ### What I verified, and how — so you can disagree with the verdict on evidence * **The new prose does not blind the guard.** Guard's `awk` extracted verbatim and run against the branch's actual bytes: clean on base, clean on head, and it still bites on `branches:`, `paths:`, `branches-ignore:` and `types:` injected into the `on:` block *of the head file*. Full method in 1/5. Your pre-push check was correct. * **Scope is comment-only, mechanically.** 0 non-comment non-blank added lines, 0 removals, 1 file, non-comment lines identical 139==139, `yaml.safe_load` shows `on: {pull_request, workflow_dispatch}` with no filter, and the merged tree's `checks.yml` is byte-identical to the head's (sha256 `ead9ed32…84484a`). * **The thesis is true.** The guard genuinely cannot catch blind spots 1 and 3, because a guard inside a skipped workflow does not run. Confirmed against the forge, not just the comment: PR #101's head sha carries a single `skipped` / `Skipped` with an empty `target_url`, and `/status` rolls it up `success, total_count 1`. PR #96 is `merged: true` on nothing but that same laundered `skipped`. * **Blind spot 3's "every subsequent pull request is skipped too" is sound, with one honest caveat.** It is *entailed* by the measured premise — Gitea grades the workflow at `refs/pull/<n>/head`, measured on PR #101 — rather than independently re-measured by me: once the filtered file is on `release/1.0`, every branch cut from it carries the filter in its own head. I did not construct a fresh throwaway to watch the second pull request skip. If you want that measured rather than entailed, say so and I will do it on throwaway branches. * **CI re-run against the current base.** The green this pull request was displaying was not its own — see 4/5. Run 821 / job 1946 is the first genuine grading of `ci9-gateproof` merged into `239e120`: **`success`, `Successful in 7m57s`**, all **30** test files `PASS`, and the guard step printing `OK: the \`on:\` block of the merged checks.yml carries no event filter.` So the twelve lines are clean against the current base; the block is on the prose, not on the gate. ### Why it is *not* redundant, and should not be closed I considered "the situation changed, this no longer earns its place" and rejected it. The opposite happened: * The sentence it corrects — *"The `guard the trigger` step below then keeps it that way **mechanically** rather than by asking the next editor to read this comment"* — is still there on `release/1.0` today, unqualified, and it is still false for two of the three blind spots. Nothing since 2026-09-19 has fixed it. * The correction it points at lives at line ~383, **270 lines below** the false sentence. A reader who stops after the summary gets the false version. Closing that gap is the whole point of these twelve lines and no other change has closed it. * `docs/RELEASE.md`, which landed four days *after* this head, does not duplicate or supersede this clause — it **delegates to it**: *"A `skipped` status is laundered into a `success` rollup by this Gitea and the merge gate accepts it — see the long note at the top of `.gitea/workflows/checks.yml`. Never add an event filter there."* SEC-2 made the top of this file load-bearing for the release procedure. That raises the value of an accurate summary there; it does not make it redundant. So: fix the three sentences and I will merge it. It is a good edit aimed at a real falsehood, which is why it needs to be accurate itself.
admin added 1 commit 2026-09-23 20:18:54 +02:00
Three sentences in the blind-spot clause went stale between writing and review.

`no pull request in 99 has ever targeted master` is no longer true: SEC-2 made
`release/1.0` -> `master` the mandated release path and protected `master`
with the same required context, and a PR against that base has run for real.
The census it came from also erases its own evidence - PR #102 was itself
opened against `master` before retargeting, which the /pulls listing no longer
shows, exactly as the caveat 25 lines above this clause warns.

The clause also named `branches: [main]` as the hazard. That is the version
that looks wrong. The one someone would actually write is
`branches: [release/1.0]` - it reads as tightening onto the protected branch,
every day-to-day PR keeps running green, and the only thing it switches off is
the release PR whose content reaches an installed system.

And the prescribed fix named only `release/1.0`'s status_check_contexts. Since
SEC-2 `master` carries the same rule, so a reader following it literally would
half-close the hole and believe it shut.

Guard re-verified against the edited bytes: passes on this file (whose new
prose quotes `branches: [release/1.0]` in a comment) and still refuses a real
filter added to the `on:` block. Comment-only: 0 non-comment lines changed.

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

Round 2, 7/8 — re-verified on 628279a: all three blockers fixed, guard still not blinded, scope still clean

Re-read from the remote (git fetch --prune), not from your message. Head 628279a68aca8d9197e0951b91f4d455f98b6d24, base now c9aaf7f.

The three blockers

  1. Census gone. no pull request in 99 has ever targeted master is deleted. The clause makes no claim about the repo's pull request history any more. ✅
  2. branches: [release/1.0] is now the centrepiece — and framed exactly right: it "looks like tightening onto the protected branch and is what a careful person would write", day-to-day pull requests keep going green so nothing appears broken, and the thing switched off is the release pull request. ✅
  3. Fix instruction now says BOTH release/1.0 AND master, with the "adding it to one alone leaves the hole open on the other" consequence spelled out. Matches the forge: both rules read back enable_status_check: true, ['checks / unit (pull_request)'], required_approvals: 0. ✅

Guard, re-verified from the branch's bytes

The new prose quotes `branches: [release/1.0]` and `branches: [main]`. I re-extracted the awk from the head file (confirmed byte-identical to the program I ran in 1/5 — the step itself is untouched) and ran it:

  • base c9aaf7f → clean
  • head 628279a → clean
  • negative controls injected under pull_request: in the head file:
branches: [release/1.0]  -> BITES  "152:     branches: [release/1.0]"
branches: [main]         -> BITES  "152:     branches: [main]"
paths: + a path entry    -> BITES  "152:     paths:"
paths-ignore: [docs/**]  -> BITES  "152:     paths-ignore: [docs/**]"
tags: [v1]               -> BITES  "152:     tags: [v1]"

Your check was right. The new lines are full-line comments at column 0, 33 lines above on:, and /^[[:space:]]*#/ { next } still precedes the on: rule.

Scope, mechanically

  • non-comment non-blank added lines: 0; removed lines: 0; files: .gitea/workflows/checks.yml only
  • non-comment lines base vs head: identical, 139 == 139
  • yaml.safe_load(head) → name: checks, on: {pull_request: None, workflow_dispatch: None}
  • merged tree (git merge-tree origin/release/1.0 origin/ci9-gateproof) differs from c9aaf7f in exactly one path, .gitea/workflows/checks.yml, and that blob is byte-identical to the head's: sha256 2f7237e7…e37e7a

What moved underneath — no contradiction

  • SAFETY-56 (#112, merged 21:27:41) touches freshness.py, main.py, test_freshness.py, a behaviour trace and two decision notes. It does not touch checks.yml or docs/RELEASE.md. Orthogonal.
  • docs/RELEASE.md is byte-identical at 239e120 and c9aaf7f. Its delegation still reads "see the long note at the top of .gitea/workflows/checks.yml. Never add an event filter there", and step 3 rests on "carries no branches: filter, so a master base matches". Your new paragraph is precisely about the filter that would break that sentence, and it cites docs/RELEASE.md back. They reinforce each other; nothing to reconcile.

Green traced (CI-10)

The success on 628279a is run 822 / job 1947, and its payload is genuinely this pull request's:

  PRNUM: 102
  BASEREF: release/1.0
  base  release/1.0         239e120bf626bb69bc76d98af5296715578ebb55
  head  refs/pull/102/head  628279a68aca8d9197e0951b91f4d455f98b6d24
  OK: the `on:` block of the merged checks.yml carries no event filter.
  30 test file(s) to run   ->  30 PASS, 0 FAIL

No repeat of the run-819 problem: the push produced its own run, and that run names PR 102 on release/1.0. One caveat: run 822 started 20:18:58 and checked out base 239e120; SAFETY-56 merged at 21:27:41, so the green grades 628279a merged into 239e120, not into today's c9aaf7f. Given SAFETY-56 is runtime-message work with zero overlap, and the merged tree's only differing path is this comment file, I am not burning another 8 minutes of the shared capacity: 1 runner on that — a fix push for 8/8 will produce a fresh run against the current tip anyway.

One thing still blocks, in 8/8.

## Round 2, 7/8 — re-verified on `628279a`: all three blockers fixed, guard still not blinded, scope still clean Re-read from the remote (`git fetch --prune`), not from your message. Head `628279a68aca8d9197e0951b91f4d455f98b6d24`, base now `c9aaf7f`. ### The three blockers 1. **Census gone.** `no pull request in 99 has ever targeted master` is deleted. The clause makes no claim about the repo's pull request history any more. ✅ 2. **`branches: [release/1.0]` is now the centrepiece** — and framed exactly right: it *"looks like tightening onto the protected branch and is what a careful person would write"*, day-to-day pull requests keep going green so nothing appears broken, and the thing switched off is the release pull request. ✅ 3. **Fix instruction** now says BOTH `release/1.0` AND `master`, with the "adding it to one alone leaves the hole open on the other" consequence spelled out. Matches the forge: both rules read back `enable_status_check: true`, `['checks / unit (pull_request)']`, `required_approvals: 0`. ✅ ### Guard, re-verified from the branch's bytes The new prose quotes `` `branches: [release/1.0]` `` and `` `branches: [main]` ``. I re-extracted the `awk` from the head file (confirmed byte-identical to the program I ran in 1/5 — the step itself is untouched) and ran it: * base `c9aaf7f` → clean * head `628279a` → clean * negative controls injected under `pull_request:` **in the head file**: ``` branches: [release/1.0] -> BITES "152: branches: [release/1.0]" branches: [main] -> BITES "152: branches: [main]" paths: + a path entry -> BITES "152: paths:" paths-ignore: [docs/**] -> BITES "152: paths-ignore: [docs/**]" tags: [v1] -> BITES "152: tags: [v1]" ``` Your check was right. The new lines are full-line comments at column 0, 33 lines above `on:`, and `/^[[:space:]]*#/ { next }` still precedes the `on:` rule. ### Scope, mechanically * non-comment non-blank added lines: **0**; removed lines: **0**; files: `.gitea/workflows/checks.yml` only * non-comment lines base vs head: **identical, 139 == 139** * `yaml.safe_load(head)` → `name: checks`, `on: {pull_request: None, workflow_dispatch: None}` * merged tree (`git merge-tree origin/release/1.0 origin/ci9-gateproof`) differs from `c9aaf7f` in **exactly one path**, `.gitea/workflows/checks.yml`, and that blob is byte-identical to the head's: sha256 `2f7237e7…e37e7a` ### What moved underneath — no contradiction * **SAFETY-56 (#112, merged 21:27:41)** touches `freshness.py`, `main.py`, `test_freshness.py`, a behaviour trace and two decision notes. It does **not** touch `checks.yml` or `docs/RELEASE.md`. Orthogonal. * **`docs/RELEASE.md` is byte-identical at `239e120` and `c9aaf7f`.** Its delegation still reads *"see the long note at the top of `.gitea/workflows/checks.yml`. Never add an event filter there"*, and step 3 rests on *"carries no `branches:` filter, so a `master` base matches"*. Your new paragraph is precisely about the filter that would break that sentence, and it cites `docs/RELEASE.md` back. They reinforce each other; nothing to reconcile. ### Green traced (CI-10) The `success` on `628279a` is **run 822 / job 1947**, and its payload is genuinely this pull request's: ``` PRNUM: 102 BASEREF: release/1.0 base release/1.0 239e120bf626bb69bc76d98af5296715578ebb55 head refs/pull/102/head 628279a68aca8d9197e0951b91f4d455f98b6d24 OK: the `on:` block of the merged checks.yml carries no event filter. 30 test file(s) to run -> 30 PASS, 0 FAIL ``` No repeat of the run-819 problem: the push produced its own run, and that run names PR 102 on `release/1.0`. **One caveat:** run 822 started 20:18:58 and checked out base `239e120`; SAFETY-56 merged at 21:27:41, so the green grades `628279a` merged into `239e120`, not into today's `c9aaf7f`. Given SAFETY-56 is runtime-message work with zero overlap, and the merged tree's only differing path is this comment file, I am not burning another 8 minutes of the shared `capacity: 1` runner on that — a fix push for 8/8 will produce a fresh run against the current tip anyway. One thing still blocks, in 8/8.
Author
Owner

Round 2, 8/8 — BLOCKING, one word: the rewritten mechanism sentence now states the mechanism backwards

This is new in 628279a — it was not in 33e17db, and it is a side effect of generalising the sentence away from the [main] example. It is the only thing standing between this pull request and a merge.

Lines 117-120 of the head file:

Blind spot 3 is the one that matters: a filter naming a branch THIS pull request does not target is itself skipped, lands green and untested, and then every subsequent pull request matching it is skipped too.

Parse the last clause. The sentence's subject is "a filter"; it is the only thing it can attach to that a pull request can be said to match. A pull request that matches a branches: filter is one whose base is named in it — and that pull request runs. It is the pull request that does not match that gets skipped, laundered and merged untested. As written, the clause asserts the exact inverse of the behaviour the rest of the file measures.

The previous wording did not have this problem: "and then every subsequent pull request is skipped too" — unqualified, and true for the [main] case it was attached to. Generalising the first half to "a branch THIS pull request does not target" made a qualifier necessary on the second half, and matching it is the wrong qualifier.

Why one word matters here rather than being a nit

The paragraph immediately below states the same mechanism correctly and concretely ([release/1.0]: "Every day-to-day pull request still runs and still goes green … the only thing switched off is the release PR"). So the file now contains a summary whose plain reading contradicts the paragraph under it. That is not a hypothetical standard I am importing — it is the standard this file already applied to itself, 260 lines down:

⚠️ AND IT IS THE HEAD'S WORKFLOW THAT DECIDES WHETHER ANYTHING RUNS, NOT THE BASE'S. An earlier draft of this comment claimed … That was wrong, it contradicted the checkout step's own note a few lines below, and the error was not cosmetic: it made this step sound like an unconditional ratchet, which is exactly the false comfort CI-9 exists to stamp out.

Same shape, same file, one round later. And the sentence is load-bearing: it is the one-line statement of blind spot 3 in the preamble that docs/RELEASE.md sends every releaser to.

What a reader would wrongly do

Takes away that a branches: filter skips the pull requests it names. From that, the safe move looks like omitting your base from the filter — the precise edit that turns the gate off — and the [release/1.0] paragraph below reads as a contradiction they must resolve on their own, with no measurement in front of them to resolve it with.

Fix — pick either

…and then every subsequent pull request that it does not name is skipped too.

or

…and then every subsequent pull request it excludes is skipped too.

Push that and I will merge on sight — everything else on 628279a verifies clean (7/8): guard not blinded, comment-only, 0 non-comment changed lines, merged tree differing from c9aaf7f in this one file, no contradiction with docs/RELEASE.md or SAFETY-56, and the green traced to a genuine PR-102 run.

## Round 2, 8/8 — BLOCKING, one word: the rewritten mechanism sentence now states the mechanism **backwards** This is new in `628279a` — it was not in `33e17db`, and it is a side effect of generalising the sentence away from the `[main]` example. It is the only thing standing between this pull request and a merge. Lines 117-120 of the head file: > Blind spot 3 is the one that matters: a filter naming a branch THIS pull request does not target is itself skipped, lands green and untested, and then **every subsequent pull request matching it is skipped too**. Parse the last clause. The sentence's subject is *"a filter"*; it is the only thing `it` can attach to that a pull request can be said to *match*. A pull request that **matches** a `branches:` filter is one whose base is named in it — and that pull request **runs**. It is the pull request that does **not** match that gets skipped, laundered and merged untested. As written, the clause asserts the exact inverse of the behaviour the rest of the file measures. The previous wording did not have this problem: *"and then every subsequent pull request is skipped too"* — unqualified, and true for the `[main]` case it was attached to. Generalising the first half to *"a branch THIS pull request does not target"* made a qualifier necessary on the second half, and `matching it` is the wrong qualifier. ### Why one word matters here rather than being a nit The paragraph immediately below states the same mechanism correctly and concretely (`[release/1.0]`: *"Every day-to-day pull request still runs and still goes green … the only thing switched off is the release PR"*). So the file now contains a summary whose plain reading contradicts the paragraph under it. That is not a hypothetical standard I am importing — it is the standard this file already applied to itself, 260 lines down: > ⚠️ AND IT IS THE HEAD'S WORKFLOW THAT DECIDES WHETHER ANYTHING RUNS, NOT THE BASE'S. An earlier draft of this comment claimed … **That was wrong, it contradicted the checkout step's own note a few lines below, and the error was not cosmetic:** it made this step sound like an unconditional ratchet, which is exactly the false comfort CI-9 exists to stamp out. Same shape, same file, one round later. And the sentence is load-bearing: it is the one-line statement of blind spot 3 in the preamble that `docs/RELEASE.md` sends every releaser to. ### What a reader would wrongly do Takes away that a `branches:` filter skips the pull requests it names. From that, the safe move looks like *omitting* your base from the filter — the precise edit that turns the gate off — and the `[release/1.0]` paragraph below reads as a contradiction they must resolve on their own, with no measurement in front of them to resolve it with. ### Fix — pick either > …and then every subsequent pull request **that it does not name** is skipped too. or > …and then every subsequent pull request **it excludes** is skipped too. Push that and I will merge on sight — everything else on `628279a` verifies clean (7/8): guard not blinded, comment-only, 0 non-comment changed lines, merged tree differing from `c9aaf7f` in this one file, no contradiction with `docs/RELEASE.md` or SAFETY-56, and the green traced to a genuine PR-102 run.
admin added 1 commit 2026-09-24 14:27:23 +02:00
CI-9 follow-up: the skip direction, written the right way round
checks / unit (pull_request) Successful in 8m37s
8f9c2cad57
`every subsequent pull request matching it is skipped too` is the inverse of
the behaviour. A pull request that MATCHES a `branches:` filter still RUNS;
the one the filter EXCLUDES is what Gitea sits out, reports `skipped`, and the
rollup launders into `success`.

The error arrived with the generalisation in the previous commit: `33e17db`
said "every subsequent pull request is skipped too", which was unqualified and
true for the `[main]` case it described. Naming the wider hazard made a
qualifier necessary, and the wrong one went in - contradicting the paragraph
directly below it, which describes `[release/1.0]` correctly as leaving every
day-to-day PR running and green.

Corrected, and the direction is now stated explicitly with the failed draft
recorded, because this file already carries one instance of the same shape:
"That was wrong, it contradicted the checkout step's own note a few lines
below, and the error was not cosmetic."

Guard re-verified on the edited bytes: passes here, still refuses
`branches: [release/1.0]` added to the `on:` block. Comment-only, 0
non-comment lines changed.

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

Round 3, 9/9 — APPROVED, merging. The direction is now right, and the failed draft is on the record.

Re-read from the remote at head 8f9c2cad5781e90240de6179c4427a5895507895, base c9aaf7f.

The inversion is fixed

…and then every subsequent pull request the filter DOES NOT NAME is skipped too.

and the new paragraph states the direction outright — "A pull request that MATCHES the filter still runs; it is the one the filter excludes that Gitea sits out, reports skipped, and the rollup launders into success." That matches every measurement in this file and on the forge. Recording the failed draft rather than quietly correcting it was the right call: this file now carries two worked instances of the same error shape, which is worth more than one plus a silent fix. I checked the self-reference too — "contradicted the paragraph directly below" still resolves correctly, since the [main] / [release/1.0] paragraph is what sits directly below the new one.

Guard, re-verified from the bytes (third time, new bytes)

The guard's awk in the head file is byte-identical to the copy on c9aaf7f (diffed, not eyeballed), so the program I ran is the program that runs. Clean on base, clean on head. Eight negative controls injected under pull_request: in the head file, all refused:

branches: [release/1.0]   BITES      paths: + entry         BITES
branches: [main]          BITES      paths-ignore: [...]    BITES
branches: [master]        BITES      branches-ignore: [wip] BITES
tags-ignore: [x]          BITES      types: [opened, ...]   BITES

Plus one I had not tried before: the inline-flow form on: { pull_request: { branches: [release/1.0] } } on the on: line itself — also refused, at line 157. The three new ⚠️ blocks quote branches: six times between them and trip nothing.

Scope

0 non-comment non-blank added lines · 0 removals · 1 file · non-comment lines identical 139 == 139 · yaml.safe_load → on: {pull_request, workflow_dispatch} · the merged tree differs from c9aaf7f in exactly one path, .gitea/workflows/checks.yml, and that blob is byte-identical to the head's (sha256 b9ecd31f…0ebc5).

Green traced, CI-10 satisfied — and this time against the real tip

Run 824 / job 1949:

  PRNUM: 102
  BASEREF: release/1.0
  base  release/1.0         c9aaf7f04db6a3a8f4bbcfc12d11c1e121458b80   <- current tip
  head  refs/pull/102/head  8f9c2cad5781e90240de6179c4427a5895507895
  GRADED: refs/pull/102/head merged into release/1.0
  OK: the `on:` block of the merged checks.yml carries no event filter.
  30 test file(s) to run   ->  30 PASS, 0 FAIL, Successful in 8m37s

One status on the head, rollup success, total_count 1, and the payload names this pull request on this base. Your caveat from last round is cleared: this is the first grading of these twelve-turned-thirty-one lines against c9aaf7f.

SAFETY-56, now that it is actually on the base

Touches DECISIONS/SAFETY-56*.md, DECISIONS/SAFETY-57*.md, app/freshness.py, app/main.py, test_freshness.py, tools/safety56_behaviour_trace.py. Grepped its whole diff for checks.yml, branches:, status_check, required context, merge gate, laundered, branch protection, RELEASE.md — no hits. docs/RELEASE.md is unchanged since 239e120. The word "gate" appears in SAFETY-56 only in its runtime sense (the freshness gate), not the merge gate, and in files this clause never refers to. No contradiction.

Remaining claims spot-checked against the forge: no refs/heads/main exists (git ls-remote), so "which at least looks wrong" holds; release/1.0 and master both read back enable_status_check: true, ['checks / unit (pull_request)'], required_approvals: 0, block_admin_merge_override: true, so "both carry the rule since SEC-2" holds; docs/RELEASE.md §1 does mandate the release/1.0 → master pull request.

One non-blocking note, for the record rather than for this pull request

"the only thing switched off is the release/1.0 -> master release PR" is very slightly stronger than true: branches: [release/1.0] would also skip a stacked pull request (base a feature branch), which is blind spot 2 and the exact case CI-9 removed the old filter to fix. The release pull request is rightly the headline — it is the one that reaches an installed system — and the stacked case is documented both in the CI-9 paragraph above and as blind spot 2 below, so nothing is lost. Not worth a fourth round; noting it so it is not mistaken for a missed check.

Merging.

## Round 3, 9/9 — APPROVED, merging. The direction is now right, and the failed draft is on the record. Re-read from the remote at head `8f9c2cad5781e90240de6179c4427a5895507895`, base `c9aaf7f`. ### The inversion is fixed > …and then every subsequent pull request **the filter DOES NOT NAME** is skipped too. and the new paragraph states the direction outright — *"A pull request that MATCHES the filter still runs; it is the one the filter excludes that Gitea sits out, reports `skipped`, and the rollup launders into `success`."* That matches every measurement in this file and on the forge. Recording the failed draft rather than quietly correcting it was the right call: this file now carries two worked instances of the same error shape, which is worth more than one plus a silent fix. I checked the self-reference too — *"contradicted the paragraph directly below"* still resolves correctly, since the `[main]` / `[release/1.0]` paragraph is what sits directly below the new one. ### Guard, re-verified from the bytes (third time, new bytes) The guard's `awk` in the head file is **byte-identical to the copy on `c9aaf7f`** (diffed, not eyeballed), so the program I ran is the program that runs. Clean on base, clean on head. Eight negative controls injected under `pull_request:` **in the head file**, all refused: ``` branches: [release/1.0] BITES paths: + entry BITES branches: [main] BITES paths-ignore: [...] BITES branches: [master] BITES branches-ignore: [wip] BITES tags-ignore: [x] BITES types: [opened, ...] BITES ``` Plus one I had not tried before: the inline-flow form `on: { pull_request: { branches: [release/1.0] } }` on the `on:` line itself — also refused, at line 157. The three new `⚠️` blocks quote `branches:` six times between them and trip nothing. ### Scope 0 non-comment non-blank added lines · 0 removals · 1 file · non-comment lines identical 139 == 139 · `yaml.safe_load` → `on: {pull_request, workflow_dispatch}` · the merged tree differs from `c9aaf7f` in **exactly one path**, `.gitea/workflows/checks.yml`, and that blob is byte-identical to the head's (sha256 `b9ecd31f…0ebc5`). ### Green traced, CI-10 satisfied — and this time against the real tip Run **824 / job 1949**: ``` PRNUM: 102 BASEREF: release/1.0 base release/1.0 c9aaf7f04db6a3a8f4bbcfc12d11c1e121458b80 <- current tip head refs/pull/102/head 8f9c2cad5781e90240de6179c4427a5895507895 GRADED: refs/pull/102/head merged into release/1.0 OK: the `on:` block of the merged checks.yml carries no event filter. 30 test file(s) to run -> 30 PASS, 0 FAIL, Successful in 8m37s ``` One status on the head, rollup `success, total_count 1`, and the payload names this pull request on this base. Your caveat from last round is cleared: this is the first grading of these twelve-turned-thirty-one lines against `c9aaf7f`. ### SAFETY-56, now that it is actually on the base Touches `DECISIONS/SAFETY-56*.md`, `DECISIONS/SAFETY-57*.md`, `app/freshness.py`, `app/main.py`, `test_freshness.py`, `tools/safety56_behaviour_trace.py`. Grepped its whole diff for `checks.yml`, `branches:`, `status_check`, `required context`, `merge gate`, `laundered`, `branch protection`, `RELEASE.md` — **no hits**. `docs/RELEASE.md` is unchanged since `239e120`. The word "gate" appears in SAFETY-56 only in its runtime sense (the freshness gate), not the merge gate, and in files this clause never refers to. No contradiction. Remaining claims spot-checked against the forge: no `refs/heads/main` exists (`git ls-remote`), so *"which at least looks wrong"* holds; `release/1.0` and `master` both read back `enable_status_check: true`, `['checks / unit (pull_request)']`, `required_approvals: 0`, `block_admin_merge_override: true`, so *"both carry the rule since SEC-2"* holds; `docs/RELEASE.md` §1 does mandate the `release/1.0 → master` pull request. ### One non-blocking note, for the record rather than for this pull request *"the only thing switched off is the `release/1.0` -> `master` release PR"* is very slightly stronger than true: `branches: [release/1.0]` would also skip a **stacked** pull request (base a feature branch), which is blind spot 2 and the exact case CI-9 removed the old filter to fix. The release pull request is rightly the headline — it is the one that reaches an installed system — and the stacked case is documented both in the CI-9 paragraph above and as blind spot 2 below, so nothing is lost. Not worth a fourth round; noting it so it is not mistaken for a missed check. **Merging.**
admin merged commit 9f6b9f14cd into release/1.0 2026-09-24 14:37:16 +02:00
Author
Owner

Merged — release/1.0 is now 9f6b9f14cd260a4b84e1561d96a76f9368d6e23e

Merge commit 9f6b9f1, merged 2026-09-24T14:37:16+02:00, on head 8f9c2cad5781e90240de6179c4427a5895507895 (passed as head_commit_id, so the merge would have been refused 409 head out of date had the branch moved under it).

Confirmed from the remote, two independent ways:

GET /api/v1/.../branches/release%2F1.0  ->  9f6b9f14cd260a4b84e1561d96a76f9368d6e23e   protected: true
git ls-remote origin                    ->  9f6b9f14...  refs/heads/release/1.0
                                            0f14c2a5...  refs/heads/master   (unmoved)

Post-merge checks on the landed content:

  • .gitea/workflows/checks.yml on release/1.0 is byte-identical to the head's — sha256 b9ecd31f…0ebc5
  • the guard's awk, run against the landed file, is clean
  • both branch protection rules read back unchanged: enable_status_check: true, ['checks / unit (pull_request)'], required_approvals: 0, block_admin_merge_override: true

master is untouched at 0f14c2a. Nothing here reaches an installed system; shipping this still requires §1 of docs/RELEASE.md.

Three rounds, three real blockers, and the last one was an inversion you had written into the sentence that states the mechanism. Worth the rounds.

## Merged — `release/1.0` is now `9f6b9f14cd260a4b84e1561d96a76f9368d6e23e` Merge commit `9f6b9f1`, merged `2026-09-24T14:37:16+02:00`, on head `8f9c2cad5781e90240de6179c4427a5895507895` (passed as `head_commit_id`, so the merge would have been refused `409 head out of date` had the branch moved under it). Confirmed from the remote, two independent ways: ``` GET /api/v1/.../branches/release%2F1.0 -> 9f6b9f14cd260a4b84e1561d96a76f9368d6e23e protected: true git ls-remote origin -> 9f6b9f14... refs/heads/release/1.0 0f14c2a5... refs/heads/master (unmoved) ``` Post-merge checks on the landed content: * `.gitea/workflows/checks.yml` on `release/1.0` is byte-identical to the head's — sha256 `b9ecd31f…0ebc5` * the guard's `awk`, run against the **landed** file, is clean * both branch protection rules read back unchanged: `enable_status_check: true`, `['checks / unit (pull_request)']`, `required_approvals: 0`, `block_admin_merge_override: true` `master` is untouched at `0f14c2a`. Nothing here reaches an installed system; shipping this still requires §1 of `docs/RELEASE.md`. Three rounds, three real blockers, and the last one was an inversion you had written into the sentence that states the mechanism. Worth the rounds.
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: admin/goodwe-addon#102