improvise/scripts/pr-reviews

#!/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; any --wait
                    above 0 polls at least once more after the first read
                    (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 combine
# would misnumber. Both paths flatten through here, so a fixture can carry two pages.
pages_to_array() { jq -s 'add // []'; }

# The three documents reach jq on STDIN, never as --argjson: a thread past
# ARG_MAX is exit 126 before jq runs (improvise-5jxt). printf is a builtin.
combine() {
  printf '%s\n%s\n%s\n' "$1" "$2" "$3" | jq -s \
    '{head: (.[0].headRefOid // ""), reviews: (.[0].reviews // []),
      inline: .[1], issue: .[2]}'
}

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)
    combine "$pr" "$inline" "$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)
  combine "$pr" "$inline" "$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

# The deadline is checked AFTER each poll: checked first, a second ticking over
# between two reads of the clock skipped the only re-poll (improvise-qnpi).
deadline=$(( $(date +%s) + WAIT ))
while :; 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
  [ "$(date +%s)" -lt "$deadline" ] || break
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