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
Member

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
Member

🔴 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.

## 🔴 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
Member

📌 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.