051f54111b
Both found by ChatGPT Codex on PR #133, both invisible on macOS, both arriving as exit 2 with no output -- jq's answer to an invalid --argjson, so a shell fault at the top of the script wears jq's exit code. `local n="$1" name="$2" f="$DIR/$n.$name.json"` creates every name before running the assignments on bash 5, so $n is unbound in the third clause and set -u kills the shell. The dead command substitution then hands --argjson an empty string. Works on macOS bash 3.2: the suite passed 19/19 locally and 2/19 in CI on the same commit. Each local now gets its own statement. `gh api --paginate` emits one JSON array PER PAGE, so a comment thread past the page size is several concatenated documents that --argjson rejects the same way. Both fetch paths now flatten through one pages_to_array (`jq -s 'add // []'`), which is a no-op on a single page. The fixture path shares that helper, which is the only reason a fixture can test it: a fixture holding a single array would exercise a shape the live fetch never produces. Three cases added, and removing the helper from the fixture path fails exactly those three, the wait case with the live exit-2 signature. Suite is 22 cases. No PR in this repo has yet had enough inline comments to reach a second page, so nothing but the test will see that one before a user does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
174 lines
7.3 KiB
Bash
Executable File
174 lines
7.3 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Prints everything a reviewer may have written on a PR -- review bodies,
|
|
# INLINE review comments and issue-level comments -- and optionally waits for
|
|
# one against the current head. See context/code/pr-review.md.
|
|
set -euo pipefail
|
|
|
|
usage() {
|
|
cat >&2 <<'EOF'
|
|
usage: pr-reviews <pr-number> [--wait <sec>] [--interval <sec>]
|
|
|
|
--wait <sec> wait up to this long for a review or an inline review
|
|
comment whose commit is the current head
|
|
(default 0 = print what exists and exit)
|
|
--interval <sec> seconds between polls (default 30)
|
|
|
|
exit 0 something against the current head exists (or --wait was 0)
|
|
exit 1 --wait elapsed with nothing against the current head
|
|
exit 4 usage or environment error
|
|
|
|
THIS SCRIPT'S `$?` MUST BE THE ONE YOU READ. Nothing may follow it in the same
|
|
pipeline OR the same `;` list: `pr-reviews 1 --wait 60 | tail` gives you tail's
|
|
status, and so does `pr-reviews 1 --wait 60; echo "exit=$?"` -- the echo runs
|
|
first and $? is then the echo's. Either way a --wait that TIMED OUT reads as
|
|
success. That trap has cost this repo five separate diagnoses, the most recent
|
|
an agent that nearly reported a review which did not exist. Redirect to a file,
|
|
then read the file in a later command.
|
|
|
|
Three surfaces, because a reviewer writes on whichever one is nearest:
|
|
[REVIEW] the review's own body (pulls/{n}/reviews)
|
|
[INLINE] a comment on a line of the diff (pulls/{n}/comments)
|
|
[COMMENT] a comment on the PR thread (issues/{n}/comments)
|
|
They are interleaved in time order. INLINE is the one a human is most likely to
|
|
use and the one this script was blind to until improvise-xgra: PR #122 was
|
|
reviewed with the body "Generally good, added comments inline." and five
|
|
substantive rulings underneath it, of which this script printed none.
|
|
|
|
A review or comment may target an EARLIER head, which is why the head is
|
|
compared rather than the count: a finding against an old commit may be fixed or
|
|
still live, and only reading it tells you which. An issue-level [COMMENT] is
|
|
attached to no commit at all, so it can never satisfy --wait.
|
|
EOF
|
|
exit 4
|
|
}
|
|
|
|
PR=""; WAIT=0; INTERVAL=30
|
|
while [ $# -gt 0 ]; do
|
|
case "$1" in
|
|
--wait) WAIT="${2:-}"; shift 2 ;;
|
|
--interval) INTERVAL="${2:-}"; shift 2 ;;
|
|
-h|--help) usage ;;
|
|
*) [ -z "$PR" ] || usage; PR="$1"; shift ;;
|
|
esac
|
|
done
|
|
[ -n "$PR" ] || usage
|
|
|
|
# `gh` is shell-aliased to a different tool in this repo's shells, so the name
|
|
# is never trusted: a real gh is located explicitly.
|
|
gh_() {
|
|
if command -v gh >/dev/null 2>&1 && gh --version 2>/dev/null | grep -q '^gh version'; then
|
|
gh "$@"
|
|
else
|
|
nix shell nixpkgs#gh --command gh "$@"
|
|
fi
|
|
}
|
|
|
|
# One poll reads all three surfaces, so the counter is bumped once per poll and
|
|
# not once per file. It lives on disk because fetch is called as "$(fetch)" and
|
|
# a command substitution is a SUBSHELL -- a variable incremented in it is lost,
|
|
# which is how pr-watch's fixtures could never reach their second poll.
|
|
fixture_poll() {
|
|
local c="$PR_REVIEWS_FIXTURE_DIR/.poll"
|
|
local n=$(( $(cat "$c" 2>/dev/null || echo 0) + 1 ))
|
|
echo "$n" > "$c"
|
|
echo "$n"
|
|
}
|
|
|
|
# Each `local` gets its own statement. Bash 5 creates every name in a
|
|
# multi-assignment `local` before running the assignments, so a later one
|
|
# reading an earlier one is unbound under `set -u` -- see context/code/pr-review.md.
|
|
fixture_file() {
|
|
local n="$1"
|
|
local name="$2"
|
|
local f="$PR_REVIEWS_FIXTURE_DIR/$n.$name.json"
|
|
[ -f "$f" ] || f="$PR_REVIEWS_FIXTURE_DIR/last.$name.json"
|
|
[ -f "$f" ] && cat "$f" || echo 'null'
|
|
}
|
|
|
|
# `gh api --paginate` emits one JSON array PER PAGE, so a comment thread past
|
|
# the page size arrives as several concatenated documents that --argjson
|
|
# rejects. Both paths flatten through here, so a fixture can carry two pages.
|
|
pages_to_array() { jq -s 'add // []'; }
|
|
|
|
fetch() {
|
|
if [ -n "${PR_REVIEWS_FIXTURE_DIR:-}" ]; then
|
|
local n; n=$(fixture_poll)
|
|
local pr; pr=$(fixture_file "$n" pr)
|
|
local inline; inline=$(fixture_file "$n" inline | pages_to_array)
|
|
local issue; issue=$(fixture_file "$n" issue | pages_to_array)
|
|
jq -n --argjson pr "$pr" --argjson inline "$inline" --argjson issue "$issue" \
|
|
'{head: ($pr.headRefOid // ""), reviews: ($pr.reviews // []),
|
|
inline: $inline, issue: $issue}'
|
|
return
|
|
fi
|
|
local repo; repo=$(gh_ repo view --json nameWithOwner -q .nameWithOwner)
|
|
local pr; pr=$(gh_ pr view "$PR" --json headRefOid,reviews)
|
|
local inline; inline=$(gh_ api "repos/$repo/pulls/$PR/comments" --paginate | pages_to_array)
|
|
local issue; issue=$(gh_ api "repos/$repo/issues/$PR/comments" --paginate | pages_to_array)
|
|
jq -n --argjson pr "$pr" --argjson inline "$inline" --argjson issue "$issue" \
|
|
'{head: ($pr.headRefOid // ""), reviews: ($pr.reviews // []),
|
|
inline: $inline, issue: $issue}'
|
|
}
|
|
|
|
# `line` is null once the line has moved under the comment; `original_line` is
|
|
# where it was written, and is all an outdated comment has left.
|
|
normalize='
|
|
[ (.reviews[]? | {at: .submittedAt, kind: "REVIEW", state: (.state // ""),
|
|
who: (.author.login // "?"), commit: (.commit.oid // ""),
|
|
at_line: "", body: (.body // "")}),
|
|
(.inline[]? | {at: .created_at, kind: "INLINE", state: "",
|
|
who: (.user.login // "?"), commit: (.commit_id // ""),
|
|
at_line: "\(.path // "?"):\(.line // .original_line // "?")",
|
|
body: (.body // "")}),
|
|
(.issue[]? | {at: .created_at, kind: "COMMENT", state: "",
|
|
who: (.user.login // "?"), commit: "",
|
|
at_line: "", body: (.body // "")}) ]
|
|
| sort_by(.at)'
|
|
|
|
report() {
|
|
local json="$1" head
|
|
head=$(printf '%s' "$json" | jq -r '.head')
|
|
printf 'pr-reviews: #%s head=%s\n' "$PR" "${head:0:8}"
|
|
printf '%s' "$json" | jq -r --arg head "$head" "$normalize"'
|
|
| if length == 0 then " (nothing written on any of the three surfaces)"
|
|
else .[] |
|
|
" [\(.kind)\(if .state == "" then "" else " " + .state end)] \(.at) @\(.who)" +
|
|
(if .at_line == "" then "" else " " + .at_line end) +
|
|
(if .commit == "" then " <- not attached to a commit"
|
|
elif .commit == $head then " commit=\(.commit[0:8]) <- CURRENT HEAD"
|
|
else " commit=\(.commit[0:8]) <- an EARLIER head; verify whether it still applies" end) +
|
|
"\n" + (.body | split("\n") | map(" " + .) | join("\n"))
|
|
end'
|
|
}
|
|
|
|
# An inline comment at the head counts, and that is the whole point of the fix:
|
|
# the case that failed was a reviewer whose substance was entirely inline. An
|
|
# issue-level comment carries no commit and so cannot count -- it makes no claim
|
|
# about which tree was read.
|
|
has_head_review() {
|
|
printf '%s' "$1" | jq -e --arg h "$(printf '%s' "$1" | jq -r .head)" \
|
|
'any((.reviews[]?.commit.oid), (.inline[]?.commit_id); . == $h)' >/dev/null 2>&1
|
|
}
|
|
|
|
json=$(fetch)
|
|
report "$json"
|
|
|
|
[ "$WAIT" -gt 0 ] || exit 0
|
|
|
|
if has_head_review "$json"; then exit 0; fi
|
|
|
|
deadline=$(( $(date +%s) + WAIT ))
|
|
while [ "$(date +%s)" -lt "$deadline" ]; do
|
|
sleep "$INTERVAL"
|
|
json=$(fetch)
|
|
if has_head_review "$json"; then
|
|
printf '\npr-reviews: a review or inline comment against the current head arrived.\n\n'
|
|
report "$json"
|
|
exit 0
|
|
fi
|
|
done
|
|
|
|
printf '\npr-reviews: waited %ss; nothing against the current head.\n' "$WAIT"
|
|
printf 'Reviews arrive as the owner quota allows, so absence is not approval.\n'
|
|
exit 1
|