amtronics/platform-actions
Watch
7
0
Fork
You've already forked platform-actions
 0
Code Issues 7 Pull requests Projects Releases 36 Packages Wiki Activity Actions

fix: Report a release that released nothing as skipped #82

Merged
ahmad merged 2 commits from feature/81-no-release-status into main 2026-09-09 22:48:49 +00:00
Conversation 3 Commits 2 Files changed 4 +114 -3
ahmad commented 2026-09-09 18:58:29 +00:00
Owner
Copy link

Issue

Closes #81

Problem

A merge that warrants no release still notifies, and claims an outcome that did not happen. ahmad/trip announced its last such run as ❌ failure; the repositories converted by #72 announce it as ✅ success. Nothing succeeded and nothing failed — nothing ran, and the reader loses exactly the distinction they open the message for.

Solution

notify-telegram gains an icon for skipped, a status it already documented but had no branch for, so it was landing on the catch-all warning. The shared template now sends skipped when released is not true, checked before the job results — on a no-release run publish and deploy are skipped, which would otherwise read as a healthy release.

Review notes

Four behaviour cases cover the icon table, including that skipped is distinguishable from both success and failure, and that an unknown status is still announced with a warning rather than swallowed. The catch-all stays a warning deliberately: an unknown status is a bug in the caller and should look like one.

Consumers copy this template, so each repository needs its own follow-up; ahmad/trip and ahmad/portfolio are the urgent two, because they report failure today.

Risks and trade-offs

A third state means a reader who filtered on ✅/❌ sees something new. That is the point, but it is a visible change to every release message.

### Issue Closes #81 ### Problem A merge that warrants no release still notifies, and claims an outcome that did not happen. `ahmad/trip` announced its last such run as ❌ failure; the repositories converted by #72 announce it as ✅ success. Nothing succeeded and nothing failed — nothing ran, and the reader loses exactly the distinction they open the message for. ### Solution `notify-telegram` gains an icon for `skipped`, a status it already documented but had no branch for, so it was landing on the catch-all warning. The shared template now sends `skipped` when `released` is not `true`, checked before the job results — on a no-release run publish and deploy are skipped, which would otherwise read as a healthy release. ### Review notes Four behaviour cases cover the icon table, including that `skipped` is distinguishable from both success and failure, and that an unknown status is still announced with a warning rather than swallowed. The catch-all stays a warning deliberately: an unknown status is a bug in the caller and should look like one. Consumers copy this template, so each repository needs its own follow-up; `ahmad/trip` and `ahmad/portfolio` are the urgent two, because they report failure today. ### Risks and trade-offs A third state means a reader who filtered on ✅/❌ sees something new. That is the point, but it is a visible change to every release message.
fix: Report a release that released nothing as skipped
All checks were successful
PR / validate (pull_request) Successful in 20s
Details
64ba9ff3f2 
A merge with no releasable commit announced itself as a success in the
converted repositories and as a failure in the two that still ask
whether publish succeeded. Neither is true: nothing ran.

Give notify-telegram an icon for the status it already documented, and
have the template send it when no release was warranted.
ahmad changed title from WIP: fix: Report a release that released nothing as skipped to fix: Report a release that released nothing as skipped 2026-09-09 18:59:17 +00:00
ahmad_bot reviewed 2026-09-09 19:19:18 +00:00
ahmad_bot left a comment
Copy link

One finding, inline, non-blocking — the fourth outcome, left where the third one was. The change itself is right and I checked the part the suite cannot reach.

Verified

  • The icon table is properly pinned. Mutated three ways at 64ba9ff3:

    drop the `skipped` icon (falls to the catch-all)  -> FAIL "a release that released nothing is neither tick nor cross"
map `skipped` to the success icon                 -> FAIL, same case
make the catch-all a tick instead of a warning    -> FAIL "an unknown status is announced, not swallowed"
restored                                          -> all cases pass

    Asserting calls_not_contain=["✅","❌"] alongside calls_contain=["➖"] is what makes the first two both fail — an icon test that only asserted the new icon would have passed the second mutation. check-actions green, whole suite green.

  • The template expression is correct, and it is the part no behaviour case can exercise, so I traced it against the expression semantics rather than assuming. (A && B && C) && (D && 'success' || 'skipped') || 'failure': with A&&B&&C true the outer && yields the inner value, which is 'success' when released == 'true' and 'skipped' otherwise; with it false the outer && yields false and the || yields 'failure'. The hazard in this idiom is a middle operand that can be falsy — an empty string there collapses the whole expression to the last branch — and both inner values are non-empty literals, so it does not bite. Checking released before the job results is also the right order, exactly as the comment says: on a no-release run publish and deploy are skipped, which the first three conjuncts accept.

  • needs.version.outputs.released is only ever the literal true or false when version succeeds (release/action.yml writes one or the other and exits 0), so there is no third value to fall through.

  • The README line and the input description now both list four statuses, and the catch-all staying a warning is the right call for the stated reason.

  • The scope note is accurate and worth repeating: consumers hold their own copy of this template, so merging this changes nothing for them until each is converted. The action half is shared via @v3 but inert until a consumer sends skipped.

  • CI green (validate).

One finding, inline, non-blocking — the fourth outcome, left where the third one was. The change itself is right and I checked the part the suite cannot reach. **Verified** - **The icon table is properly pinned.** Mutated three ways at `64ba9ff3`: drop the `skipped` icon (falls to the catch-all) -> FAIL "a release that released nothing is neither tick nor cross" map `skipped` to the success icon -> FAIL, same case make the catch-all a tick instead of a warning -> FAIL "an unknown status is announced, not swallowed" restored -> all cases pass Asserting `calls_not_contain=["✅","❌"]` alongside `calls_contain=["➖"]` is what makes the first two both fail — an icon test that only asserted the new icon would have passed the second mutation. `check-actions` green, whole suite green. - **The template expression is correct, and it is the part no behaviour case can exercise**, so I traced it against the expression semantics rather than assuming. `(A && B && C) && (D && 'success' || 'skipped') || 'failure'`: with `A&&B&&C` true the outer `&&` yields the inner value, which is `'success'` when `released == 'true'` and `'skipped'` otherwise; with it false the outer `&&` yields `false` and the `||` yields `'failure'`. The hazard in this idiom is a middle operand that can be falsy — an empty string there collapses the whole expression to the last branch — and both inner values are non-empty literals, so it does not bite. Checking `released` **before** the job results is also the right order, exactly as the comment says: on a no-release run publish and deploy are `skipped`, which the first three conjuncts accept. - `needs.version.outputs.released` is only ever the literal `true` or `false` when `version` succeeds (`release/action.yml` writes one or the other and exits 0), so there is no third value to fall through. - The README line and the input description now both list four statuses, and the catch-all staying a warning is the right call for the stated reason. - **The scope note is accurate and worth repeating**: consumers hold their own copy of this template, so merging this changes nothing for them until each is converted. The action half is shared via `@v3` but inert until a consumer sends `skipped`. - CI green (`validate`).
templates/release.yml Outdated
@ -315,0 +320,4 @@
${{ (needs.version.result == 'success'
&& (needs.publish.result == 'success' || needs.publish.result == 'skipped')
&& (needs.deploy.result == 'success' || needs.deploy.result == 'skipped'))
&& (needs.version.outputs.released == 'true' && 'success' || 'skipped')
ahmad_bot commented 2026-09-09 19:19:18 +00:00
Member
Copy link

The fourth outcome is still collapsed into failure, and it is the same shape as the bug this fixes.

needs.<job>.result has four values. This expression now distinguishes three — and a cancelled job falls into the || 'failure' branch, so a cancelled release announces ❌ while notify-telegram carries a ⚪ icon for exactly that case and the README documents it as one of the four statuses. Nothing in the template ever sends cancelled.

It is reachable, and if: always() on this job is what makes it so: the notify job runs when the upstream ones are cancelled, and reports failure. concurrency.cancel-in-progress: false rules out the automatic path, so what remains is a human cancelling a run — uncommon, which is why this is not blocking, but it is precisely the reader whose message is now wrong in the same way #81 describes: nothing failed.

The shape you have already written extends to it directly:

status: >-
  ${{ (needs.version.result == 'cancelled'
       || needs.publish.result == 'cancelled'
       || needs.deploy.result == 'cancelled') && 'cancelled'
      || ((needs.version.result == 'success'
           && (needs.publish.result == 'success' || needs.publish.result == 'skipped')
           && (needs.deploy.result == 'success' || needs.deploy.result == 'skipped'))
          && (needs.version.outputs.released == 'true' && 'success' || 'skipped')
          || 'failure') }}

Cancelled first, because a cancelled job is neither a success nor a failure and the later conjuncts would read it as the latter — the same reason released is checked before the job results here.

Worth deciding rather than deferring, because of the follow-up cost you name yourself: every consumer copies this template, so the difference between three outcomes and four is the difference between converting each repository once and converting it twice. If you would rather leave cancelled reading as a failure, that is defensible — a cancelled release usually is something a person wants to look at — but then the action's ⚪ branch has no caller, and the README's four-status line has no producer, which is worth a sentence somewhere so the next reader does not go looking for the path that sends it.

**The fourth outcome is still collapsed into `failure`, and it is the same shape as the bug this fixes.** `needs.<job>.result` has four values. This expression now distinguishes three — and a `cancelled` job falls into the `|| 'failure'` branch, so a cancelled release announces ❌ while `notify-telegram` carries a ⚪ icon for exactly that case and the README documents it as one of the four statuses. Nothing in the template ever sends `cancelled`. It is reachable, and `if: always()` on this job is what makes it so: the notify job runs when the upstream ones are cancelled, and reports failure. `concurrency.cancel-in-progress: false` rules out the automatic path, so what remains is a human cancelling a run — uncommon, which is why this is not blocking, but it is precisely the reader whose message is now wrong in the same way `#81` describes: nothing failed. The shape you have already written extends to it directly: ```yaml status: >- ${{ (needs.version.result == 'cancelled' || needs.publish.result == 'cancelled' || needs.deploy.result == 'cancelled') && 'cancelled' || ((needs.version.result == 'success' && (needs.publish.result == 'success' || needs.publish.result == 'skipped') && (needs.deploy.result == 'success' || needs.deploy.result == 'skipped')) && (needs.version.outputs.released == 'true' && 'success' || 'skipped') || 'failure') }} ``` Cancelled first, because a cancelled job is neither a success nor a failure and the later conjuncts would read it as the latter — the same reason `released` is checked before the job results here. Worth deciding rather than deferring, because of the follow-up cost you name yourself: every consumer copies this template, so the difference between three outcomes and four is the difference between converting each repository once and converting it twice. If you would rather leave `cancelled` reading as a failure, that is defensible — a cancelled release usually *is* something a person wants to look at — but then the action's ⚪ branch has no caller, and the README's four-status line has no producer, which is worth a sentence somewhere so the next reader does not go looking for the path that sends it.
ahmad force-pushed feature/81-no-release-status from 64ba9ff3f2
All checks were successful
PR / validate (pull_request) Successful in 20s
Details
to 2a39b7bc61
All checks were successful
PR / validate (pull_request) Successful in 24s
Details
2026-09-09 20:17:59 +00:00 Compare
fix: Report a cancelled release as cancelled
All checks were successful
PR / validate (pull_request) Successful in 17s
Details
02c21d3e41 
The notify job runs on always(), so it runs when the release above it
was cancelled — and the expression collapsed that into failure, which
is the same lie this change set out to fix. The icon for it already
existed and nothing ever sent it.
ahmad_bot approved these changes 2026-09-09 21:50:06 +00:00
Dismissed
ahmad_bot left a comment
Copy link

Approved. The fourth outcome is in, and the ordering question I flagged is resolved the way it had to be.

The icon is pinned. Mutated at 02c21d3e:

baseline                                   all cases pass  (check-actions ok)
drop the `cancelled` icon (→ catch-all) -> FAIL "a cancelled release is neither failed nor merely unknown"
map `cancelled` to the failure icon      -> FAIL, same case
restored                                   all cases pass

calls_not_contain=["❌", "⚠"] beside calls_contain=["⚪"] is what makes both fail — the same shape that made the skipped case worth having, and it is why a test asserting only the new icon would have passed the second mutation.

The expression is correct, and this time I evaluated it rather than traced it. No behaviour case can reach a workflow expression, so I implemented the &&/|| value semantics and enumerated all 192 combinations of (version, publish, deploy) × released:

success     4  — everything success-or-skipped AND released == 'true'
skipped     8  — the pipeline healthy, nothing released
cancelled 111  — any of the three cancelled
failure    69  — everything else

all green + released      -> success        no-release run (skips)  -> skipped
all green + not released  -> skipped        deploy cancelled        -> cancelled
version cancelled         -> cancelled      publish failed          -> failure
released empty + green    -> skipped

The last row matters: an empty released — which is what a reader would fear from a job that wrote no output — lands on skipped, not on a false success. And the nesting is safe for the same reason as before: every operand that can be returned is a non-empty literal, so nothing collapses to the last branch.

One consequence worth stating rather than discovering. cancelled is checked first, so a run where something failed and something else was cancelled reports cancelled. That is the right way round — checking failure first is exactly what produced #81, since a pure cancellation leaves version.result != 'success' and would report ❌ — but it does mean the icon answers "was this run stopped" before "did anything break". A reader is already opening a cancelled run, and the message carries the link, so I would leave it; it is worth knowing it is a choice and not an accident.

Doing both outcomes in one PR was the right call, and it settles what I raised last time: consumers copy this template, so three outcomes then four would have meant converting eleven repositories twice.

Approved. The fourth outcome is in, and the ordering question I flagged is resolved the way it had to be. **The icon is pinned.** Mutated at `02c21d3e`: baseline all cases pass (check-actions ok) drop the `cancelled` icon (→ catch-all) -> FAIL "a cancelled release is neither failed nor merely unknown" map `cancelled` to the failure icon -> FAIL, same case restored all cases pass `calls_not_contain=["❌", "⚠"]` beside `calls_contain=["⚪"]` is what makes both fail — the same shape that made the `skipped` case worth having, and it is why a test asserting only the new icon would have passed the second mutation. **The expression is correct, and this time I evaluated it rather than traced it.** No behaviour case can reach a workflow expression, so I implemented the `&&`/`||` value semantics and enumerated all 192 combinations of `(version, publish, deploy) × released`: success 4 — everything success-or-skipped AND released == 'true' skipped 8 — the pipeline healthy, nothing released cancelled 111 — any of the three cancelled failure 69 — everything else all green + released -> success no-release run (skips) -> skipped all green + not released -> skipped deploy cancelled -> cancelled version cancelled -> cancelled publish failed -> failure released empty + green -> skipped The last row matters: an empty `released` — which is what a reader would fear from a job that wrote no output — lands on `skipped`, not on a false `success`. And the nesting is safe for the same reason as before: every operand that can be returned is a non-empty literal, so nothing collapses to the last branch. **One consequence worth stating rather than discovering.** `cancelled` is checked first, so a run where something *failed* and something else was *cancelled* reports `cancelled`. That is the right way round — checking failure first is exactly what produced `#81`, since a pure cancellation leaves `version.result != 'success'` and would report ❌ — but it does mean the icon answers "was this run stopped" before "did anything break". A reader is already opening a cancelled run, and the message carries the link, so I would leave it; it is worth knowing it is a choice and not an accident. **Doing both outcomes in one PR was the right call**, and it settles what I raised last time: consumers copy this template, so three outcomes then four would have meant converting eleven repositories twice.
ahmad force-pushed feature/81-no-release-status from 02c21d3e41
All checks were successful
PR / validate (pull_request) Successful in 17s
Details
to f83ef17dec
All checks were successful
PR / validate (pull_request) Successful in 19s
Details
2026-09-09 21:54:46 +00:00 Compare
ahmad dismissed ahmad_bot's review 2026-09-09 21:54:46 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

ahmad_bot approved these changes 2026-09-09 22:48:36 +00:00
ahmad_bot left a comment
Copy link

Approved, re-affirmed after the rebase over #80.

Two of this PR's four files differ from the head I approved — behaviour-tests.py and README.md — and both are the merge rather than new work: the script now carries #80's excluded-types cases and this PR's cancelled case in one file, and the README carries both status-row edits. That is the file where two PRs adding cases to the same list can drop one, so I checked it by running rather than reading: whole suite green, check-actions green, and the cancelled icon is still pinned — deleting cancelled) ICON="⚪" fails exactly a cancelled release is neither failed nor merely unknown.

The verdict is otherwise unchanged from 02c21d3e, where the work itself was verified:

  • The icon table pinned two ways, calls_not_contain=["❌", "⚠"] beside calls_contain=["⚪"] being what makes a mapping mutation fail rather than pass.
  • The four-branch expression evaluated exhaustively rather than traced — 192 combinations of (version, publish, deploy) × released giving success 4, skipped 8, cancelled 111, failure 69, with an empty released landing on skipped rather than a false success.
  • cancelled first is the right precedence, since checking failure first is what produced #81.
Approved, re-affirmed after the rebase over `#80`. Two of this PR's four files differ from the head I approved — `behaviour-tests.py` and `README.md` — and both are the merge rather than new work: the script now carries `#80`'s excluded-types cases **and** this PR's cancelled case in one file, and the README carries both status-row edits. That is the file where two PRs adding cases to the same list can drop one, so I checked it by running rather than reading: **whole suite green, `check-actions` green**, and the cancelled icon is still pinned — deleting `cancelled) ICON="⚪"` fails exactly *a cancelled release is neither failed nor merely unknown*. The verdict is otherwise unchanged from `02c21d3e`, where the work itself was verified: - The icon table pinned two ways, `calls_not_contain=["❌", "⚠"]` beside `calls_contain=["⚪"]` being what makes a mapping mutation fail rather than pass. - The four-branch expression evaluated exhaustively rather than traced — 192 combinations of `(version, publish, deploy) × released` giving success 4, skipped 8, cancelled 111, failure 69, with an empty `released` landing on `skipped` rather than a false `success`. - `cancelled` first is the right precedence, since checking failure first is what produced `#81`.
ahmad merged commit 0ffab747f0 into main 2026-09-09 22:48:49 +00:00
ahmad deleted branch feature/81-no-release-status 2026-09-09 22:48:49 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
ahmad_bot
Labels
Clear labels   
priority/P0
Drop everything

  
priority/P1
High

  
priority/P2
Normal

  
priority/P3
Low

  
size/L
Several days

  
size/M
A day or two

  
size/S
A few hours

  
size/XL

   
size/XS
Under an hour

  
state/blocked

   
state/done
Merged and verified

  
state/in-progress
Being worked on

  
state/in-review
PR open

  
state/needs-refinement
Not ready to pick up

  
state/ready
Refined and ready to start

  
type/bug
Something is broken

  
type/chore
Maintenance, tooling, setup

  
type/feature
New functionality

  
type/refactor
Restructuring without behaviour change

No labels
priority/P0
priority/P1
priority/P2
priority/P3
size/L
size/M
size/S
size/XL
size/XS
state/blocked
state/done
state/in-progress
state/in-review
state/needs-refinement
state/ready
type/bug
type/chore
type/feature
type/refactor
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
You do not have permission to read 13 dependencies
Reference
amtronics/platform-actions!82
No description provided.