amtronics/platform-actions

Watch

Fork 0

fix: Name a failed version step in the release notification #87

Merged

ahmad merged 1 commit from feature/86-version-failure-details into main

2026-09-11 01:13:37 +00:00

ahmad commented

2026-09-11 00:18:00 +00:00

Owner

Issue

Refs #86 — the template. The thirteen consumers that carry the same line are separate pull requests; see below.

Problem

When the version job fails, needs.version.outputs.released is empty rather than 'true', so the details expression falls through every branch to its last one and the notification reads:

❌ Release — no release warranted by the commits

The status is right. The sentence beside it describes a clean run over commits that warranted nothing — a non-event — where a job has actually fallen over.

Solution

A needs.version.result != 'success' branch, placed exactly where the checks branch sits and for the same reason: the two failures want different people looking.

Review notes

Enumerated rather than reasoned about. Evaluating both expressions over all 1024 combinations of checks × version × publish × deploy (four results each) crossed with released and notes_fallback, using operand semantics rather than boolean (a && b is a when a is falsy else b):

combinations                                 1024
unchanged                                     832
changed                                       192
every combination with version == 'success' unchanged   True

So criterion 3 holds exactly: the change is confined to version not succeeding. Criterion 2 checked directly — commits warranting no release still reads no release warranted by the commits.

The change is larger than the issue describes, in a good way. Of the 192 changed combinations:

previously said	count
`no release warranted by the commits`	48
the full success line — `version: v1.2.3 · publish: … · deploy: …`	48

The second group is the one the issue does not mention and I think matters more. A version job can set released and version as outputs and then fail in a later step; those runs printed a sentence claiming a release that did not happen, beside a red status. Reporting a success that did not occur is worse than reporting a non-event.

(The remaining 96 are the same two groups with notes_fallback set, which does not affect which branch is taken.)

Risks and trade-offs

The size label was wrong and I changed it from XS to M. The expression is one line, but it is one line in fourteen repositories: this template plus thirteen consumers that inline it — imamah, trip, portfolio, launchpad, namey, expiro, larda, bayan-web, kidpoints, suhba-web, suhba-docs, khitta and forgejo-mcp. I checked each: all thirteen carry the old line and none already guards version.result. (bayan/stem does not carry it at all.)

This PR is the template only. I will work through the consumers in following iterations unless you would rather they waited — it is a notification wording fix at P3, and thirteen pull requests is real churn for it. Say so and I will hold them; otherwise they come one at a time, each verified the same way.

### Issue Refs #86 — the template. The thirteen consumers that carry the same line are separate pull requests; see below. ### Problem When the `version` job fails, `needs.version.outputs.released` is **empty** rather than `'true'`, so the `details` expression falls through every branch to its last one and the notification reads: > ❌ Release — no release warranted by the commits The status is right. The sentence beside it describes a clean run over commits that warranted nothing — a non-event — where a job has actually fallen over. ### Solution A `needs.version.result != 'success'` branch, placed exactly where the `checks` branch sits and for the same reason: the two failures want different people looking. ### Review notes **Enumerated rather than reasoned about.** Evaluating both expressions over **all 1024 combinations** of `checks × version × publish × deploy` (four results each) crossed with `released` and `notes_fallback`, using operand semantics rather than boolean (`a && b` is `a` when `a` is falsy else `b`): ``` combinations 1024 unchanged 832 changed 192 every combination with version == 'success' unchanged True ``` So criterion 3 holds exactly: the change is confined to `version` not succeeding. Criterion 2 checked directly — commits warranting no release still reads `no release warranted by the commits`. **The change is larger than the issue describes, in a good way.** Of the 192 changed combinations: | previously said | count | |---|---| | `no release warranted by the commits` | 48 | | the **full success line** — `version: v1.2.3 · publish: … · deploy: …` | 48 | The second group is the one the issue does not mention and I think matters more. A `version` job can set `released` and `version` as outputs and then fail in a later step; those runs printed a sentence **claiming a release that did not happen**, beside a red status. Reporting a success that did not occur is worse than reporting a non-event. *(The remaining 96 are the same two groups with `notes_fallback` set, which does not affect which branch is taken.)* ### Risks and trade-offs **The size label was wrong and I changed it from `XS` to `M`.** The expression is one line, but it is one line in **fourteen repositories**: this template plus thirteen consumers that inline it — `imamah`, `trip`, `portfolio`, `launchpad`, `namey`, `expiro`, `larda`, `bayan-web`, `kidpoints`, `suhba-web`, `suhba-docs`, `khitta` and `forgejo-mcp`. I checked each: all thirteen carry the old line and none already guards `version.result`. (`bayan/stem` does not carry it at all.) This PR is the template only. I will work through the consumers in following iterations unless you would rather they waited — it is a notification wording fix at P3, and thirteen pull requests is real churn for it. Say so and I will hold them; otherwise they come one at a time, each verified the same way.

ahmad added 1 commit

2026-09-11 00:18:00 +00:00

fix: Name a failed version step in the release notification

PR / validate (pull_request) Successful in 19s

Details

8789517394

When the version job fails, released is empty rather than 'true', so the
details expression fell through every branch to its last one and the
notification read "no release warranted by the commits" beside a red
status — a clean non-event described where a job had fallen over.

A branch for version.result now names it, as the checks branch already
does for the same reason: the two want different people looking.

Enumerating the expression over all 1024 combinations shows the change
is confined to version failing. Every combination where version
succeeded is byte-identical, including the no-release-warranted case.

It also corrects 48 combinations the issue did not name: a version job
can set released and its version output and then fail in a later step,
and those printed the full success line beside a red status — claiming a
release that did not happen, which is worse than the reported symptom.

Refs #86

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

ahmad changed title from ~~WIP: fix: Name a failed version step in the release notification~~ to fix: Name a failed version step in the release notification

2026-09-11 00:19:13 +00:00

ahmad commented

2026-09-11 00:19:13 +00:00

Author

Owner

Ready. Verified against each criterion by enumerating the expression rather than reasoning about it.

Criterion 1 — "given the version job fails, the details line says the version step failed and does not claim the commits warranted no release." With checks: success, version: failure, the line is now the version step did not pass (failure) — nothing was tagged, published or deployed. It reads the same for cancelled and skipped, naming which.

Criterion 2 — "a run over commits that warrant no release still reads as it does today." Checked directly: checks: success, version: success, released: '' gives no release warranted by the commits, before and after.

Criterion 3 — "every other combination is unchanged." This is the one worth a measurement rather than a claim. Evaluating both expressions over all 1024 combinations — checks × version × publish × deploy at four results each, crossed with released and notes_fallback — with operand semantics rather than boolean:

unchanged                                       832
changed                                         192
every combination where version == 'success'    UNCHANGED

The change is confined to version not succeeding, which is criterion 1's own territory.

And a second defect, which the issue does not mention. Of the 192 changed combinations, only 48 previously said no release warranted by the commits. The other 48 (plus their notes_fallback variants) printed the full success line — version: v1.2.3 · publish: success · deploy: success — while version had failed. That happens because a job can set its outputs and then fall over in a later step, leaving released == 'true' on a failed job. Those runs announced a release that never happened, beside a red status. I would rather have found that here than have it reported later.

Scope correction, and the thing that needs your steer. I relabelled the issue size/XS → size/M: the expression is one line, but it is one line in fourteen repositories. I checked each of the thirteen consumers — all carry the old line and none already guards version.result; bayan/stem does not carry it at all. This pull request is the template. The consumers are thirteen more, and at P3 for a notification's wording that is real churn, so I have said on the issue that I will work through them one per iteration unless you would rather they waited.

PR / validate is green on this head.

Ready. Verified against each criterion by enumerating the expression rather than reasoning about it. **Criterion 1 — "given the `version` job fails, the details line says the version step failed and does not claim the commits warranted no release."** With `checks: success, version: failure`, the line is now `the version step did not pass (failure) — nothing was tagged, published or deployed`. It reads the same for `cancelled` and `skipped`, naming which. **Criterion 2 — "a run over commits that warrant no release still reads as it does today."** Checked directly: `checks: success, version: success, released: ''` gives `no release warranted by the commits`, before and after. **Criterion 3 — "every other combination is unchanged."** This is the one worth a measurement rather than a claim. Evaluating both expressions over **all 1024 combinations** — `checks × version × publish × deploy` at four results each, crossed with `released` and `notes_fallback` — with operand semantics rather than boolean: ``` unchanged 832 changed 192 every combination where version == 'success' UNCHANGED ``` The change is confined to `version` not succeeding, which is criterion 1's own territory. **And a second defect, which the issue does not mention.** Of the 192 changed combinations, only 48 previously said `no release warranted by the commits`. The other 48 (plus their `notes_fallback` variants) printed the **full success line** — `version: v1.2.3 · publish: success · deploy: success` — while `version` had failed. That happens because a job can set its outputs and then fall over in a later step, leaving `released == 'true'` on a failed job. Those runs announced a release that never happened, beside a red status. I would rather have found that here than have it reported later. **Scope correction, and the thing that needs your steer.** I relabelled the issue `size/XS` → `size/M`: the expression is one line, but it is one line in **fourteen repositories**. I checked each of the thirteen consumers — all carry the old line and none already guards `version.result`; `bayan/stem` does not carry it at all. This pull request is the template. The consumers are thirteen more, and at P3 for a notification's wording that is real churn, so I have said on the issue that I will work through them one per iteration unless you would rather they waited. `PR / validate` is green on this head.

ahmad_bot approved these changes

2026-09-11 00:46:07 +00:00

ahmad_bot left a comment

Approved. I re-ran the enumeration rather than reading the table, and it reproduces exactly.

1024 combinations of checks × version × publish × deploy (four results each) crossed with released and notes_fallback, evaluated with operand semantics — a && b is a when a is falsy else b, a || b is a when a is truthy else b:

combinations  1024   unchanged  832   changed  192
every combination with version == 'success' unchanged:  True
changed, by what the old expression said:  96 × full success line, 96 × 'no release warranted by the commits'

Criterion 3 holds exactly as you state: nothing moves unless version failed to succeed. Criterion 2 checked directly — checks success, version success, released empty still reads no release warranted by the commits.

Your point about the second group is the one that matters, and the numbers bear it out. 96 of the 192 previously printed version: v1.2.3 · publish: … · deploy: … — a sentence claiming a release, beside a red status — on runs where the version job set its outputs and then fell over. That is a worse failure than the one the issue describes, and it is the one I would have led with.

The new branch can only fire on a version job that actually ran and did not succeed. version declares needs: [checks] and carries no if:, so the only way it is skipped is checks having failed — and the checks branch, which sits ahead of it, catches that first. So there is no path where the new sentence appears on a run that never attempted a version.

It also brings details into line with status, which was already right. The status expression requires needs.version.result == 'success' for a green result, so a failed version job has been reporting ❌ since #81 while the sentence beside it described a clean non-event. This is the half that had not caught up.

On the consumers, your list is correct — I checked all fourteen repositories. Twelve carry the old line and amtronics/forgejo-mcp does too (release.yml:355; my first pass missed it and that was a stale local clone with no origin/main ref, not a fact about the repo). None of the thirteen guards version.result in details. Worth stating precisely, because twelve of them do reference needs.version.result twice — in the status expression, which is a different thing and already correct.

bayan/stem is not merely missing this line; it has a different and worse version of the same defect. Its details (release.yml:277) has no released branch at all — checks != 'success' falls straight through to format('version: v{0} · …'). So a failed version job there prints a success line with an empty version number: version: v · publish: … · deploy: …. It will not be fixed by copying this change, and it should not be filed as the fourteenth consumer PR.

Whether thirteen pull requests are worth a P3 notification fix is the owner's call, not mine. What I can say is that the verification cost per consumer is near zero — the line is identical in all thirteen, so the enumeration above covers each of them and a diff is enough.

CI green. Nothing to fix.

Approved. I re-ran the enumeration rather than reading the table, and it reproduces exactly. **1024 combinations of `checks × version × publish × deploy` (four results each) crossed with `released` and `notes_fallback`, evaluated with operand semantics** — `a && b` is `a` when `a` is falsy else `b`, `a || b` is `a` when `a` is truthy else `b`: ``` combinations 1024 unchanged 832 changed 192 every combination with version == 'success' unchanged: True changed, by what the old expression said: 96 × full success line, 96 × 'no release warranted by the commits' ``` Criterion 3 holds exactly as you state: nothing moves unless `version` failed to succeed. Criterion 2 checked directly — `checks` success, `version` success, `released` empty still reads `no release warranted by the commits`. **Your point about the second group is the one that matters, and the numbers bear it out.** 96 of the 192 previously printed `version: v1.2.3 · publish: … · deploy: …` — a sentence claiming a release, beside a red status — on runs where the version job set its outputs and then fell over. That is a worse failure than the one the issue describes, and it is the one I would have led with. **The new branch can only fire on a version job that actually ran and did not succeed.** `version` declares `needs: [checks]` and carries no `if:`, so the only way it is `skipped` is `checks` having failed — and the `checks` branch, which sits ahead of it, catches that first. So there is no path where the new sentence appears on a run that never attempted a version. **It also brings `details` into line with `status`, which was already right.** The status expression requires `needs.version.result == 'success'` for a green result, so a failed version job has been reporting ❌ since #81 while the sentence beside it described a clean non-event. This is the half that had not caught up. **On the consumers, your list is correct — I checked all fourteen repositories.** Twelve carry the old line and `amtronics/forgejo-mcp` does too (`release.yml:355`; my first pass missed it and that was a stale local clone with no `origin/main` ref, not a fact about the repo). None of the thirteen guards `version.result` **in `details`**. Worth stating precisely, because twelve of them *do* reference `needs.version.result` twice — in the `status` expression, which is a different thing and already correct. **`bayan/stem` is not merely missing this line; it has a different and worse version of the same defect.** Its `details` (`release.yml:277`) has no `released` branch at all — `checks != 'success'` falls straight through to `format('version: v{0} · …')`. So a failed version job there prints a success line with an **empty version number**: `version: v · publish: … · deploy: …`. It will not be fixed by copying this change, and it should not be filed as the fourteenth consumer PR. Whether thirteen pull requests are worth a P3 notification fix is the owner's call, not mine. What I can say is that the verification cost per consumer is near zero — the line is identical in all thirteen, so the enumeration above covers each of them and a diff is enough. CI green. Nothing to fix.

ahmad merged commit 42de701228 into main

2026-09-11 01:13:37 +00:00

ahmad deleted branch feature/86-version-failure-details

2026-09-11 01:13:37 +00:00

ahmad referenced this pull request from a commit

2026-09-11 01:13:39 +00:00

fix: Name a failed version step in the release notification (#87)