chore: Extract the failure-tail reporter into a shared action #38

Closed
opened 2026-09-04 11:15:14 +00:00 by ahmad · 2 comments
Owner

Context

Four jobs across three repositories now carry the same hand-copied twenty lines: capture a job's output with tee, and on failure post the tail as a pull request comment. It exists because Forgejo serves no job logs over its API, so a red check is otherwise a bare word.

ahmad/portfolio                 validate
ahmad/expiro                    image, validate
amtronics/platform-actions      validate
ahmad/imamah                    none yet — #147 would make five

The copies have already drifted, and one of them is wrong. expiro's image job uses exec > >(tee -a log) 2>&1; the newer ones use set -o pipefail with cmd 2>&1 | tee -a log. Both preserve the command's exit status, but the process-substitution form's tee outlives the shell, so the log can be unflushed — or absent — when a later step reads it. Measured on 2026-09-03: the file did not exist immediately after the step returned. expiro#27 tracks that copy specifically.

A second drift: the reporter's fallback for a missing log was unreachable in its first two copies, because tail -c 40000 log > log.tail || true creates the file before tail fails, so the read returns an empty string rather than throwing. Fixed in expiro#26; whether every copy has it is exactly the question a shared action removes.

This is the shape the repository exists for. An action referenced by every pipeline is the difference between fixing a bug once and finding four copies of it.

Scope

  • In: a report-job-failure composite action taking the log path and the pull request number, posting the tail on failure; the README entry; migrating the consumers.
  • Out: the capture half — a job must still tee its own output, since an action cannot wrap steps it does not own. This action covers the reporting only.
  • Out: security-scan's internal capture (platform-actions#29), which already exposes log-path and would become a consumer.

Acceptance criteria

  • Given a job that failed with output in the log, when the action runs, then a comment on the pull request holds the tail.
  • Given the log is missing or empty, when the action runs, then the comment says so rather than showing an empty code fence.
  • Given the comment cannot be posted, when the action runs, then the job's own failure is still what the run reports — a reporting failure must not mask or replace it.
  • Given a consumer migrates to the action, when its job fails, then the comment is the same as before the migration.
  • Given the README, when a new repository adds a job, then it says to use pipefail with a pipeline and not the exec > >(tee) form, and why.

Notes

The third criterion is the one to be careful about. The reporter runs if: failure(), and a curl that fails there must not turn a clear red into a confusing one — the build failure is the news.

### Context Four jobs across three repositories now carry the same hand-copied twenty lines: capture a job's output with `tee`, and on failure post the tail as a pull request comment. It exists because Forgejo serves no job logs over its API, so a red check is otherwise a bare word. ``` ahmad/portfolio validate ahmad/expiro image, validate amtronics/platform-actions validate ahmad/imamah none yet — #147 would make five ``` **The copies have already drifted, and one of them is wrong.** `expiro`'s `image` job uses `exec > >(tee -a log) 2>&1`; the newer ones use `set -o pipefail` with `cmd 2>&1 | tee -a log`. Both preserve the command's exit status, but the process-substitution form's `tee` outlives the shell, so the log can be unflushed — or absent — when a later step reads it. Measured on 2026-09-03: the file did not exist immediately after the step returned. `expiro#27` tracks that copy specifically. A second drift: the reporter's fallback for a missing log was unreachable in its first two copies, because `tail -c 40000 log > log.tail || true` creates the file before `tail` fails, so the read returns an empty string rather than throwing. Fixed in `expiro#26`; whether every copy has it is exactly the question a shared action removes. This is the shape the repository exists for. An action referenced by every pipeline is the difference between fixing a bug once and finding four copies of it. ### Scope - In: a `report-job-failure` composite action taking the log path and the pull request number, posting the tail on failure; the README entry; migrating the consumers. - Out: the *capture* half — a job must still `tee` its own output, since an action cannot wrap steps it does not own. This action covers the reporting only. - Out: `security-scan`'s internal capture (`platform-actions#29`), which already exposes `log-path` and would become a consumer. ### Acceptance criteria - [ ] Given a job that failed with output in the log, when the action runs, then a comment on the pull request holds the tail. - [ ] Given the log is missing or empty, when the action runs, then the comment says so rather than showing an empty code fence. - [ ] Given the comment cannot be posted, when the action runs, then the job's own failure is still what the run reports — a reporting failure must not mask or replace it. - [ ] Given a consumer migrates to the action, when its job fails, then the comment is the same as before the migration. - [ ] Given the README, when a new repository adds a job, then it says to use `pipefail` with a pipeline and not the `exec > >(tee)` form, and why. ### Notes The third criterion is the one to be careful about. The reporter runs `if: failure()`, and a `curl` that fails there must not turn a clear red into a confusing one — the build failure is the news.
Author
Owner

One sentence in the context is wrong: "It exists because Forgejo serves no job logs over its API." It does serve them —

GET /api/v1/repos/{owner}/{repo}/actions/jobs/{job_id}/logs

— and the reason I concluded otherwise is that /actions/tasks returns task ids while that endpoint wants a job id, which comes from /actions/runs/{run_id}/jobs. I had been passing the wrong id and reading the 404 as "no such API".

This does not weaken the case for extracting the action. That case is the drift, and the drift is unaffected: four hand-copied copies, one of them using the exec > >(tee …) form whose log can be absent when a later step reads it, and a fallback that was unreachable in the first two copies. Consolidating one correct implementation is worth doing whether or not the log is reachable by other means — arguably more so, since the copies are now the only thing that can be quietly wrong.

What does change is the framing of the benefit. The reporter is a convenience — the failure lands on the pull request, next to the reviewer — rather than the only way to see a red job. Worth saying plainly in the extracted action's README so nobody re-derives my mistake from the code.

I have corrected the same sentence on ahmad/imamah#147, which is the other issue that leans on it.

One sentence in the context is wrong: *"It exists because Forgejo serves no job logs over its API."* It does serve them — ``` GET /api/v1/repos/{owner}/{repo}/actions/jobs/{job_id}/logs ``` — and the reason I concluded otherwise is that `/actions/tasks` returns *task* ids while that endpoint wants a *job* id, which comes from `/actions/runs/{run_id}/jobs`. I had been passing the wrong id and reading the 404 as "no such API". **This does not weaken the case for extracting the action.** That case is the drift, and the drift is unaffected: four hand-copied copies, one of them using the `exec > >(tee …)` form whose log can be absent when a later step reads it, and a fallback that was unreachable in the first two copies. Consolidating one correct implementation is worth doing whether or not the log is reachable by other means — arguably more so, since the copies are now the *only* thing that can be quietly wrong. What does change is the framing of the benefit. The reporter is a convenience — the failure lands on the pull request, next to the reviewer — rather than the only way to see a red job. Worth saying plainly in the extracted action's README so nobody re-derives my mistake from the code. I have corrected the same sentence on `ahmad/imamah#147`, which is the other issue that leans on it.
ahmad self-assigned this 2026-09-05 06:03:34 +00:00
Author
Owner

Taking this. Three files in this repository: a new report-job-failure/action.yml, this repo's own pr.yml migrated onto it, and the README entry.

Migrating this repository's own reporter is how the fourth criterion gets met without a four-repo pull request. "Migrating the consumers" spans expiro, portfolio and imamah as well, and a single change touching four repositories could not be reviewed or reverted as a unit — those follow as their own issues. This repo has a consumer of its own, so equivalence can be demonstrated here.

That consumer has the bug the shared action must not inherit. Its reporter reads:

tail -c 30000 /tmp/validate.log > /tmp/report.tail 2>/dev/null || true
... fs.existsSync("/tmp/report.tail") ? readFileSync(...) : "(no output was captured…)"

The redirection creates the file before tail can fail, so existsSync is always true, the read returns "", and the fallback is unreachable — an empty code fence, exactly the defect fixed in ahmad/expiro#27. So this is not only a consolidation: it fixes a live reporter here.

The third criterion is the one to be careful about. A reporter that cannot post must not turn a clear red into a confusing one. The action will not use set -e, will not fail the step on a non-2xx, and will say on stderr that the comment could not be posted — the job's own failure stays the news.

Verification. The action driven against a local HTTP server with only the hostname substituted, over the four log states (absent, empty, populated, stale tail) plus a post that fails; a mutation per guard; and bash -n over every run: body through this repo's own checker, which the new action must also satisfy.

One thing I will decide rather than ask: curl versus node's fetch. imamah posts with fetch because its container is node-based; the others use curl. A shared action cannot assume either, so it will use curl — it is what a composite action can rely on across the images here — and I will note in the README that a node-only image needs the caller to install it, rather than leaving the divergence undocumented.

Taking this. Three files in this repository: a new `report-job-failure/action.yml`, this repo's own `pr.yml` migrated onto it, and the README entry. **Migrating this repository's own reporter is how the fourth criterion gets met without a four-repo pull request.** "Migrating the consumers" spans `expiro`, `portfolio` and `imamah` as well, and a single change touching four repositories could not be reviewed or reverted as a unit — those follow as their own issues. This repo has a consumer of its own, so equivalence can be demonstrated here. **That consumer has the bug the shared action must not inherit.** Its reporter reads: ```sh tail -c 30000 /tmp/validate.log > /tmp/report.tail 2>/dev/null || true ... fs.existsSync("/tmp/report.tail") ? readFileSync(...) : "(no output was captured…)" ``` The redirection creates the file before `tail` can fail, so `existsSync` is always true, the read returns `""`, and the fallback is unreachable — an empty code fence, exactly the defect fixed in `ahmad/expiro#27`. So this is not only a consolidation: it fixes a live reporter here. **The third criterion is the one to be careful about.** A reporter that cannot post must not turn a clear red into a confusing one. The action will not use `set -e`, will not fail the step on a non-2xx, and will say on stderr that the comment could not be posted — the job's own failure stays the news. **Verification.** The action driven against a local HTTP server with only the hostname substituted, over the four log states (absent, empty, populated, stale tail) plus a post that fails; a mutation per guard; and `bash -n` over every `run:` body through this repo's own checker, which the new action must also satisfy. **One thing I will decide rather than ask:** `curl` versus node's `fetch`. `imamah` posts with `fetch` because its container is node-based; the others use `curl`. A shared action cannot assume either, so it will use `curl` — it is what a composite action can rely on across the images here — and I will note in the README that a node-only image needs the caller to install it, rather than leaving the divergence undocumented.
ahmad 2026-09-05 07:03:20 +00:00
Sign in to join this conversation.
No description provided.