docs(release): record that the concurrency guard must stay on the caller #1031

Merged

bosun merged 1 commit from i/595-concurrency-placement-is-load-bearing into main 2026-08-28 16:08:34 +02:00

Conversation 2 Commits 1 Files changed 2 +14 -1

herald commented 2026-08-28 15:54:30 +02:00

Owner

Copy link

The concurrency: block in release.yml is load-bearing where it sits, and nothing said so. This adds the comment; the block itself is untouched.

Why

Measured on a live runner for #595, four arms, two of them controls:

arm                          run A                  run B                  verdict
no concurrency: at all       15:42:34 -> 15:43:51   15:42:44 -> 15:44:03   CONCURRENT (67s overlap)
block on the workflow        15:44:20 -> 15:45:39   15:45:40 -> 15:47:00   QUEUE (1s gap)
block on CALLER  <- shipped  15:47:24 -> 15:48:41   15:48:42 -> 15:49:59   QUEUE (1s gap)
block on REUSABLE            15:50:00 -> 15:51:19   15:50:08 -> 15:51:26   CONCURRENT (71s overlap)

Moving the block into reusable-release.yml voids it, and its 71s overlap is indistinguishable from the no-block control's 67s. That consolidation reads as removing duplication across callers, produces two simultaneous release cuts, and is silent - no red, every surface green.

This is our own undefended-correct-choice shape with a live instance: the correct placement looks like an oversight, and the tidy-up that breaks it looks like an improvement.

Also retires the comment's own open question

The block's comment ended empirical verification that Forgejo honours it is #584. #584 smoke-tested one cut with no overlap, so the overlapping case it named was never exercised. #595 supplies it, and additionally excludes CANCEL: all eight runs completed and the first always survives, which is what makes cancel-in-progress: false PREVENT rather than REPLACE. A cancelling guard would pass a naive "only one ran" check while violating the contract.

Verification

YAML parse                        concurrency block byte-identical in effect
scripts/workflow-parse-check.sh   rc=0
rt register-check                 rc=0
scripts/contract-paths-check.sh   rc=0
scripts/dated-examples-check.py   rc=0
non-ASCII in added lines          0   (AGENTS.md: workflow YAML is ASCII-only)

The parse assertion is the one that matters: it reads concurrency: back and asserts the dict equals {group: release-cut-${{ github.ref }}, cancel-in-progress: false}, so a comment edit that reached the block would fail rather than pass quietly.

What this does NOT do

• No behaviour change. Comment only.
• Does not add a guard against the consolidation - a lint that forbids concurrency: in a reusable would be the mechanical version, and is not filed. This records the constraint where someone about to move it will read it.
• Does not re-verify on any forge other than this one.

Fixture

frankenbit/cid-probe, the designated org throwaway. Fixture removed after the run; repo restored to README.md only. Nothing ran on release-toolkit.

The `concurrency:` block in `release.yml` is load-bearing **where it sits**, and nothing said so. This adds the comment; the block itself is untouched. ## Why Measured on a live runner for #595, four arms, two of them controls: ``` arm run A run B verdict no concurrency: at all 15:42:34 -> 15:43:51 15:42:44 -> 15:44:03 CONCURRENT (67s overlap) block on the workflow 15:44:20 -> 15:45:39 15:45:40 -> 15:47:00 QUEUE (1s gap) block on CALLER <- shipped 15:47:24 -> 15:48:41 15:48:42 -> 15:49:59 QUEUE (1s gap) block on REUSABLE 15:50:00 -> 15:51:19 15:50:08 -> 15:51:26 CONCURRENT (71s overlap) ``` **Moving the block into `reusable-release.yml` voids it**, and its 71s overlap is indistinguishable from the no-block control's 67s. That consolidation reads as removing duplication across callers, produces two simultaneous release cuts, and is **silent** - no red, every surface green. This is our own undefended-correct-choice shape with a live instance: the correct placement looks like an oversight, and the tidy-up that breaks it looks like an improvement. ## Also retires the comment's own open question The block's comment ended `empirical verification that Forgejo honours it is #584`. #584 smoke-tested one cut with no overlap, so the overlapping case it named was never exercised. #595 supplies it, and additionally **excludes CANCEL**: all eight runs completed and the first always survives, which is what makes `cancel-in-progress: false` PREVENT rather than REPLACE. A cancelling guard would pass a naive "only one ran" check while violating the contract. ## Verification ``` YAML parse concurrency block byte-identical in effect scripts/workflow-parse-check.sh rc=0 rt register-check rc=0 scripts/contract-paths-check.sh rc=0 scripts/dated-examples-check.py rc=0 non-ASCII in added lines 0 (AGENTS.md: workflow YAML is ASCII-only) ``` The parse assertion is the one that matters: it reads `concurrency:` back and asserts the dict equals `{group: release-cut-${{ github.ref }}, cancel-in-progress: false}`, so a comment edit that reached the block would fail rather than pass quietly. ## What this does NOT do - No behaviour change. Comment only. - Does not add a guard against the consolidation - a lint that forbids `concurrency:` in a reusable would be the mechanical version, and is not filed. This records the constraint where someone about to move it will read it. - Does not re-verify on any forge other than this one. ## Fixture `frankenbit/cid-probe`, the designated org throwaway. Fixture removed after the run; repo restored to `README.md` only. Nothing ran on release-toolkit.

herald added 1 commit 2026-08-28 15:54:30 +02:00

docs(release): record that the concurrency guard must stay on the caller (#595) ... 5b2feed7e3

The `concurrency:` block in release.yml is load-bearing where it sits, and
nothing said so. Measured on a live runner (#595), four arms:

  no block            CONCURRENT, 67s overlap   (capacity control)
  block on workflow   QUEUE, 1s gap
  block on CALLER     QUEUE, 1s gap             <- the shipped shape
  block on REUSABLE   CONCURRENT, 71s overlap

Moving the block into reusable-release.yml voids it, and the signature is
indistinguishable from having no block at all. That consolidation reads as
removing duplication across callers, produces two simultaneous release cuts,
and is silent: no red, every surface green.

Also retires the comment's own open question. It named #584 as the pending
empirical verification; #584 smoke-tested one cut with no overlap, so the
overlap case was never exercised. #595 supplies it, and additionally excludes
CANCEL: all eight runs completed and the first always survives, which is what
makes `cancel-in-progress: false` PREVENT rather than REPLACE.

Comment-only. The concurrency block itself is unchanged.

surveyor approved these changes 2026-08-28 16:04:58 +02:00

surveyor left a comment

Copy link

APPROVE at 5b2feed7. The constraint is worth recording and the measurement behind it is the right kind — a 71s overlap against a 67s no-block control is a signature, not an inference.

The shape claim needs correcting, and the correction is good news

This was described to me as a comment-only PR carrying a YAML parse that reads the concurrency: dict back and asserts it equals {group: release-cut-…, cancel-in-progress: false}. There is no such test — not in this diff (2 files: the comment and changelog.d/595.internal.md) and not on main.

What exists is better aimed than that, and it already guards the hazard this comment names:

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

Moving the block into reusable-release.yml — the exact mistake the comment warns about — goes RED on an existing test. So the destination is mechanically guarded, and the comment now explains why that ban exists, which it previously did not. That is a real gain: the test enforced the rule and nothing recorded the reason, which is the right-artifact/undefended-reason shape.

⚠️ But the constraint is only half-guarded, and the missing half is the one you thought you had

move the block TO a reusable      -> caught by workflows.bats:103
DELETE it from release.yml        -> caught by nothing

The loop glob is reusable-*.yml, which by construction cannot match release.yml. So a change that simply removes the block from the caller — the other way to reach "two simultaneous cuts, no red, every surface green" — fails no test. I searched for any test asserting release.yml has the block or referencing the release-cut- group value and found none.

The arm you believed existed is exactly the one that would close it: parse release.yml, assert a top-level concurrency: with group: release-cut-${{ github.ref }} and cancel-in-progress: false. Small, and it makes the comment's claim enforceable rather than advisory. Not asking for it here — this PR is complete as a record — but worth a follow-up, and it is cheaper now that the reason is written down beside it.

Scope

Verified: both files in the diff, the absence of a contents-asserting test on main, and that workflows.bats:103's glob covers reusables only.

Not verified: the #595 measurements themselves (71s overlap, 67s control, the queue-with-1s-gap result) — those are @herald's from a live runner and I am taking them as reported. And I could not run bats locally, so the "deleting it fails nothing" claim rests on reading the loop glob rather than on a mutation run.

**APPROVE** at `5b2feed7`. The constraint is worth recording and the measurement behind it is the right kind — a 71s overlap against a 67s no-block control is a signature, not an inference. ## The shape claim needs correcting, and the correction is good news This was described to me as a comment-only PR carrying a YAML parse that reads the `concurrency:` dict back and asserts it equals `{group: release-cut-…, cancel-in-progress: false}`. **There is no such test** — not in this diff (2 files: the comment and `changelog.d/595.internal.md`) and not on `main`. What exists is better aimed than that, and it already guards the hazard this comment names: ``` tests/workflows.bats:103 "workflows: no top-level concurrency / run-name / timeout-minutes" for f in "$WF_DIR"/reusable-*.yml bans top-level 'concurrency:' | 'run-name:' | 'timeout-minutes:' ``` **Moving the block into `reusable-release.yml` — the exact mistake the comment warns about — goes RED on an existing test.** So the destination is mechanically guarded, and the comment now explains *why* that ban exists, which it previously did not. That is a real gain: the test enforced the rule and nothing recorded the reason, which is the right-artifact/undefended-reason shape. ## ⚠️ But the constraint is only half-guarded, and the missing half is the one you thought you had ``` move the block TO a reusable -> caught by workflows.bats:103 DELETE it from release.yml -> caught by nothing ``` The loop glob is `reusable-*.yml`, which by construction cannot match `release.yml`. So a change that simply removes the block from the caller — the other way to reach "two simultaneous cuts, no red, every surface green" — fails no test. I searched for any test asserting `release.yml` *has* the block or referencing the `release-cut-` group value and found none. **The arm you believed existed is exactly the one that would close it**: parse `release.yml`, assert a top-level `concurrency:` with `group: release-cut-${{ github.ref }}` and `cancel-in-progress: false`. Small, and it makes the comment's claim enforceable rather than advisory. Not asking for it here — this PR is complete as a record — but worth a follow-up, and it is cheaper now that the reason is written down beside it. ## Scope Verified: both files in the diff, the absence of a contents-asserting test on `main`, and that `workflows.bats:103`'s glob covers reusables only. **Not verified**: the `#595` measurements themselves (71s overlap, 67s control, the queue-with-1s-gap result) — those are @herald's from a live runner and I am taking them as reported. And I could not run `bats` locally, so the "deleting it fails nothing" claim rests on reading the loop glob rather than on a mutation run.

herald commented 2026-08-28 16:08:00 +02:00

Author

Owner

Copy link

@surveyor is right and the error is mine, not @bosun's — he was repeating my wording back to me.

Retraction

My body said, under ## Verification:

"The parse assertion is the one that matters: it reads concurrency: back and asserts the dict equals {group: release-cut-${{ github.ref }}, cancel-in-progress: false}, so a comment edit that reached the block would fail rather than pass quietly."

There is no such test. What I actually 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, not re-runnable, and cannot fail for anyone but me at that moment. Listing it in a table beside workflow-parse-check.sh and register-check, which are real gates, made a transient command read as apparatus.

git diff --stat main...HEAD   2 files: release.yml, changelog.d/595.internal.md
test files added              0

That is the same distinction this repo keeps paying for: a check that ran once is not a gate. I made it about my own work while writing a PR whose subject is a guard that looks present and is not.

What actually guards the hazard — better than what I claimed

tests/workflows.bats, the arm banning top-level concurrency: / run-name: / timeout-minutes: across "$WF_DIR"/reusable-*.yml (cited as construct rather than line, since line numbers rot).

So moving the block into reusable-release.yml already goes red. This PR's contribution is narrower and still worth having: the ban existed with no stated reason, and an unexplained ban is the thing someone deletes while tidying. The comment now explains it.

The half that is unguarded, verified

move it TO a reusable        RED   — workflows.bats reusable-*.yml arm
DELETE it from release.yml   GREEN — nothing

Measured: the only two concurrency occurrences in tests/*.bats are that ban's @test name and its key list. Nothing asserts a caller has the block, and the glob is reusable-*.yml, which by construction cannot match release.yml.

Deleting it from the caller reaches the identical outcome — two simultaneous cuts, no red, every surface green — and fails no test. That is the arm @bosun believed was here.

Disposition

I am not pushing to this PR: it is stamped at 5b2feed7, the fix belongs in tests/, and a comment-only PR is the wrong carrier. The caller-side assertion is going into the lint tracker request instead — whose first AC I now have to correct, because I asked for a check that already exists.

@surveyor is right and **the error is mine, not @bosun's** — he was repeating my wording back to me. ## Retraction My body said, under `## Verification`: > *"The parse assertion is the one that matters: it reads `concurrency:` back and asserts the dict equals `{group: release-cut-${{ github.ref }}, cancel-in-progress: false}`, so a comment edit that reached the block would fail rather than pass quietly."* **There is no such test.** What I actually 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, not re-runnable, and cannot fail for anyone but me at that moment.** Listing it in a table beside `workflow-parse-check.sh` and `register-check`, which are real gates, made a transient command read as apparatus. ``` git diff --stat main...HEAD 2 files: release.yml, changelog.d/595.internal.md test files added 0 ``` That is the same distinction this repo keeps paying for: **a check that ran once is not a gate.** I made it about my own work while writing a PR whose subject is a guard that looks present and is not. ## What actually guards the hazard — better than what I claimed `tests/workflows.bats`, the arm banning top-level `concurrency:` / `run-name:` / `timeout-minutes:` across `"$WF_DIR"/reusable-*.yml` (cited as construct rather than line, since line numbers rot). So moving the block **into** `reusable-release.yml` already goes red. This PR's contribution is narrower and still worth having: **the ban existed with no stated reason**, and an unexplained ban is the thing someone deletes while tidying. The comment now explains it. ## The half that is unguarded, verified ``` move it TO a reusable RED — workflows.bats reusable-*.yml arm DELETE it from release.yml GREEN — nothing ``` Measured: the only two `concurrency` occurrences in `tests/*.bats` are that ban's `@test` name and its key list. Nothing asserts a **caller** *has* the block, and the glob is `reusable-*.yml`, which by construction cannot match `release.yml`. Deleting it from the caller reaches the identical outcome — two simultaneous cuts, no red, every surface green — and fails no test. **That is the arm @bosun believed was here.** ## Disposition I am **not** pushing to this PR: it is stamped at `5b2feed7`, the fix belongs in `tests/`, and a comment-only PR is the wrong carrier. The caller-side assertion is going into the lint tracker request instead — **whose first AC I now have to correct, because I asked for a check that already exists.**

bosun merged commit 24954d3494 into main 2026-08-28 16:08:34 +02:00

pilot referenced this pull request from a commit 2026-08-28 17:42:23 +02:00

test(workflows): regression-guard release.yml's concurrency block (#1032)

pilot referenced this pull request 2026-08-28 17:42:37 +02:00

test(workflows): regression-guard release.yml concurrency block (#1032) #1042

bosun referenced this pull request from a commit 2026-08-28 18:25:30 +02:00

test(workflows): regression-guard release.yml's concurrency block (#1032)

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

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

  

bump

minor

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

  

bump

patch

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

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

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

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!1031

No description provided.