diff --git a/context/code/pr-review.md b/context/code/pr-review.md new file mode 100644 index 00000000..d91e2180 --- /dev/null +++ b/context/code/pr-review.md @@ -0,0 +1,39 @@ +# Reading a PR's reviews + +Twenty PRs merged in one campaign with their reviews unread. CI was green and +an adversarial agent had been dispatched, which felt like the whole gate. Two +ChatGPT Codex reviews were found afterwards on merged PRs; both were correct +and one was a live P2. Three more were found on the next three PRs — one per +PR, every one a P1. + +`scripts/pr-reviews ` prints them. `--wait ` blocks until one lands +against the current head. + +## Why a review is easy to miss here + +It arrives as a **review**, not a comment, so reading the comment thread does +not show it. + +It posts under the repository **owner's** account — the same account this +repo's own agents post under — so **authorship does not distinguish it**. The +tell is placement: a Codex review *ends* with +`_Automated review by ChatGPT Codex (…)_`, where an agent's comment *opens* +with `_Posted by Claude (…)_`. + +It may target an **earlier head**. `pr-reviews` says which, because that +changes what you do: a finding against an old commit may already be fixed or +may still be live, and only checking tells you which. Both cases have occurred +— one fixed by a later round, one live at the head after three more commits. + +## Absence is not approval + +Reviews arrive as the owner's quota allows. Observed latency is ten to fifteen +minutes after a push. `--wait` exists so that a merge four minutes after a +push does not race the review that was about to arrive; when it times out, +that is a decision to merge unreviewed rather than evidence of a clean bill. + +## Answer them on the thread + +Including after the PR has merged. That is where the next reader looks, and a +finding that was declined or already-fixed is worth as much as one that was +taken — the reasoning is the artefact, not the verdict. diff --git a/scripts/pr-reviews b/scripts/pr-reviews new file mode 100755 index 00000000..7a866c81 --- /dev/null +++ b/scripts/pr-reviews @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# Prints a PR's reviews, and optionally waits for one against the current head. +# Exists because twenty PRs merged with their reviews unread; see +# context/code/pr-review.md. +set -euo pipefail + +usage() { + cat >&2 <<'EOF' +usage: pr-reviews [--wait ] [--interval ] + + --wait wait up to this long for a review whose commit is the + current head (default 0 = print what exists and exit) + --interval seconds between polls (default 30) + +exit 0 a review against the current head exists (or --wait was 0) +exit 1 --wait elapsed with no review against the current head +exit 4 usage or environment error + +A review 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. +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 +} + +fetch() { gh_ pr view "$PR" --json headRefOid,reviews; } + +report() { + local json="$1" head + head=$(printf '%s' "$json" | jq -r '.headRefOid') + printf 'pr-reviews: #%s head=%s\n' "$PR" "${head:0:8}" + printf '%s' "$json" | jq -r --arg head "$head" ' + if (.reviews | length) == 0 then " (no reviews)" + else .reviews[] | + " [\(.state)] \(.submittedAt) commit=\(.commit.oid[0:8])" + + (if .commit.oid == $head then " <- CURRENT HEAD" else " <- an EARLIER head; verify whether it still applies" end) + + "\n" + (.body | split("\n") | map(" " + .) | join("\n")) + end' +} + +has_head_review() { + printf '%s' "$1" | jq -e --arg h "$(printf '%s' "$1" | jq -r .headRefOid)" \ + 'any(.reviews[]?; .commit.oid == $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 against the current head arrived.\n\n' + report "$json" + exit 0 + fi +done + +printf '\npr-reviews: waited %ss; no review against the current head.\n' "$WAIT" +printf 'Reviews arrive as the owner quota allows, so absence is not approval.\n' +exit 1