amtronics/platform-actions

Watch

Fork 0

fix: Build the comment body without requiring jq #53

Merged

ahmad merged 1 commit from feature/52-report-without-jq into main

2026-09-06 01:34:22 +00:00

ahmad commented

2026-09-06 00:42:58 +00:00

Owner

Issue

Closes #52

Problem

report-job-failure built its comment body with jq and refused to post without it — command -v jq || { echo …; exit 0; }. jq is absent from node:22-bookworm, which is the image every consumer of this action runs in. This repository already knew that: release.yml says so in a comment and reaches for node instead, for exactly this reason.

Because the guard exits 0, the failure is invisible. The action reports success and posts nothing.

That is not hypothetical — this repository's own validate has been a silent no-op since #38 migrated it. It simply has not failed since, so nothing revealed it. The inline reporter it replaced used node, not curl + jq, precisely because of this image.

Solution

Prefer jq, fall back to node's JSON.stringify, and only give up when neither exists.

Both are real JSON encoders, which is the property that matters: jq --arg was chosen over hand-built JSON because a tail containing a quote, a backslash or a newline breaks string concatenation. JSON.stringify gives the same guarantee.

Review notes

Driven by extracting the run: body and running it under env -i with a PATH containing only the stubbed tools, so "not installed" is real rather than simulated, and with a stub curl that captures the payload that would have been POSTed. The log fixture contains a double quote, a backslash, a backtick and newlines.

tools available	exit	posted?
`jq` + `node`	0	yes — 187 bytes
`node` only	0	yes — 181 bytes
neither	0	no, with `Neither jq nor node is installed…`

The middle row is the fix; against the previous code that case posted nothing.

On "identical body", stated precisely. The raw JSON differs in whitespace — jq -n pretty-prints, JSON.stringify is compact — but the decoded body string is equal, and the tail round-trips intact through both paths:

decoded bodies equal: True
tail intact (jq / node): True / True
sample: '```
line1 "quoted"
back\slash and `tick`
last
```'

So the comment the API receives is the same; the bytes on the wire are not. I would rather say that than claim byte-identity.

The curl guard is unchanged — curl is present in these images.

check-actions.py passes, its own suite is 31/31, README lint is clean.

Risks and trade-offs

An image with neither jq nor node still posts nothing, and still exits 0. That is deliberate and unchanged: a reporter must not fail a job that already failed. It is now the only way to reach that branch, where before it was the normal case.

Once this releases, #46 can proceed — migrating ahmad/portfolio, ahmad/imamah and ahmad/expiro onto the action becomes safe, where today it would silently disable three working reporters.

### Issue Closes #52 ### Problem `report-job-failure` built its comment body with `jq` and refused to post without it — `command -v jq || { echo …; exit 0; }`. **`jq` is absent from `node:22-bookworm`**, which is the image every consumer of this action runs in. This repository already knew that: `release.yml` says so in a comment and reaches for `node` instead, for exactly this reason. Because the guard exits 0, the failure is invisible. The action reports success and posts nothing. **That is not hypothetical — this repository's own `validate` has been a silent no-op since #38 migrated it.** It simply has not failed since, so nothing revealed it. The inline reporter it replaced used `node`, not `curl` + `jq`, precisely because of this image. ### Solution Prefer `jq`, fall back to `node`'s `JSON.stringify`, and only give up when neither exists. Both are real JSON encoders, which is the property that matters: `jq --arg` was chosen over hand-built JSON because a tail containing a quote, a backslash or a newline breaks string concatenation. `JSON.stringify` gives the same guarantee. ### Review notes Driven by extracting the `run:` body and running it under `env -i` with a `PATH` containing **only** the stubbed tools, so "not installed" is real rather than simulated, and with a stub `curl` that captures the payload that would have been POSTed. The log fixture contains a double quote, a backslash, a backtick and newlines. | tools available | exit | posted? | | --- | --- | --- | | `jq` + `node` | 0 | yes — 187 bytes | | `node` only | 0 | **yes — 181 bytes** | | neither | 0 | no, with `Neither jq nor node is installed…` | The middle row is the fix; against the previous code that case posted nothing. **On "identical body", stated precisely.** The raw JSON differs in whitespace — `jq -n` pretty-prints, `JSON.stringify` is compact — but the decoded `body` string is **equal**, and the tail round-trips intact through both paths: ``` decoded bodies equal: True tail intact (jq / node): True / True sample: '``` line1 "quoted" back\slash and `tick` last ```' ``` So the comment the API receives is the same; the bytes on the wire are not. I would rather say that than claim byte-identity. The `curl` guard is unchanged — `curl` *is* present in these images. `check-actions.py` passes, its own suite is 31/31, README lint is clean. ### Risks and trade-offs An image with neither `jq` nor `node` still posts nothing, and still exits 0. That is deliberate and unchanged: a reporter must not fail a job that already failed. It is now the only way to reach that branch, where before it was the normal case. **Once this releases, #46 can proceed** — migrating `ahmad/portfolio`, `ahmad/imamah` and `ahmad/expiro` onto the action becomes safe, where today it would silently disable three working reporters.

ahmad added 1 commit

2026-09-06 00:42:58 +00:00

fix: Build the comment body without requiring jq

PR / validate (pull_request) Successful in 1m29s

Details

bb2c9c34bb

The action refused to post when jq was missing, and jq is absent from
node:22-bookworm — the image every consumer runs in. The guard exits 0, so
the reporter was a silent no-op wherever it was used, including this
repository's own validate since #38.

Prefer jq, fall back to node's JSON.stringify: both are real encoders, so a
tail containing a quote, a backslash or a newline survives either path.

ahmad changed title from ~~WIP: fix: Build the comment body without requiring jq~~ to fix: Build the comment body without requiring jq

2026-09-06 01:02:39 +00:00

ahmad commented

2026-09-06 01:02:40 +00:00

Author

Owner

Marked ready on bb2c9c3. validate green.

Verified against each acceptance criterion of #52, by extracting the run: body and running it under env -i with a PATH containing only stubbed tools — so "not installed" is genuinely absent rather than simulated — and a stub curl that captures the payload that would have been POSTed. The log fixture contains a double quote, a backslash, a backtick and newlines, because quoting is where this breaks.

node but no jq → the comment is posted. exit 0, payload captured, 181 bytes. Against the previous code this case posted nothing, which is the kill.
jq present → posted, body identical to the jq-built one. Posted, 187 bytes. The decoded body strings are equal; the raw JSON differs in whitespace only, because jq -n pretty-prints and JSON.stringify is compact. The API receives the same body; the bytes on the wire are not identical, and I would rather state that than claim they are.
A tail with a quote, a backslash and a newline renders intact. True on both paths — verified by decoding the captured payload and asserting the original fixture appears verbatim: '```\nline1 "quoted"\nback\\slash and `tick`\nlast\n```'.
Neither jq nor node → says so and exits 0 without failing the job. exit 0, nothing posted, Neither jq nor node is installed, so the failure comment cannot be built.

Criterion 5 — a comment appearing when this repository's own validate fails — cannot be checked here: it needs a released v3 and a genuinely failing validate. It is the one claim I am not making from this branch.

check-actions.py passes on all six action files, its own suite is 31/31, README lint clean.

A note on how the earlier version passed review. #38 migrated this repository's validate onto the action and was accepted as proving equivalence. It did not: validate runs in node:22-bookworm, the action needed jq, and the guard exits 0 — so the migration replaced a working node reporter with one that could not run there, and every pr.yml run since has passed, so nothing surfaced it. The equivalence that was demonstrated was of the output, not of the runtime. Worth stating because the same shape — a guard that exits 0 on a missing dependency — will read as success anywhere it appears.

Marked ready on `bb2c9c3`. `validate` green. **Verified against each acceptance criterion of #52**, by extracting the `run:` body and running it under `env -i` with a `PATH` containing **only** stubbed tools — so "not installed" is genuinely absent rather than simulated — and a stub `curl` that captures the payload that would have been POSTed. The log fixture contains a double quote, a backslash, a backtick and newlines, because quoting is where this breaks. 1. *`node` but no `jq` → the comment is posted.* `exit 0`, payload captured, 181 bytes. **Against the previous code this case posted nothing**, which is the kill. 2. *`jq` present → posted, body identical to the `jq`-built one.* Posted, 187 bytes. The decoded `body` strings are **equal**; the raw JSON differs in whitespace only, because `jq -n` pretty-prints and `JSON.stringify` is compact. The API receives the same body; the bytes on the wire are not identical, and I would rather state that than claim they are. 3. *A tail with a quote, a backslash and a newline renders intact.* True on both paths — verified by decoding the captured payload and asserting the original fixture appears verbatim: ``'```\nline1 "quoted"\nback\\slash and `tick`\nlast\n```'``. 4. *Neither `jq` nor `node` → says so and exits 0 without failing the job.* `exit 0`, nothing posted, `Neither jq nor node is installed, so the failure comment cannot be built.` Criterion 5 — a comment appearing when this repository's own `validate` fails — cannot be checked here: it needs a released `v3` and a genuinely failing `validate`. It is the one claim I am not making from this branch. `check-actions.py` passes on all six action files, its own suite is 31/31, README lint clean. **A note on how the earlier version passed review.** #38 migrated this repository's `validate` onto the action and was accepted as proving equivalence. It did not: `validate` runs in `node:22-bookworm`, the action needed `jq`, and the guard exits 0 — so the migration replaced a working `node` reporter with one that could not run there, and every `pr.yml` run since has passed, so nothing surfaced it. The equivalence that was demonstrated was of the *output*, not of the *runtime*. Worth stating because the same shape — a guard that exits 0 on a missing dependency — will read as success anywhere it appears.

ahmad_bot approved these changes

2026-09-06 01:33:08 +00:00

ahmad_bot left a comment

Approved. CI green. And this one found a real defect that I approved and should have caught.

The premise is correct, and I verified it from the image definitions rather than taking it. node:22-bookworm derives from buildpack-deps:bookworm → scm → curl → debian. The complete package list across that chain is ca-certificates curl gnupg netbase sq wget git mercurial openssh-client subversion procps plus the compiler/-dev set — no jq. And this repository's own validate job runs container: node:22-bookworm and calls ./report-job-failure on if: failure(). So the reporter has been unable to build a body here since #38, exiting 0 with a message on stderr that nobody reads.

That is my miss, and worth naming precisely because the shape recurs. I reviewed #45, and in the follow-up I raised the jq/curl asymmetry and asked for a symmetric curl guard — I even quoted the README line about a node-only image needing both installed. I tested that the jq guard behaved correctly: exits 0, prints its message, never posts. What I never asked was which side of that guard the actual consumer was on. I treated "no jq" as a hypothetical about future adopters while it was already true of the only consumer in the repository, sitting three files away in pr.yml.

Verifying that a guard works is not the same as verifying that the branch it guards is the unreached one. A guard whose fallback path is the live path is a no-op wearing a guard's clothes.

The fix, driven from this branch's own YAML under env -i with only the named tools on PATH:

jq + node    exit 0   built, 187 bytes   posts
node only    exit 0   built, 181 bytes   posts      <- previously posted nothing
neither      exit 0   not built          "Neither jq nor node is installed…"

Your care about "identical body" was the right call. The encodings differ — jq -n pretty-prints, JSON.stringify is compact, hence 187 against 181 — but I decoded both and the body strings are equal, with the tail surviving intact through both paths including a double quote, a backslash, a backtick and newlines:

decoded bodies equal: True
sample: '```\nline1 "quoted"\nback\\slash and `tick`\nlast\n```'

Claiming byte-identity there would have been wrong and would have been believed. Saying the decoded bodies match, and why the bytes do not, is the accurate claim.

Keeping both encoders rather than dropping jq is also right: both are real JSON encoders, which is the property that matters, and hand-built JSON would break on exactly the characters your fixture contains.

Approved. CI green. And this one found a real defect that I approved and should have caught. **The premise is correct, and I verified it from the image definitions rather than taking it.** `node:22-bookworm` derives from `buildpack-deps:bookworm` → `scm` → `curl` → `debian`. The complete package list across that chain is `ca-certificates curl gnupg netbase sq wget git mercurial openssh-client subversion procps` plus the compiler/-dev set — **no `jq`**. And this repository's own `validate` job runs `container: node:22-bookworm` and calls `./report-job-failure` on `if: failure()`. So the reporter has been unable to build a body here since `#38`, exiting 0 with a message on stderr that nobody reads. **That is my miss, and worth naming precisely because the shape recurs.** I reviewed `#45`, and in the follow-up I raised the `jq`/`curl` asymmetry and asked for a symmetric `curl` guard — I even quoted the README line about a node-only image needing both installed. I tested that the `jq` guard *behaved* correctly: exits 0, prints its message, never posts. What I never asked was **which side of that guard the actual consumer was on**. I treated "no jq" as a hypothetical about future adopters while it was already true of the only consumer in the repository, sitting three files away in `pr.yml`. Verifying that a guard works is not the same as verifying that the branch it guards is the unreached one. A guard whose *fallback* path is the live path is a no-op wearing a guard's clothes. **The fix, driven from this branch's own YAML under `env -i` with only the named tools on `PATH`:** ``` jq + node exit 0 built, 187 bytes posts node only exit 0 built, 181 bytes posts <- previously posted nothing neither exit 0 not built "Neither jq nor node is installed…" ``` **Your care about "identical body" was the right call.** The encodings differ — `jq -n` pretty-prints, `JSON.stringify` is compact, hence 187 against 181 — but I decoded both and the `body` strings are equal, with the tail surviving intact through both paths including a double quote, a backslash, a backtick and newlines: ``` decoded bodies equal: True sample: '```\nline1 "quoted"\nback\\slash and `tick`\nlast\n```' ``` Claiming byte-identity there would have been wrong and would have been believed. Saying the decoded bodies match, and why the bytes do not, is the accurate claim. Keeping both encoders rather than dropping `jq` is also right: both are real JSON encoders, which is the property that matters, and hand-built JSON would break on exactly the characters your fixture contains.

ahmad merged commit e9f1bddf9f into main

2026-09-06 01:34:21 +00:00

ahmad referenced this pull request from a commit

2026-09-06 01:34:23 +00:00

fix: Build the comment body without requiring jq (#53)

ahmad deleted branch feature/52-report-without-jq

2026-09-06 01:34:25 +00:00