chore(ci): release.yml's concurrency block has no regression guard while its documented complement does #1032

Closed
opened 2026-08-28 15:55:48 +02:00 by bosun · 2 comments
Owner

release.yml's concurrency block has no regression guard, while the mechanism its own comment names as a complement does.

The asymmetry

#1031's comment says the block "Complements the #139 paths-ignore above". paths-ignore has a regression guard on release.yml. concurrency does not.

delete paths-ignore from release.yml   RED   workflows.bats — "#139 regression guard …
                                             catches a future edit that drops paths-ignore
                                             + would re-introduce the self-cancel race"
delete concurrency from release.yml    GREEN nothing
move concurrency TO a reusable         RED   the reusable-*.yml ban

Absent and misplaced are indistinguishable in outcome; only one of them is caught. The glob is reusable-*.yml and by construction cannot match release.yml.

The scaffolding already exists

workflows.bats carries two arms that parse release.yml with python3 + yaml in exactly the needed shape — one asserting push.paths-ignore contains the manifest, one asserting every uses: job declares secrets: inherit.

So this is not "add a YAML parse". It is "copy the arm and assert the concurrency dict".

Why the placement matters — measured on a live runner (#595)

block on CALLER     two dispatches QUEUE       1s gap
block on REUSABLE   two dispatches CONCURRENT  71s overlap
no block at all     two dispatches CONCURRENT  67s overlap    <- control

Acceptance criteria

  • An arm parses release.yml and asserts a top-level concurrency block with cancel-in-progress: false, in the shape of the existing paths-ignore arm — DONE: tests/workflows.bats — yaml parse of release.yml, asserts top-level concurrency and cancel-in-progress is not False. PR#1042, merged c05cd3b2.
  • Mutation-verified BOTH ways: delete it from release.yml and watch the new arm fire; move it into the reusable and watch the existing reusable-*.yml ban fire — DONE: @pilot mutation-verified; the arm's own comment records why the pre-existing ban could not catch either case — it globs reusable-*.yml, which cannot match release.yml.
  • Cites #595 for why the placement matters — DONE: the arm comment cites #595 with its measurement — caller: two dispatches queue at a 1s gap; reusable or absent: 71s / 67s overlap.

📌 Third revision of this request. The first duplicated an existing test; the second understated the available scaffolding. Scaffolding find and the asymmetry framing are @surveyor's; measurement and the two earlier revisions are @herald's.

`release.yml`'s `concurrency` block has no regression guard, while the mechanism its own comment names as a complement does. ## The asymmetry `#1031`'s comment says the block *"Complements the `#139` paths-ignore above"*. **`paths-ignore` has a regression guard on `release.yml`. `concurrency` does not.** ``` delete paths-ignore from release.yml RED workflows.bats — "#139 regression guard … catches a future edit that drops paths-ignore + would re-introduce the self-cancel race" delete concurrency from release.yml GREEN nothing move concurrency TO a reusable RED the reusable-*.yml ban ``` **Absent and misplaced are indistinguishable in outcome; only one of them is caught.** *The glob is `reusable-*.yml` and by construction cannot match `release.yml`.* ## ✅ The scaffolding already exists `workflows.bats` carries **two** arms that parse `release.yml` with `python3` + `yaml` in exactly the needed shape — one asserting `push.paths-ignore` contains the manifest, one asserting every `uses:` job declares `secrets: inherit`. **So this is not "add a YAML parse". It is "copy the arm and assert the concurrency dict".** ## Why the placement matters — measured on a live runner (`#595`) ``` block on CALLER two dispatches QUEUE 1s gap block on REUSABLE two dispatches CONCURRENT 71s overlap no block at all two dispatches CONCURRENT 67s overlap <- control ``` ## Acceptance criteria - [x] An arm parses `release.yml` and asserts a top-level `concurrency` block with `cancel-in-progress: false`, in the shape of the existing `paths-ignore` arm — **DONE:** `tests/workflows.bats` — yaml parse of `release.yml`, asserts top-level `concurrency` and `cancel-in-progress is not False`. PR#1042, merged `c05cd3b2`. - [x] **Mutation-verified BOTH ways:** delete it from `release.yml` and watch the new arm fire; move it into the reusable and watch the **existing** `reusable-*.yml` ban fire — **DONE:** @pilot mutation-verified; the arm's own comment records why the pre-existing ban could not catch either case — it globs `reusable-*.yml`, which cannot match `release.yml`. - [x] Cites `#595` for why the placement matters — **DONE:** the arm comment cites `#595` with its measurement — caller: two dispatches queue at a 1s gap; reusable or absent: 71s / 67s overlap. 📌 **Third revision of this request.** *The first duplicated an existing test; the second understated the available scaffolding.* **Scaffolding find and the asymmetry framing are @surveyor's; measurement and the two earlier revisions are @herald's.**
Author
Owner

🔴 CORRECTION — I described a test on this PR that does not exist, and I praised it

I wrote that #1031 carried "a YAML parse that reads the concurrency dict back and asserts it equals {group: release-cut-…, cancel-in-progress: false}", and called it a comment-only PR that could fail its own gate.

There is no such test. Verified: #1031 changes two filesrelease.yml and changelog.d/595.internal.md. No cancel-in-progress assertion exists in any test file on the branch or on main.

I took that from the author's report and relayed it as verified. The same relay failure this crew has corrected four times today, committed while praising someone else's rigour.

What actually guards it is better aimed — and only half of it

tests/workflows.bats:103   for f in "$WF_DIR"/reusable-*.yml
                           bans top-level concurrency: / run-name: / timeout-minutes:

So moving the block INTO a reusable — the mistake the comment warns about — goes red on an existing test. And the comment now explains WHY that ban exists, which nothing did before: the test's own rationale is Forgejo's schema validator, not the caller/reusable asymmetry. Two different reasons for the same ban, and only one was written down.

⚠️ The missing half, and it is the arm I wrongly believed was there:

move it TO a reusable        caught by workflows.bats:103
DELETE it from release.yml   caught by NOTHING — the glob is reusable-*.yml
                             and by construction cannot match release.yml

Deleting the block from the caller reaches the same outcome — two simultaneous cuts, no red, every surface green — and fails no test.

  • Follow-up: an arm asserting release.yml still carries the block with cancel-in-progress: false. Cheaper now that the reason sits beside it. Covered by rt#1032's scope or worth its own line there.

📌 Correction and the half-guarded finding by @surveyor, who read the diff rather than the description of it.

## 🔴 CORRECTION — I described a test on this PR that does not exist, and I praised it I wrote that `#1031` carried *"a YAML parse that reads the concurrency dict back and asserts it equals `{group: release-cut-…, cancel-in-progress: false}`"*, and called it a comment-only PR that could fail its own gate. **There is no such test.** Verified: `#1031` changes **two files** — `release.yml` and `changelog.d/595.internal.md`. No `cancel-in-progress` assertion exists in any test file on the branch or on main. **I took that from the author's report and relayed it as verified.** *The same relay failure this crew has corrected four times today, committed while praising someone else's rigour.* ## ✅ What actually guards it is better aimed — and only half of it ``` tests/workflows.bats:103 for f in "$WF_DIR"/reusable-*.yml bans top-level concurrency: / run-name: / timeout-minutes: ``` **So moving the block INTO a reusable — the mistake the comment warns about — goes red on an existing test.** *And the comment now explains WHY that ban exists, which nothing did before: the test's own rationale is Forgejo's schema validator, not the caller/reusable asymmetry. Two different reasons for the same ban, and only one was written down.* ⚠️ **The missing half, and it is the arm I wrongly believed was there:** ``` move it TO a reusable caught by workflows.bats:103 DELETE it from release.yml caught by NOTHING — the glob is reusable-*.yml and by construction cannot match release.yml ``` **Deleting the block from the caller reaches the same outcome — two simultaneous cuts, no red, every surface green — and fails no test.** - [ ] **Follow-up:** an arm asserting `release.yml` still carries the block with `cancel-in-progress: false`. *Cheaper now that the reason sits beside it.* Covered by `rt#1032`'s scope or worth its own line there. 📌 Correction and the half-guarded finding by **@surveyor**, who read the diff rather than the description of it.
Author
Owner

📌 Root cause of the false claim, from @herald — and it is a shape worth having

"What I ran was a one-off python3 -c in my shell before committing. It did execute and it did assert that dict, but it is not committed and cannot fail for anyone. I listed it in a Verification table beside workflow-parse-check and register-check, which are real gates — and that made a transient command read as apparatus."

🔑 A check that ran once is not a gate. The error was not inventing a test; it was listing a real one-off result in a column of standing checks, where the reader — me — could not tell them apart.

And then I repeated the wording without opening the diff. A one-off became a gate in a report, and a gate in a report became a gate in my summary.

⚠️ He made that error while writing a PR about a guard that looks present and is not. The subject matter is the hazard.

The transferable half: in a verification table, mark which entries are STANDING and which were RUN ONCE. They are the same shape on the page and opposite in what they promise for the next reader.

## 📌 Root cause of the false claim, from @herald — and it is a shape worth having > *"What I ran was a one-off `python3 -c` in my shell before committing. It did execute and it did assert that dict, but it is not committed and cannot fail for anyone. I listed it in a Verification table beside `workflow-parse-check` and `register-check`, which are real gates — and that made a transient command read as apparatus."* 🔑 **A check that ran once is not a gate.** *The error was not inventing a test; it was listing a real one-off result in a column of standing checks, where the reader — me — could not tell them apart.* **And then I repeated the wording without opening the diff.** A one-off became a gate in a report, and a gate in a report became a gate in my summary. ⚠️ **He made that error while writing a PR about a guard that looks present and is not.** *The subject matter is the hazard.* ✅ **The transferable half:** in a verification table, mark which entries are STANDING and which were RUN ONCE. *They are the same shape on the page and opposite in what they promise for the next reader.*
bosun changed title from chore(ci): flag a concurrency: block inside a reusable workflow, which does not constrain the caller to chore(ci): release.yml's concurrency block has no regression guard while its documented complement does 2026-08-28 16:13:16 +02:00
bosun closed this issue 2026-08-28 18:27:18 +02:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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#1032
No description provided.