amtronics/platform-actions

Watch

Fork 0

fix: A failed notification must not fail the release #65

New issue

Closed

opened 2026-09-08 20:03:54 +00:00 by ahmad · 3 comments

ahmad commented

2026-09-08 20:03:54 +00:00

Owner

Context

A release notification that cannot be delivered currently fails the release. ahmad/trip's run 2462 (2026-09-07) is the example: version succeeded, but the notify job exited 127 and the whole run is recorded as a failure, so a release that was fine reads as broken.

The chain is notify → openbao-fetch → an on-demand apt-get install curl jq in the job container → jq missing → curl … | jq exits 127. The immediate cause of the failed install was bullseye's dead security suite and is fixed (amtronics/infrastructure-vps#420), but the shape remains: every notify job reaches the network to install a tool before it can read a secret, and any failure along that path — a slow mirror, an OpenBao blip, a Telegram outage — is reported as a failed release.

The conventions state that notification delivery is non-blocking and that an outage at the provider must never fail a build or a release. Today it does.

Scope

In: the notify path in the reusable workflows and the composite actions it uses — making a delivery failure visible without failing the run, and removing the runtime dependency on installing jq where it can be avoided.
Out: the Docker CLI install step (#59); which events send notifications; the message format.

Acceptance criteria

Given the Telegram API is unreachable or refuses the message, when a release runs, then the run's conclusion is unchanged and the failure is visible as a warning in the job log.
Given the secret fetch fails, when a release runs, then the run's conclusion is unchanged and the reason is visible.
Given a job container without jq, when the notify path runs, then it either does not need jq or its absence is not fatal.
Given a genuinely failed build, then the notification still reports failure — a non-blocking notify must not hide the thing it is reporting.
Given the change is merged, then no consumer repository needs an edit for the above to hold.

### Context A release notification that cannot be delivered currently fails the release. `ahmad/trip`'s run 2462 (2026-09-07) is the example: `version` succeeded, but the `notify` job exited 127 and the whole run is recorded as a failure, so a release that was fine reads as broken. The chain is `notify` → `openbao-fetch` → an on-demand `apt-get install curl jq` in the job container → `jq` missing → `curl … | jq` exits 127. The immediate cause of the failed install was bullseye's dead security suite and is fixed (`amtronics/infrastructure-vps#420`), but the shape remains: every notify job reaches the network to install a tool before it can read a secret, and any failure along that path — a slow mirror, an OpenBao blip, a Telegram outage — is reported as a failed release. The conventions state that notification delivery is non-blocking and that an outage at the provider must never fail a build or a release. Today it does. ### Scope - In: the notify path in the reusable workflows and the composite actions it uses — making a delivery failure visible without failing the run, and removing the runtime dependency on installing `jq` where it can be avoided. - Out: the Docker CLI install step (#59); which events send notifications; the message format. ### Acceptance criteria - [ ] Given the Telegram API is unreachable or refuses the message, when a release runs, then the run's conclusion is unchanged and the failure is visible as a warning in the job log. - [ ] Given the secret fetch fails, when a release runs, then the run's conclusion is unchanged and the reason is visible. - [ ] Given a job container without `jq`, when the notify path runs, then it either does not need `jq` or its absence is not fatal. - [ ] Given a genuinely failed build, then the notification still reports failure — a non-blocking notify must not hide the thing it is reporting. - [ ] Given the change is merged, then no consumer repository needs an edit for the above to hold.

ahmad added the

labels

2026-09-08 20:04:03 +00:00

ahmad referenced this issue

2026-09-08 20:41:29 +00:00

fix: Parallel image pushes race on shared layers and fail the release #66

ahmad self-assigned this

2026-09-08 21:29:54 +00:00

ahmad added

state/in-progress

and removed

state/ready

labels

2026-09-08 21:29:55 +00:00

ahmad commented

2026-09-08 21:34:49 +00:00

Author

Owner

Picking this up. Plan, and one tension in the criteria I want stated before I write code rather than discovered in review.

What the log actually says. I pulled the job log for ahmad/trip run 2462 (job 6322) instead of reasoning from the description. curl is present in that container — the (23) Failed writing body is curl noticing its output pipe died — and jq is not: 1-composite-0.sh: line 16: jq: command not found, then exitcode '127'. Line 16 is openbao-fetch's AppRole login. The apt install above it printed the expired-bullseye-security error and did not produce a jq. So the failing dependency is jq alone, and it is openbao-fetch's, not the notifier's.

notify-telegram is already non-blocking — no set -e, an empty token is a skip, a delivery failure is a ::warning:: and exit 0. It never needed jq. Criterion 1 holds today, and I will show that rather than assume it.

The approach.

Drop jq from openbao-fetch, and the apt install with it. node is present in every job container on this fleet — act runs JS actions like actions/checkout@v4 with docker exec cmd=[node …] inside the job container, and that step succeeded in this very run — so the JSON parsing moves to node -e. That removes the network round-trip before a secret can be read, which is the "slow mirror" half of the problem as well as the jq half. It reaches every consumer with no edit, because they all pin openbao-fetch@v3 and v3 tracks this branch.
Add a required input, defaulting to true. When false, a fetch failure writes an empty output file, emits a ::warning:: with the reason, and exits 0. The template's notify job passes required: false; notify-telegram then sees an empty token and skips, which is the behaviour it already documents.

Why the default is true and not false. Making the fetch non-fatal everywhere would be smaller and I am not doing it. larda's deploy reads .env.secrets with a while … done < .env.secrets under set -e, so a missing file fails loudly there — but that is one repository's guard, and a deploy that proceeds with empty secrets because the fetcher shrugged is a worse failure than the one this issue is about. A secret fetcher stays fatal by default.

The tension, stated plainly. Criterion 5 asks that no consumer repository need an edit. Item 1 meets that. Item 2 cannot, because the notify job is copied into each consumer, not called — I checked, and no consumer references a reusable workflow. So required: false reaches a repository only when its release.yml is refreshed. My reading: criterion 5 rules out fixing this by hand-editing eleven repositories, and is satisfied by the central fix carrying the cause that has actually fired; the residual tolerance for a genuine OpenBao outage arrives with the template. I will list the consumers carrying the old copy and open one follow-up, the way #66's fourth criterion asks. If you meant criterion 5 more strictly than that, say so and I will stop at item 1.

Verification. The pipeline here only parses YAML and runs bash -n, so it cannot see whether the node reader is correct. I will test that myself against the failure mode already recorded for this substitution: a multi-byte value split across a read boundary silently becoming replacement characters without setEncoding('utf8'), and non-scalar values rendering as [object Object] where jq wrote JSON. A corrupted password reaching .env fails at the application, far from the step that broke it, so this is the part worth measuring rather than reading.

Files: openbao-fetch/action.yml, templates/release.yml, README.md.

Picking this up. Plan, and one tension in the criteria I want stated before I write code rather than discovered in review. **What the log actually says.** I pulled the job log for `ahmad/trip` run 2462 (job 6322) instead of reasoning from the description. `curl` is present in that container — the `(23) Failed writing body` is curl noticing its output pipe died — and `jq` is not: `1-composite-0.sh: line 16: jq: command not found`, then `exitcode '127'`. Line 16 is `openbao-fetch`'s AppRole login. The apt install above it printed the expired-`bullseye-security` error and did not produce a `jq`. So the failing dependency is `jq` alone, and it is `openbao-fetch`'s, not the notifier's. **`notify-telegram` is already non-blocking** — no `set -e`, an empty token is a skip, a delivery failure is a `::warning::` and `exit 0`. It never needed `jq`. Criterion 1 holds today, and I will show that rather than assume it. **The approach.** 1. **Drop `jq` from `openbao-fetch`, and the apt install with it.** `node` is present in every job container on this fleet — act runs JS actions like `actions/checkout@v4` with `docker exec cmd=[node …]` *inside the job container*, and that step succeeded in this very run — so the JSON parsing moves to `node -e`. That removes the network round-trip before a secret can be read, which is the "slow mirror" half of the problem as well as the `jq` half. It reaches every consumer with no edit, because they all pin `openbao-fetch@v3` and `v3` tracks this branch. 2. **Add a `required` input, defaulting to `true`.** When `false`, a fetch failure writes an empty output file, emits a `::warning::` with the reason, and exits 0. The template's notify job passes `required: false`; `notify-telegram` then sees an empty token and skips, which is the behaviour it already documents. **Why the default is `true` and not `false`.** Making the fetch non-fatal everywhere would be smaller and I am not doing it. `larda`'s deploy reads `.env.secrets` with a `while … done < .env.secrets` under `set -e`, so a missing file fails loudly there — but that is one repository's guard, and a deploy that proceeds with empty secrets because the fetcher shrugged is a worse failure than the one this issue is about. A secret fetcher stays fatal by default. **The tension, stated plainly.** Criterion 5 asks that no consumer repository need an edit. Item 1 meets that. **Item 2 cannot**, because the notify job is *copied* into each consumer, not called — I checked, and no consumer references a reusable workflow. So `required: false` reaches a repository only when its `release.yml` is refreshed. My reading: criterion 5 rules out fixing this by hand-editing eleven repositories, and is satisfied by the central fix carrying the cause that has actually fired; the residual tolerance for a genuine OpenBao outage arrives with the template. I will list the consumers carrying the old copy and open one follow-up, the way #66's fourth criterion asks. If you meant criterion 5 more strictly than that, say so and I will stop at item 1. **Verification.** The pipeline here only parses YAML and runs `bash -n`, so it cannot see whether the node reader is correct. I will test that myself against the failure mode already recorded for this substitution: a multi-byte value split across a read boundary silently becoming replacement characters without `setEncoding('utf8')`, and non-scalar values rendering as `[object Object]` where `jq` wrote JSON. A corrupted password reaching `.env` fails at the application, far from the step that broke it, so this is the part worth measuring rather than reading. **Files:** `openbao-fetch/action.yml`, `templates/release.yml`, `README.md`.

ahmad commented

2026-09-08 21:37:54 +00:00

Author

Owner

Correction to the plan above, and I am putting the issue back rather than carrying on — the work has been redirected to ahmad/larda.

Item 1 of that plan is already done, and I read a stale tree. I described openbao-fetch as still installing jq. It does not: 2bccd5e ("fix: Parse OpenBao responses with node, not jq", #58) landed on main and both parses are node -e with setEncoding("utf8"), with the apt fallback reduced to curl-only. My worktree was four commits behind origin/main and I quoted it as current. The run-2462 evidence stands — that failure predates 2bccd5e — but the conclusion I drew from it about what still needs doing was wrong, and I would have written a PR removing something already removed.

So criterion 3 is already satisfied on main, and what remains for this issue is narrower than the plan says: the required input and the template's notify job, which is criteria 2 and 5, plus showing that criteria 1 and 4 hold as they stand.

One thing I noticed and am not folding in. The node renderer writes key + "=" + value, so a non-scalar field renders [object Object] where jq wrote {"a":1}. Every value in secret/<app> is a string today, so nothing is broken now; typeof value === "string" ? value : JSON.stringify(value) restores parity. That is a different defect from this one and belongs in its own issue rather than in a notify fix.

Unassigning and putting this back to state/ready. Nothing was pushed and no branch remains.

Correction to the plan above, and I am putting the issue back rather than carrying on — the work has been redirected to `ahmad/larda`. **Item 1 of that plan is already done, and I read a stale tree.** I described `openbao-fetch` as still installing `jq`. It does not: `2bccd5e` ("fix: Parse OpenBao responses with node, not jq", #58) landed on `main` and both parses are `node -e` with `setEncoding("utf8")`, with the apt fallback reduced to curl-only. My worktree was four commits behind `origin/main` and I quoted it as current. The run-2462 evidence stands — that failure predates `2bccd5e` — but the conclusion I drew from it about what still needs doing was wrong, and I would have written a PR removing something already removed. **So criterion 3 is already satisfied on `main`**, and what remains for this issue is narrower than the plan says: the `required` input and the template's notify job, which is criteria 2 and 5, plus showing that criteria 1 and 4 hold as they stand. **One thing I noticed and am not folding in.** The node renderer writes `key + "=" + value`, so a non-scalar field renders `[object Object]` where `jq` wrote `{"a":1}`. Every value in `secret/<app>` is a string today, so nothing is broken now; `typeof value === "string" ? value : JSON.stringify(value)` restores parity. That is a different defect from this one and belongs in its own issue rather than in a notify fix. Unassigning and putting this back to `state/ready`. Nothing was pushed and no branch remains.

ahmad removed their assignment

2026-09-08 21:37:55 +00:00

ahmad added

state/ready

and removed

state/in-progress

labels

2026-09-08 21:37:55 +00:00

ahmad commented

2026-09-09 04:44:56 +00:00

Author

Owner

Resuming this, and the design question I left open above now has data behind it rather than a judgement.

I checked whether openbao-fetch could simply be non-fatal by default, which would have satisfied both criterion 2 and criterion 5 with no consumer edit at all. It cannot. I surveyed every use of the action across the fleet and looked at what follows it:

Guarded (a missing value fails loudly at the point of use): ahmad/expiro deploy, ahmad/larda deploy, suhba/suhba-docs publish and validate, suhba/suhba-web cms-pr, ahmad/imamah version.
Unguarded: everything else — including the deploy job of ahmad/trip, ahmad/portfolio, bayan/kidpoints, bayan/stem, suhba/khitta, suhba/suhba-web and ahmad/imamah.

So a non-fatal default would let a deploy proceed with an empty .env on seven repositories: containers started with no DATABASE_URL, no error, a green run. That is a worse failure than the one this issue is about, and it is the "trades a loud failure for a quiet one" shape the fleet has been bitten by before. The fetcher stays fatal by default.

The blast radius of the actual problem, since I now have the numbers. A notify job that fetches from OpenBao exists in eight repositories — ahmad/portfolio, ahmad/trip, ahmad/imamah, bayan/kidpoints, bayan/stem, suhba/khitta, suhba/suhba-docs, suhba/suhba-web — and in portfolio it is on pr.yml and monitoring.yml as well. Every one of those turns an OpenBao blip into a failed run for a build that was fine.

What I am building, unchanged from the plan above except that item 1 is already done by 2bccd5e:

openbao-fetch gains required, default "true". With required: false a failure writes an empty output file, emits a ::warning:: naming the reason, and exits 0.
templates/release.yml's notify job passes required: false. notify-telegram already treats an empty token as a skip, so the run continues and says why.

Criterion 5, stated plainly rather than claimed. The central change reaches every consumer through @v3 for the jq half, which is what has actually been failing. The tolerance half needs each consumer's copied notify job to gain one line, because the job is copied rather than called — I verified again that no consumer references a reusable workflow. I will list them on a follow-up issue rather than edit eight repositories inside this PR.

Resuming this, and the design question I left open above now has data behind it rather than a judgement. **I checked whether `openbao-fetch` could simply be non-fatal by default**, which would have satisfied both criterion 2 and criterion 5 with no consumer edit at all. It cannot. I surveyed every use of the action across the fleet and looked at what follows it: - **Guarded** (a missing value fails loudly at the point of use): `ahmad/expiro` deploy, `ahmad/larda` deploy, `suhba/suhba-docs` publish and validate, `suhba/suhba-web` cms-pr, `ahmad/imamah` version. - **Unguarded**: everything else — including the `deploy` job of `ahmad/trip`, `ahmad/portfolio`, `bayan/kidpoints`, `bayan/stem`, `suhba/khitta`, `suhba/suhba-web` and `ahmad/imamah`. So a non-fatal default would let a deploy proceed with an empty `.env` on seven repositories: containers started with no `DATABASE_URL`, no error, a green run. That is a worse failure than the one this issue is about, and it is the "trades a loud failure for a quiet one" shape the fleet has been bitten by before. The fetcher stays fatal by default. **The blast radius of the actual problem, since I now have the numbers.** A `notify` job that fetches from OpenBao exists in **eight** repositories — `ahmad/portfolio`, `ahmad/trip`, `ahmad/imamah`, `bayan/kidpoints`, `bayan/stem`, `suhba/khitta`, `suhba/suhba-docs`, `suhba/suhba-web` — and in `portfolio` it is on `pr.yml` and `monitoring.yml` as well. Every one of those turns an OpenBao blip into a failed run for a build that was fine. **What I am building**, unchanged from the plan above except that item 1 is already done by `2bccd5e`: - `openbao-fetch` gains `required`, default `"true"`. With `required: false` a failure writes an empty output file, emits a `::warning::` naming the reason, and exits 0. - `templates/release.yml`'s notify job passes `required: false`. `notify-telegram` already treats an empty token as a skip, so the run continues and says why. **Criterion 5, stated plainly rather than claimed.** The central change reaches every consumer through `@v3` for the *jq* half, which is what has actually been failing. The tolerance half needs each consumer's copied `notify` job to gain one line, because the job is copied rather than called — I verified again that no consumer references a reusable workflow. I will list them on a follow-up issue rather than edit eight repositories inside this PR.

ahmad self-assigned this

2026-09-09 04:44:57 +00:00

ahmad added

state/in-progress

and removed

state/ready

labels

2026-09-09 04:44:57 +00:00

ahmad referenced this issue from a commit

2026-09-09 04:49:49 +00:00

fix: Let a notify job survive an unreachable vault

ahmad referenced this issue from a pull request that will close it,