report_job_log.py always says the job failed, even when it passed #8

New Issue

claude · 2026-09-07T18:23:57Z

claude commented

2026-09-07 18:23:57 +00:00

report_job_log.py:148 hardcodes the word failed into the comment header:

body = (f"**`{args.job}` failed**{f' on `{sha}`' if sha else ''}.\n\n"

Every current caller guards it with if: failure(), so today the wording is
always true and this is latent, not broken.

It stopped being purely theoretical during weblib-viewer#10. A throwaway probe
job ran the script with an unguarded step to read the runner's egress back —
job logs still 500 on this Gitea, so posting the tail as a comment is the only
way to see anything — and the successful run posted a comment that reads as
a failure report. Anyone scrolling that PR would conclude the probe had failed
when it had passed.

Fix

Take the outcome as an argument rather than assuming it, e.g. --status failed|passed (or read ${{ job.status }} at the call site), and pick the
header from it. Keep failed as the default so no existing caller changes
behaviour.

Worth doing because the interesting uses of this script are exactly the ones
that want if: always() — a probe, or a job whose output is the point rather
than its exit code. Right now those all lie in the header.

Filed from weblib-viewer#10; Type/Bug because the output is wrong, Priority/Low
because no current caller can hit it.

`report_job_log.py:148` hardcodes the word *failed* into the comment header: ```python body = (f"**`{args.job}` failed**{f' on `{sha}`' if sha else ''}.\n\n" ``` Every current caller guards it with `if: failure()`, so today the wording is always true and this is latent, not broken. It stopped being purely theoretical during weblib-viewer#10. A throwaway probe job ran the script with an unguarded step to read the runner's egress back — job logs still 500 on this Gitea, so posting the tail as a comment is the only way to see anything — and the **successful** run posted a comment that reads as a failure report. Anyone scrolling that PR would conclude the probe had failed when it had passed. ## Fix Take the outcome as an argument rather than assuming it, e.g. `--status failed|passed` (or read `${{ job.status }}` at the call site), and pick the header from it. Keep `failed` as the default so no existing caller changes behaviour. Worth doing because the interesting uses of this script are exactly the ones that want `if: always()` — a probe, or a job whose *output* is the point rather than its exit code. Right now those all lie in the header. Filed from weblib-viewer#10; `Type/Bug` because the output is wrong, `Priority/Low` because no current caller can hit it.

claude added the

labels 2026-09-07 18:24:04 +00:00

claude referenced this issue from a commit

2026-09-08 07:54:34 +00:00

Take the job's outcome as an argument, not as an assumption

claude referenced this issue

2026-09-08 07:55:20 +00:00