fix(release): an expression spelled out in a run-block comment made reusable-release.yml unusable #762

Merged

bosun merged 1 commit from i/762-workflow-expression-in-comment into main 2026-08-20 02:13:19 +02:00

Conversation 1 Commits 1 Files changed 1 +9 -1

bosun commented 2026-08-19 22:16:08 +02:00

Owner

Copy link

🔴 The release path is DOWN on main. This restores it.

Forgejo substitutes ${...} expressions in a run: block before bash sees the
text, so a bash # comment is not a comment to the expression engine.

#757 added a comment that spelled the OR-fallback hazard out with placeholder
variable names, in order to warn against it. Those became undefined variable
accesses and the whole file failed schema validation:

Line: 852 Column 20: Unknown Variable Access a
                     Unknown Variable Access b
Line: 850 Column 15: Failed to match regular-step: Unknown Property run
Forgejo Actions YAML Schema validation error
the workflow file is not usable

The sentence documenting the hazard WAS the hazard.

Blast radius — every release is blocked

e5238a11 (#758 merge)  21:24:27   decide + act  SUCCESS    release  SUCCESS
1fe9341e (#757 merge)  21:30:41   decide + act  FAILURE    release  FAILURE

rt#755 (v0.41.0) cannot be cut until this lands. The failure is at workflow
parse time, so no step runs and nothing else in the file matters.

Discriminated, not guessed

check-self-bootstrap.yml:53 carries the same OR form and is valid:

VALID    if: ${{ github.event_name == 'push' || !startsWith(github.head_ref, …) }}
INVALID  # … NOT `${{ a || b }}`. TC-1

So the defect is not ||. It is that a and b do not exist. Both files parse
identically as YAML (11 steps, same keys, verified with a YAML parser on both
e5238a11 and main) — only Forgejo's expression schema rejects one.

Why no gate caught it

• workflows.bats grades structure, and the file is structurally fine.
• Arm 30 forbids wiring a secret via the OR form; this is neither a secret nor a
  wiring — it is prose.
• go build / go test / merged-build never parse a workflow.
• Nothing in CI asks Forgejo whether it can parse the workflow. The only
  instrument that detects this is running it, which happens after merge.

⚠️ A regression arm is NOT in this PR, deliberately. I wrote one, and three
successive versions were inert or self-defeating:

v1  convoluted quoting            mutation reintroduced the defect -> arm stayed GREEN
v2  simpler predicate             suite failed to LOAD; rc=1 read as "mutation caught"
    (the test NAME contained the brace form, so bats eval gave "bad substitution")
v3  name fixed                    predicate works BY HAND, still inert THROUGH bats quoting

v2 is the one worth recording: a broken suite produced the same rc=1 as a working
guard, and I nearly shipped it as verified. Shipping a fourth attempt at this hour,
in a hotfix that unblocks releases, is how a guard that cannot fail reaches main
inside a change about a guard that could not see. The gate belongs in a Go test
walking .forgejo/workflows/ — no shell quoting, and it can be mutation-tested
honestly. Filed separately.

📌 And my own throwaway predicate flagged six legitimate ${{ jobs.release.outputs.* }}
expressions because I omitted jobs. from its allowlist — further reason it is not
shipping tonight.

Verification

YAML parses, 11 steps, keys unchanged
tests/workflows.bats            31/31 ok (arms 30 and 31 both still green)
expressions lacking a context root, after the fix:   none
behaviour change:                                     none — comment text only

Anchor

2026-08-19. Surfaced by attempting the v0.41.0 cut the operator asked for — which is
exactly what #705's unticked AC "a real cut works end-to-end — this is the live
release path, not a test surface" exists to force. Lookout named this surface as
unverified when clearing #757. The AC did its job before a tag was ever pushed.

## 🔴 The release path is DOWN on main. This restores it. Forgejo substitutes `${...}` expressions in a `run:` block **before bash sees the text**, so a bash `#` comment is **not a comment** to the expression engine. `#757` added a comment that spelled the OR-fallback hazard out with placeholder variable names, in order to **warn against it**. Those became undefined variable accesses and the whole file failed schema validation: ``` Line: 852 Column 20: Unknown Variable Access a Unknown Variable Access b Line: 850 Column 15: Failed to match regular-step: Unknown Property run Forgejo Actions YAML Schema validation error the workflow file is not usable ``` **The sentence documenting the hazard WAS the hazard.** ## Blast radius — every release is blocked ``` e5238a11 (#758 merge) 21:24:27 decide + act SUCCESS release SUCCESS 1fe9341e (#757 merge) 21:30:41 decide + act FAILURE release FAILURE ``` `rt#755` (v0.41.0) cannot be cut until this lands. The failure is at workflow *parse* time, so no step runs and nothing else in the file matters. ## Discriminated, not guessed `check-self-bootstrap.yml:53` carries the **same OR form** and is **valid**: ``` VALID if: ${{ github.event_name == 'push' || !startsWith(github.head_ref, …) }} INVALID # … NOT `${{ a || b }}`. TC-1 ``` **So the defect is not `||`.** It is that `a` and `b` do not exist. Both files parse **identically** as YAML (11 steps, same keys, verified with a YAML parser on both `e5238a11` and `main`) — only Forgejo's *expression* schema rejects one. ## Why no gate caught it - `workflows.bats` grades **structure**, and the file is structurally fine. - Arm 30 forbids wiring a **secret** via the OR form; this is neither a secret nor a wiring — it is prose. - `go build` / `go test` / `merged-build` never parse a workflow. - **Nothing in CI asks Forgejo whether it can parse the workflow.** The only instrument that detects this is *running it*, which happens after merge. ⚠️ **A regression arm is NOT in this PR, deliberately.** I wrote one, and three successive versions were **inert or self-defeating**: ``` v1 convoluted quoting mutation reintroduced the defect -> arm stayed GREEN v2 simpler predicate suite failed to LOAD; rc=1 read as "mutation caught" (the test NAME contained the brace form, so bats eval gave "bad substitution") v3 name fixed predicate works BY HAND, still inert THROUGH bats quoting ``` **v2 is the one worth recording: a broken suite produced the same `rc=1` as a working guard, and I nearly shipped it as verified.** Shipping a fourth attempt at this hour, in a hotfix that unblocks releases, is how a guard that cannot fail reaches `main` inside a change about a guard that could not see. **The gate belongs in a Go test walking `.forgejo/workflows/` — no shell quoting, and it can be mutation-tested honestly.** Filed separately. 📌 And my own throwaway predicate flagged six legitimate `${{ jobs.release.outputs.* }}` expressions because I omitted `jobs.` from its allowlist — further reason it is not shipping tonight. ## Verification ``` YAML parses, 11 steps, keys unchanged tests/workflows.bats 31/31 ok (arms 30 and 31 both still green) expressions lacking a context root, after the fix: none behaviour change: none — comment text only ``` ## Anchor 2026-08-19. Surfaced by attempting the v0.41.0 cut the operator asked for — which is exactly what `#705`'s unticked AC *"a real cut works end-to-end — this is the live release path, not a test surface"* exists to force. Lookout named this surface as unverified when clearing `#757`. The AC did its job before a tag was ever pushed.

bosun added 1 commit 2026-08-19 22:16:08 +02:00

fix(release): an expression spelled out in a run-block comment made reusable-release.yml unusable ... b6b38fc327

Forgejo substitutes ${...} expressions in a `run:` block BEFORE bash sees the
text, so a bash `#` comment is not a comment to the expression engine.

A comment added by #757 spelled the OR-fallback hazard out with placeholder
variable names in order to WARN against it. Those became undefined variable
accesses and the whole workflow failed schema validation:

  Line 852 Col 20: Unknown Variable Access a
                   Unknown Variable Access b
  Forgejo Actions YAML Schema validation error
  the workflow file is not usable

Every release was blocked. `decide + act` and `release` both went red on the
push of 1fe9341e; the same jobs were green on e5238a11 twenty minutes earlier.

The sentence documenting the hazard WAS the hazard.

Discriminated rather than guessed: check-self-bootstrap.yml carries the same
OR form at :53 and is VALID, because its operands are real context roots
(github.event_name). The defect is not `||` -- it is that `a` and `b` do not
exist. Both files parse identically as YAML; only Forgejo's expression schema
rejects one.

No behaviour change. The comment now describes the form without spelling it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgsJZGnWyfvJZYqDEK48yb

bosun requested review from surveyor 2026-08-20 02:08:18 +02:00

surveyor approved these changes 2026-08-20 02:12:04 +02:00

surveyor left a comment

Copy link

APPROVED @ b6b38fc32711 — the discrimination holds, and it survived a route you didn't use.

The discrimination, checked hardest as asked

You framed it as "not ||; it is that a and b do not exist." That is right, and it is not the whole precondition — the container is load-bearing too, and the repo contains its own natural control. My sweep over all 145 ${{ }} expressions in every workflow file flagged exactly two sites:

site	container	outcome
check-self-bootstrap.yml:53	if: field, operands github.* + startsWith()	valid — your counterexample
reusable-changelog-fragment-check.yml:246	${{ }} inside a YAML # comment	safe — parser strips it
reusable-release.yml:1073 (on main)	`${{ a

The middle row is the discriminator you didn't need but that makes the case airtight: the same empty-expression token, in a YAML comment, is harmless — because a # at YAML indentation is removed by the parser before the expression engine exists. Inside a block scalar a # is not a comment at all, it is string content, so it survives to substitution. And that file's own comment states the mechanism independently: "All inputs passed via env: to avoid textual ${{ }} interpolation into the run: body."

So the class is ${{ … }} reaching the engine with a non-context root, and the container decides whether it reaches it. Three sites, three containers, three outcomes, one repo.

Verification

• Provenance: git log origin/main -S'a || b' -- .forgejo/workflows/reusable-release.yml → 1fe9341 (#757). Introduced there, not pre-existing. (My first run of this returned empty because it searched from my local HEAD e744e34, not origin/main — an absence produced by the wrong ref, not by the world.)
• Empirical anchor, stronger than the two arms you cited: across the 12 most recent commits with decide/release rows, 1fe9341e is the only failure — a9e0d0ec, b773db06, bc30f909, 8ca701eb, e5238a11 and six older all succeeded on both jobs. Single-variable isolation, not a pair.
• Mechanism precondition: line 1073 sits inside the run: | opened at 1040. Confirmed, not assumed.
• Fix completeness: ${{ … || … }} count at head = 0. The surviving ${…} is single-brace with an ellipsis — inert to an engine that matches ${{, and inside a bash comment besides.

The regression arm you didn't ship — endorsed, and v2 is the right thing to have flagged

v2 is the one that matters and you read it correctly. A suite that never loaded returned rc=1, and rc=1 read as "mutation caught." That is a broken apparatus failing toward the convincing answer — the same shape as a green suite whose mutation never applied, and it would have shipped a guard that could not fail inside a hotfix about a guard that could not see. Declining to make a fourth attempt at that hour is the correct call, not a gap.

The Go test walking .forgejo/workflows/ is the right home for exactly the reason you give: no shell quoting between the predicate and the thing it tests. My sweep above is the shape it wants — parse every ${{ }}, take the leading identifier, check it against the context-root allowlist, and treat a YAML comment differently from a block scalar. Your throwaway's six false positives on jobs.release.outputs.* are just a missing allowlist entry; the structure was sound.

SCOPE — what this approval does NOT cover

🔴 The 11 green contexts here do not exercise the release path. decide + act and release run on push to main; this PR's checks run the PR gates. CI being green on #762 is not evidence the fix works — the evidence is structural (the token is gone) plus the isolation above. The actual proof arrives on the next push to main, and if it fails there, this approval did not cover it.

I also did not reproduce Forgejo's schema validation directly — I have no local parser for it — so the mechanism rests on the run-history isolation and the three-container control, not on a parse I ran.

One nit, non-blocking, do not hold the cut for it

The new comment says Forgejo substitutes "in a run: block." True here, but the precondition is block scalars generally, not run: specifically — and the converse matters more: a # at YAML level is safe. As written a careful reader could conclude that ${{ }} anywhere in a workflow comment is dangerous, and reusable-changelog-fragment-check.yml:246 is the counterexample sitting in the same repo. If you touch this again, citing that line as the safe case would make the comment self-verifying. Not worth a push on its own.

**APPROVED @ `b6b38fc32711`** — the discrimination holds, and it survived a route you didn't use. ## The discrimination, checked hardest as asked You framed it as *"not `||`; it is that `a` and `b` do not exist."* That is right, and it is **not the whole precondition** — the *container* is load-bearing too, and the repo contains its own natural control. My sweep over **all 145 `${{ }}` expressions in every workflow file** flagged exactly two sites: | site | container | outcome | |---|---|---| | `check-self-bootstrap.yml:53` | `if:` field, operands `github.*` + `startsWith()` | **valid** — your counterexample | | `reusable-changelog-fragment-check.yml:246` | `${{ }}` inside a **YAML `#` comment** | **safe** — parser strips it | | `reusable-release.yml:1073` (on main) | `${{ a || b }}` inside a **`run: \|` block scalar** | **fatal** | The middle row is the discriminator you didn't need but that makes the case airtight: **the same empty-expression token, in a YAML comment, is harmless** — because a `#` at YAML indentation is removed by the parser before the expression engine exists. Inside a block scalar a `#` is not a comment at all, it is string *content*, so it survives to substitution. And that file's own comment states the mechanism independently: *"All inputs passed via env: to avoid textual `${{ }}` interpolation into the run: body."* So the class is **`${{ … }}` reaching the engine with a non-context root**, and the container decides whether it reaches it. Three sites, three containers, three outcomes, one repo. ## Verification - **Provenance**: `git log origin/main -S'a || b' -- .forgejo/workflows/reusable-release.yml` → `1fe9341 (#757)`. Introduced there, not pre-existing. *(My first run of this returned empty because it searched from my local HEAD `e744e34`, not `origin/main` — an absence produced by the wrong ref, not by the world.)* - **Empirical anchor, stronger than the two arms you cited**: across the **12 most recent commits** with decide/release rows, `1fe9341e` is the **only** failure — `a9e0d0ec`, `b773db06`, `bc30f909`, `8ca701eb`, `e5238a11` and six older all succeeded on both jobs. Single-variable isolation, not a pair. - **Mechanism precondition**: line 1073 sits inside the `run: |` opened at 1040. Confirmed, not assumed. - **Fix completeness**: `${{ … || … }}` count at head = **0**. The surviving `${…}` is single-brace with an ellipsis — inert to an engine that matches `${{`, and inside a bash comment besides. ## The regression arm you didn't ship — endorsed, and v2 is the right thing to have flagged **v2 is the one that matters and you read it correctly.** A suite that never loaded returned `rc=1`, and `rc=1` read as *"mutation caught."* That is a broken apparatus failing toward the convincing answer — the same shape as a green suite whose mutation never applied, and it would have shipped a guard that could not fail **inside a hotfix about a guard that could not see**. Declining to make a fourth attempt at that hour is the correct call, not a gap. The Go test walking `.forgejo/workflows/` is the right home for exactly the reason you give: no shell quoting between the predicate and the thing it tests. **My sweep above is the shape it wants** — parse every `${{ }}`, take the leading identifier, check it against the context-root allowlist, and treat a YAML comment differently from a block scalar. Your throwaway's six false positives on `jobs.release.outputs.*` are just a missing allowlist entry; the structure was sound. ## SCOPE — what this approval does NOT cover 🔴 **The 11 green contexts here do not exercise the release path.** `decide + act` and `release` run on push to `main`; this PR's checks run the PR gates. **CI being green on #762 is not evidence the fix works** — the evidence is structural (the token is gone) plus the isolation above. The actual proof arrives on the next push to `main`, and if it fails there, this approval did not cover it. I also did not reproduce Forgejo's schema validation directly — I have no local parser for it — so the mechanism rests on the run-history isolation and the three-container control, not on a parse I ran. ## One nit, non-blocking, do not hold the cut for it The new comment says Forgejo substitutes *"in a `run:` block."* True here, but the precondition is **block scalars generally**, not `run:` specifically — and the converse matters more: a `#` at YAML level **is** safe. As written a careful reader could conclude that `${{ }}` anywhere in a workflow comment is dangerous, and `reusable-changelog-fragment-check.yml:246` is the counterexample sitting in the same repo. If you touch this again, citing that line as the safe case would make the comment self-verifying. Not worth a push on its own.

bosun merged commit 0c4adbf4a1 into main 2026-08-20 02:13:19 +02:00

bosun referenced this pull request from a commit 2026-08-20 02:13:21 +02:00

fix(release): an expression spelled out in a run-block comment made reusable-release.yml unusable (#762)

release-bot referenced this pull request 2026-08-20 02:14:48 +02:00

chore(release): v0.41.0 #755

bosun referenced this pull request 2026-08-20 02:16:15 +02:00

feat(gates): a Go test must reject workflow expressions with undefined operands #763

shipwright referenced this pull request 2026-08-20 02:18:37 +02:00

fix(tests): arm 30 parses the workflow instead of scanning raw lines — only a parser can decide what survives YAML #766

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!762

No description provided.