Files
improvise/scripts/pr-watch
T
Cursor Agent ec30c8a7de fix: address three Codex P2 review findings
1. Shebang (Codex P2): scripts/agent-bootstrap.sh and .claude/hooks/session-start.sh
   now use #!/usr/bin/env bash instead of #!/bin/bash, matching repo convention
   enforced by checkers.sh.

2. Fail-closed bd pin (Codex P2): .claude/hooks/session-start.sh local path now
   exits 2 with a warning when pinned or running version cannot be parsed,
   instead of silently succeeding. An unparseable version is not a pass.

3. pr-reviews failure (Codex P2): scripts/pr-watch no longer uses '|| true' to
   swallow pr-reviews failures. If pr-reviews fails (fetch error, malformed
   response), pr-watch now exits 6 with a warning. Empty successful fetch
   (no reviews) is still exit 0.

   - Added exit 6 to usage text and skill documentation
   - Added test case 'pr-reviews failure exits 6' using malformed JSON fixture
   - All 57 pr-watch tests pass

Co-authored-by: Ed Langley <fiddlerwoaroof@users.noreply.github.com>
2026-09-26 01:30:00 +00:00

171 lines
7.1 KiB
Bash
Executable File

#!/usr/bin/env bash
# Waits for a PR's checks to finish and says what happened. Every rule in here
# is one this repo got wrong first; see context/code/tooling.md section 3.
set -euo pipefail
usage() {
cat >&2 <<'EOF'
usage: pr-watch <pr-number> [--expect <sha>] [--timeout <sec>] [--interval <sec>] [--reviews|--no-reviews]
--expect <sha> fail if the PR head is not this (prefix ok) at any poll
--timeout <sec> give up after this long (default 1800)
--interval <sec> seconds between polls (default 20)
--reviews after checks complete, print reviews/comments (default)
--no-reviews skip the reviews/comments printout
exit 0 every check COMPLETED, none failed, and GitHub says MERGEABLE with a
state of CLEAN, UNSTABLE or HAS_HOOKS (and reviews fetched if --reviews)
exit 1 a check failed, or the PR is not mergeable (any other known state)
exit 2 the head moved during the wait, or is not --expect
exit 3 timed out with checks still running, or mergeability still UNKNOWN,
or still BLOCKED with every registered check green
exit 4 usage or environment error
exit 5 the PR is MERGED or CLOSED, so there is nothing left to wait for
exit 6 checks green/mergeable, but pr-reviews failed (fetch error, not "no reviews")
After exit 0, reviews and comments are printed (unless --no-reviews). Reviews are
a separate API surface from comments; Codex/ChatGPT reviews arrive as *reviews*
and are easy to miss if you only read the comment thread.
EOF
exit 4
}
PR=""; EXPECT=""; TIMEOUT=1800; INTERVAL=20; SHOW_REVIEWS=1
while [ $# -gt 0 ]; do
case "$1" in
--expect) EXPECT="${2:-}"; shift 2 ;;
--timeout) TIMEOUT="${2:-}"; shift 2 ;;
--interval) INTERVAL="${2:-}"; shift 2 ;;
--reviews) SHOW_REVIEWS=1; shift ;;
--no-reviews) SHOW_REVIEWS=0; shift ;;
-h|--help) usage ;;
*) [ -z "$PR" ] || usage; PR="$1"; shift ;;
esac
done
[ -n "$PR" ] || usage
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# `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.
fetch() {
if [ -n "${PR_WATCH_FIXTURE_DIR:-}" ]; then
# The counter lives on disk because fetch is called as "$(fetch)", and a
# command substitution is a SUBSHELL -- a variable incremented in it is
# lost, so every poll re-read the first fixture and no test could ever
# reach the second one.
local c="$PR_WATCH_FIXTURE_DIR/.poll"
local n=$(( $(cat "$c" 2>/dev/null || echo 0) + 1 ))
echo "$n" > "$c"
local f="$PR_WATCH_FIXTURE_DIR/$n.json"
[ -f "$f" ] || f="$PR_WATCH_FIXTURE_DIR/last.json"
cat "$f"
return
fi
if command -v gh >/dev/null 2>&1 && gh --version 2>/dev/null | grep -q '^gh version'; then
gh pr view "$PR" --json state,headRefOid,mergeable,mergeStateStatus,statusCheckRollup
else
nix shell nixpkgs#gh --command gh pr view "$PR" \
--json state,headRefOid,mergeable,mergeStateStatus,statusCheckRollup
fi
}
jqf() { jq -r "$1"; }
started=$(date +%s)
pinned=""
while :; do
json="$(fetch)" || { echo "pr-watch: cannot read PR $PR" >&2; exit 4; }
head="$(printf '%s' "$json" | jqf '.headRefOid')"
# A verdict about a tree that no longer exists reads as a pass. This is not
# hypothetical here: PR #67 was merged at a head that predated its fix.
if [ -n "$EXPECT" ] && [ "${head#"$EXPECT"}" = "$head" ]; then
echo "pr-watch: head is $head, expected $EXPECT*" >&2
exit 2
fi
if [ -z "$pinned" ]; then
pinned="$head"
elif [ "$head" != "$pinned" ]; then
echo "pr-watch: head moved $pinned -> $head during the wait" >&2
exit 2
fi
# A PR no longer open ends the wait (improvise-1qaa), but only after the head
# checks: a merge at a head never seen is PR #67 again (improvise-24lz).
pr_state="$(printf '%s' "$json" | jqf '.state')"
case "$pr_state" in
MERGED|CLOSED) echo "pr-watch: PR $PR is $pr_state, nothing to wait for" >&2; exit 5 ;;
esac
# total>0 AND nothing still running. "0 pending" alone reads identically for
# "all finished" and "none registered yet", which is how this exited instantly
# on a fresh push. And `conclusion` is a STRING, empty while running, never
# null -- so a null test matches nothing and every check looks finished.
total="$(printf '%s' "$json" | jqf '[.statusCheckRollup[]?] | length')"
running="$(printf '%s' "$json" | jqf '[.statusCheckRollup[]? | select(.status != "COMPLETED")] | length')"
failed="$(printf '%s' "$json" | jqf \
'[.statusCheckRollup[]? | select(.conclusion != "SUCCESS" and .conclusion != "SKIPPED" and .conclusion != "NEUTRAL")] | length')"
mergeable="$(printf '%s' "$json" | jqf '.mergeable')"
state="$(printf '%s' "$json" | jqf '.mergeStateStatus')"
# UNKNOWN and green-but-BLOCKED are waited on: see context/code/tooling.md,
# "pr-watch and mergeability" (improvise-edpl, improvise-zlj2).
if [ "$total" -gt 0 ] && [ "$running" -eq 0 ]; then
if [ "$failed" -gt 0 ] || [ "$mergeable" = CONFLICTING ]; then break; fi
case "$mergeable/$state" in
UNKNOWN/*|*/UNKNOWN|MERGEABLE/BLOCKED) ;;
*) break ;;
esac
fi
now=$(date +%s)
if [ $((now - started)) -ge "$TIMEOUT" ]; then
if [ "$total" -gt 0 ] && [ "$running" -eq 0 ]; then
echo "pr-watch: timed out after ${TIMEOUT}s with mergeability $mergeable/$state" >&2
else
echo "pr-watch: timed out after ${TIMEOUT}s with $running of $total still running" >&2
fi
exit 3
fi
sleep "$INTERVAL"
done
printf '%s' "$json" | jqf '.statusCheckRollup[]? | select(.conclusion != "SKIPPED") | " \(.conclusion) \(.name)"'
echo "pr-watch: head=$head $mergeable/$state $total checks, $failed failed"
[ "$failed" -eq 0 ] || exit 1
# An allowlist, so a state GitHub adds later fails closed. DIRTY and BLOCKED
# can wear a MERGEABLE flag: context/code/tooling.md, "pr-watch and mergeability".
[ "$mergeable" = MERGEABLE ] || exit 1
case "$state" in CLEAN|UNSTABLE|HAS_HOOKS) ;; *) exit 1 ;; esac
# ── Reviews and comments ───────────────────────────────────────────────────
# Checks are green and mergeable. Show reviews/comments so the agent sees any
# feedback that needs answering before merge. This does not change the exit
# code for success -- answering reviews is your responsibility. However, if
# pr-reviews itself fails (usage error, network, etc.), we warn and exit 6.
if [ "$SHOW_REVIEWS" -eq 1 ]; then
echo ""
echo "pr-watch: fetching reviews and comments (--no-reviews to skip)…"
reviews_exit=0
if [ -n "${PR_WATCH_FIXTURE_DIR:-}" ]; then
PR_REVIEWS_FIXTURE_DIR="$PR_WATCH_FIXTURE_DIR" "$SCRIPT_DIR/pr-reviews" "$PR" || reviews_exit=$?
else
"$SCRIPT_DIR/pr-reviews" "$PR" || reviews_exit=$?
fi
echo ""
if [ "$reviews_exit" -eq 0 ]; then
echo "pr-watch: reviews shown above. Answer substantive feedback before merging."
else
echo "pr-watch: WARNING: pr-reviews failed (exit $reviews_exit). Reviews may not be shown." >&2
echo "pr-watch: Run './scripts/pr-reviews $PR' manually to fetch reviews." >&2
exit 6
fi
fi
exit 0