mirror of
https://github.com/alexbelgium/hassio-addons.git
synced 2026-09-09 11:19:10 +02:00
bcde474c15
* skill(hassio-addon-workflow): stop pr_review.sh watch reporting a false pass
`gh pr checks` emits TAB-separated columns, but `watch` parsed it with awk's
default field splitting. Every check name containing a space was truncated to
its first word and the state column was never read:
Codacy Static Code Analysis<TAB>fail -> Codacy=Static
Addon linting (wger)<TAB>pass -> Addon=linting
Test addon build (wger)<TAB>pending -> Test=addon
All three blocking gates have multi-word names, so the `case` matched neither
*fail* nor *pending* and fell through to "settled - all passing". That is a
false pass from the one command whose job is to report CI truthfully: #3044
was called green with Codacy red, and #3042 was called green while the HA
add-on linter was failing. A build that had not started would also have read
as a pass.
- parse with `awk -F'\t'`
- judge the state column alone, never the joined name=state text, so a check
named e.g. `flaky-fail-detector` cannot read as a failure
- allowlist the good states (pass/skipping/pending) and treat anything
unrecognised as a failure, so a new state cannot reach the passing branch
- name the checks that failed instead of only saying FAILURES
- keep waiting when only advisory checks have reported
Codacy is red on essentially every add-on PR here (#3019, #3044 and #3050 all
merged with it failing; master has no branch protection), so it is excluded
from the verdict but printed every poll and called out explicitly on settle.
Agreed with the maintainer. It is a denylist of known noise rather than an
allowlist of gates, so a job added to CI later counts as blocking by default.
Verified against real PRs: #3042 (blocking linter failure) now exits 1 and
names the check where it previously exited 0; #3018/#3019/#3044/#3050 report
correctly; pending, advisory-only, unknown-state and empty-output cases
checked against a stubbed gh. shellcheck clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* skill: record PR number in the traps entry
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* skill: address review — skipped-checks wording, document TSV contract
CodeRabbit (correct): the header claimed exit 0 means "every blocking check
passed", but a skipped gate also yields 0. A PR touching no add-on skips all
three gates, so that wording overstated what a 0 means. Reworded; the runtime
warning about skipped jobs was already there.
Copilot recommended switching to `gh pr checks --json`. Not applied: that flag
does not exist before gh 2.36 and 2.23 ships in this add-on, where it fails
with `unknown flag: --json`. Its premises are also wrong for the path the
script takes — piped output carries no header and uses real tabs; the aligned
ANSI table is the TTY renderer, which $(... | awk) never gets. Documented the
non-TTY contract and the gh-version constraint in the comment and traps.md so
this is not "corrected" back into a break later.
The underlying worry — a format change reintroducing a false pass — is already
answered by the allowlist design, now verified explicitly: a header row lands
in the failure branch (exit 1) and a space-aligned table parses to zero rows,
so watch keeps waiting. Neither can return 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: BirdNET-Go Addon Builder <addon-builder@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>