scripts: pr-reviews, because twenty PRs merged with their reviews unread
Prints a PR's reviews and says whether each targets the CURRENT head or an earlier one, because that changes what you do: a finding against an old commit may be fixed or still live, and both have happened this week. --wait blocks until a review lands against the head. Observed latency is ten to fifteen minutes after a push, so a merge four minutes later races the review that was about to arrive. When it times out that is a decision to merge unreviewed, not a clean bill. The detection rule is placement, not authorship: Codex posts under the owner's account, the same one our agents use, and ENDS with its attribution where ours OPEN with theirs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -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 <n>` prints them. `--wait <sec>` 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.
|
||||
Executable
+87
@@ -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 <pr-number> [--wait <sec>] [--interval <sec>]
|
||||
|
||||
--wait <sec> wait up to this long for a review whose commit is the
|
||||
current head (default 0 = print what exists and exit)
|
||||
--interval <sec> 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
|
||||
Reference in New Issue
Block a user