Skip to content

fix: an unknown verdict now names the line the runner flagged - #8

Merged
PabloCodes7 merged 1 commit into
mainfrom
fix/runner-errors-on-unknown
Jul 14, 2026
Merged

fix: an unknown verdict now names the line the runner flagged#8
PabloCodes7 merged 1 commit into
mainfrom
fix/runner-errors-on-unknown

Conversation

@PabloCodes7

Copy link
Copy Markdown
Contributor

The bug

The action pinned patchrail>=0.5.0,<0.6.0, so its users never received the 0.6.0 precision fixes. Worse, on a log no rule matches it annotated a red run with unknown (confidence 0.15), subsystem unknown, and "inspect CI log and run the failing job locally" — nothing the user would not have known by never running the action at all.

patchrail 0.6.0 hands back the runner's own ##[error] line for the failing step in runner_errors. The action was already reading that JSON and dropping the field on the floor.

The fix

  • action.yml: range → patchrail>=0.6.0,<0.7.0. The floor moves for the same reason it moved to 0.5.0: below it, a tool the job merely named (a GRADLE_HOME= line, a Collecting mypy from pip, a tool named inside a filename) gets read as the cause of death and annotated as a confident wrong answer.
  • scripts/annotate.py: surface runner_errors — first line in the annotation, all of them in the job summary, above the generic "reproduce it locally" advice, since on unknown it is the only line worth reading.

The class stays unknown and the confidence stays 0.15: an annotation says where the job died, not why. A log that does classify is byte-for-byte unchanged, and so is any run pinned to a pre-0.6.0 patchrail via the patchrail-version input (no such key → nothing reported).

Before / after, on the real psf/requests log that motivated the patchrail fix:

- unknown (confidence 0.15) — guide: getpatchrail.com/fix
+ unknown (confidence 0.15) — runner reported: "github-token" length must be
+ less than or equal to 100 characters long — guide: getpatchrail.com/fix

Security

These lines are log text from a build any PR author can write. GitHub decodes %0A inside a workflow command back into a newline, so a log containing %0A::error::… would close our ::warning and forge a command of the author's choosing — % is escaped first, on the way out. In the job summary the text stays inside its code span. Both are tested; removing the escape turns test_a_log_line_cannot_forge_a_second_annotation red.

Tests

tests/test_runner_errors.py (12 new): the line reaches the annotation and the summary, it outranks the generic advice, unknown stays unknown, a classified result is untouched, the annotation stays on one line, the injection and code-span escapes hold, a matrix build cannot flood the summary, a 5000-char stack trace is truncated, and junk in the field (a bare string, a dict, an int, null) is ignored rather than annotated.

Full suite: 35 passed against patchrail 0.6.0. Verified end-to-end through the real explain → annotate path, not just the unit tests.

The action installed `patchrail>=0.5.0,<0.6.0` and read six fields out of the
ci-result. On a log no rule matched, that produced the worst annotation it can
produce: `unknown (confidence 0.15)`, subsystem `unknown`, and the advice to
"inspect CI log and run the failing job locally" — on a red run, from a tool the
user installed precisely so they would not have to.

patchrail 0.6.0 reports the runner's own `##[error]` line for the failing step in
`runner_errors`. psf/requests run 29295524780 died on a single self-explanatory
line — `"github-token" length must be less than or equal to 100 characters long`
— and the action had that string in the JSON it was already reading, and dropped
it on the floor.

Raise the range to `patchrail>=0.6.0,<0.7.0` and surface the field: first line in
the annotation, all of them in the job summary, above the generic advice. The
class stays `unknown` and the confidence stays 0.15 — an annotation says where the
job died, not why, and PatchRail still does not pretend to recognize a log it does
not recognize. A log that classifies is byte-for-byte unchanged.

The floor moves to 0.6.0 for the same reason it moved to 0.5.0: below it, a tool
the job merely named — a `GRADLE_HOME=` line, a `Collecting mypy` from pip, a tool
named inside a filename — is read as the cause of death and annotated as a
confident wrong answer.

These lines are log text from a build any PR author can write, so they are
untrusted. GitHub decodes `%0A` in a workflow command back into a newline, so a
log containing `%0A::error::...` would close our warning and forge a command of
the author's choosing; `%` is escaped first, on the way out. In the job summary
they stay inside their code span. Both are tested, along with the cap that keeps a
matrix build's worth of annotations from flooding the summary.
@PabloCodes7
PabloCodes7 merged commit ba7eac1 into main Jul 14, 2026
5 checks passed
@PabloCodes7
PabloCodes7 deleted the fix/runner-errors-on-unknown branch July 14, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant