mirror of
https://github.com/alexbelgium/hassio-addons.git
synced 2026-09-23 01:53:59 +02:00
* fix(skill): stop the workflow scripts reporting verdicts they have not established
Every defect here is the same species: a check printed as passed that never ran, or
never proved what it claims. All six were reproduced before and after.
validate.sh
- `run`/`finish` were selected by filename and fed to bash -n and shellcheck. 25 of the
97 such files here are `#!/usr/bin/execlineb`, so 21 add-ons -- calibre_web and seerr
among them -- reported `local validation FAILED` and a wall of parse errors no matter
what the diff contained. One list, filtered by shebang, now feeds both checks; seerr
went from 15 shellcheck findings and two bash -n failures to the single finding the
test diff actually introduced.
- `grep -q "$ADDON/CHANGELOG.md"` was unanchored with `.` as a wildcard, so a diff
bumping zzz_archived_overseerr/CHANGELOG.md reported seerr's as updated -- a false
green on the one hard CI gate. Five such name collisions exist in this repo
(also birdnet-pi/battybirdnet-pi, mealie/social_to_mealie, plex/spotify_to_plex).
`grep -Fxq`.
- The --vs-master section skipped any file absent from origin/master, so a newly added
script's findings -- all of which are by definition added by the diff -- were never
reported, under a line reading "your diff introduced no new lint findings". An added
file now compares against an empty base. A deleted one is skipped: it was being linted
at a path that no longer exists, which turned every deletion into a fabricated
`openBinaryFile: does not exist` finding.
- That loop ran as the right-hand side of a pipe, so it could not have reached `fail`
even had it tried. It now runs in this shell and new findings fail the script; the
all-clear line is printed only when nothing was listed. Findings that merely moved
lines still cancel -- the comparison strips file:line:col before comm.
- `bash -n ok` stood for an add-on with no shell files at all, and hadolint could print
`clean` directly after printing findings (`A && {...} || C` with pipefail). Counted
and branched properly.
preflight.sh
- `MATCH -- this checkout corresponds to the running image` was concluded from
config.yaml's version equalling $BUILD_VERSION. Version is bumped once per PR, so any
later commit or a dirty tree matches while differing from what runs -- the one
conclusion the script exists to establish was the one it overstated.
pr_review.sh
- `watch` exhausting its minutes with checks still pending fell out of the loop and
exited 0, reporting success for checks that never settled. Unsettled is now exit 2.
Checks reported as `skipping` still count as passing, which is correct -- for this PR
itself, three jobs skipped because no */config.* changed, and that is the right
outcome, not a failure. But a skipped job tested nothing, so `watch` now says so.
Reviewed by Codex (gpt-5.6-sol), which corrected two claims in the audit behind this:
the .templates CHANGELOG assertion (CI skips the gate entirely for a template-only PR,
so that fix is not in this diff) and a tradeoff that did not exist. The deleted-file and
watch-timeout defects are its finds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(skill): address review — cwd independence, no pass verdict for an empty check
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
117 lines
6.0 KiB
Bash
Executable File
117 lines
6.0 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Work through bot review comments on a PR. Resolving a thread needs the GraphQL API (the REST
|
|
# API cannot do it), which is the only reason this script exists.
|
|
#
|
|
# pr_review.sh list <PR> every inline comment, grouped
|
|
# pr_review.sh status <PR> checks + unresolved thread count
|
|
# pr_review.sh reply <PR> <COMMENT_ID> <text|@file>
|
|
# pr_review.sh resolve <PR> <THREAD_ID...|--all> --all = every unresolved, asks first
|
|
# pr_review.sh watch <PR> [minutes] poll checks (run this backgrounded)
|
|
#
|
|
# Reviewers seen here: coderabbitai (deepest; reviews ~9 min after open, or on
|
|
# "@coderabbitai review"), chatgpt-codex-connector, Copilot, Codacy.
|
|
#
|
|
# Verify every claim before agreeing. Bots are frequently right and occasionally confidently
|
|
# wrong; a reproduction takes a minute and decides it either way. Push back with evidence when
|
|
# you are right — a resolved-but-wrong thread is worse than an open one.
|
|
set -uo pipefail
|
|
|
|
REPO="${HASSIO_REPO:-}"
|
|
[ -z "$REPO" ] && REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner 2> /dev/null)
|
|
[ -z "$REPO" ] && { echo "cannot determine repo; set HASSIO_REPO=owner/name" >&2; exit 1; }
|
|
echo "repo: $REPO" >&2
|
|
CMD="${1:-}"; PR="${2:-}"
|
|
[ -z "$CMD" ] || [ -z "$PR" ] && { sed -n '2,16p' "$0" | sed 's/^# \?//'; exit 1; }
|
|
|
|
case "$CMD" in
|
|
list)
|
|
echo "== inline comments on #$PR =="
|
|
gh api "repos/$REPO/pulls/$PR/comments" --paginate \
|
|
--jq 'sort_by(.created_at)[] | "=== [\(.id)] \(.user.login) | \(.path):\(.line // .original_line) ===\n\(.body)\n"'
|
|
echo "== review bodies =="
|
|
gh api "repos/$REPO/pulls/$PR/reviews" \
|
|
--jq '.[] | select(.body != "") | "--- \(.user.login) (\(.state)) ---\n\(.body[0:4000])\n"'
|
|
;;
|
|
status)
|
|
gh pr checks "$PR" 2>&1 | head -15
|
|
echo
|
|
gh api graphql -f query="{repository(owner:\"${REPO%%/*}\",name:\"${REPO##*/}\"){pullRequest(number:$PR){reviewThreads(first:50){nodes{id isResolved path comments(first:1){nodes{author{login}}}}}}}}" \
|
|
--jq '.data.repository.pullRequest.reviewThreads.nodes[] | "\(if .isResolved then "resolved" else "OPEN " end) \(.id) \(.comments.nodes[0].author.login) \(.path)"'
|
|
;;
|
|
reply)
|
|
ID="${3:?comment id}"; BODY="${4:?text or @file}"
|
|
if [ "${BODY#@}" != "$BODY" ]; then
|
|
out=$(gh api "repos/$REPO/pulls/$PR/comments/$ID/replies" -F body=@"${BODY#@}" --jq '.id' 2>&1)
|
|
rc=$?
|
|
else
|
|
out=$(gh api "repos/$REPO/pulls/$PR/comments/$ID/replies" -f body="$BODY" --jq '.id' 2>&1)
|
|
rc=$?
|
|
fi
|
|
# Silently "succeeding" here is worse than failing: a later session reads the transcript and
|
|
# believes a reviewer was answered when they were not.
|
|
if [ "$rc" -eq 0 ] && [ -n "$out" ]; then
|
|
echo "replied to $ID (comment $out)"
|
|
else
|
|
echo "FAILED to reply to $ID: $out" >&2
|
|
echo " (top-level review bodies have different ids and cannot take replies here)" >&2
|
|
exit 1
|
|
fi
|
|
;;
|
|
resolve)
|
|
shift 2
|
|
ids="$*"
|
|
if [ "${ids:-}" = "--all" ]; then
|
|
echo "About to resolve EVERY unresolved thread. Only do this if you have read and"
|
|
echo "answered each one — a resolved-but-wrong thread is worse than an open one."
|
|
gh api graphql -f query="{repository(owner:\"${REPO%%/*}\",name:\"${REPO##*/}\"){pullRequest(number:$PR){reviewThreads(first:100){nodes{isResolved path comments(first:1){nodes{author{login} body}}}}}}}" \
|
|
--jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved==false) | " - \(.comments.nodes[0].author.login) \(.path): \(.comments.nodes[0].body[0:90])"'
|
|
printf 'Type "yes" to resolve all: '; read -r ok
|
|
[ "$ok" = "yes" ] || { echo "aborted"; exit 1; }
|
|
ids=""
|
|
elif [ -z "$ids" ]; then
|
|
echo "usage: pr_review.sh resolve <PR> <THREAD_ID...> (or --all, with confirmation)" >&2
|
|
echo "resolve each thread as you answer it; get ids from: pr_review.sh status $PR" >&2
|
|
exit 1
|
|
fi
|
|
if [ -z "$ids" ]; then
|
|
ids=$(gh api graphql -f query="{repository(owner:\"${REPO%%/*}\",name:\"${REPO##*/}\"){pullRequest(number:$PR){reviewThreads(first:50){nodes{id isResolved}}}}}" \
|
|
--jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved==false) | .id')
|
|
fi
|
|
[ -z "$ids" ] && { echo "nothing unresolved"; exit 0; }
|
|
rfail=0
|
|
for id in $ids; do
|
|
r=$(gh api graphql -f query="mutation{resolveReviewThread(input:{threadId:\"$id\"}){thread{isResolved}}}" \
|
|
--jq '.data.resolveReviewThread.thread.isResolved' 2>&1)
|
|
echo " $id -> $r"
|
|
[ "$r" = "true" ] || rfail=1
|
|
done
|
|
# exiting 0 on a failed mutation would let a session believe threads were resolved
|
|
exit "$rfail"
|
|
;;
|
|
watch)
|
|
MINS="${3:-180}" # the addon build alone has taken ~3h; 20 was far too short
|
|
wfail=2 # not 0: running out of minutes with checks still pending is not a pass
|
|
for i in $(seq 1 "$MINS"); do
|
|
c=$(gh pr checks "$PR" 2> /dev/null | awk '{print $1"="$2}' | tr '\n' ' ')
|
|
if [ -z "$c" ]; then
|
|
# Normal in the first minutes after `gh pr create`, and also whenever gh errors.
|
|
# Calling that "settled" would report success for checks that never ran.
|
|
echo "[$i] no checks reported yet (gh returned nothing) — still waiting"
|
|
sleep 60; continue
|
|
fi
|
|
echo "[$i] $c"
|
|
case "$c" in
|
|
*pending*) sleep 60 ;;
|
|
*fail* | *error* | *cancel*) echo "settled — with FAILURES (see above)"; wfail=1; break ;;
|
|
*) echo "settled — all passing"; wfail=0; break ;;
|
|
esac
|
|
done
|
|
[ "$wfail" -eq 2 ] && echo "gave up after ${MINS}m, checks still unsettled — NOT a pass"
|
|
# A PR touching no */config.* skips the CHANGELOG, linter and build jobs outright (#3018).
|
|
case "${c:-}" in *skipping*) echo " ...of which some were SKIPPED — a skipped job tested nothing" ;; esac
|
|
echo "note: long queues here are usually account runner contention, not your diff."
|
|
exit "$wfail"
|
|
;;
|
|
*) echo "unknown: $CMD"; exit 1 ;;
|
|
esac
|