Skip to content

Commit 4ee3df4

Browse files
CybotTMCybotTM
authored
Merge pull request #180 from netresearch/retro/pr-status-json-contract
feat(pr-status): make the --json contract discoverable and enforced
2 parents d345ca1 + 97d9c56 commit 4ee3df4

2 files changed

Lines changed: 152 additions & 1 deletion

File tree

‎skills/git-workflow/scripts/pr-status.sh‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,35 @@
2424
# soon as something can be worked on: a failing check, a new annotation, or all
2525
# required checks concluded. Waiting for `pending == 0` means learning nothing
2626
# until the slowest matrix job ends, long after the first failure was visible.
27+
#
28+
# --json contract
29+
#
30+
# One object, and its field names are THIS SCRIPT'S — not the GraphQL names
31+
# `gh pr view` uses. `mergeState`, not `mergeStateStatus`. `base`, not
32+
# `baseRefName`. Guessing costs more than reading: jq answers a missing key
33+
# with `null` and says nothing, so a loop waiting for
34+
# `.mergeStateStatus == "CLEAN"` never fires and reads as "still running"
35+
# forever. Top-level keys:
36+
#
37+
# state mergeable mergeState draft number title repo author
38+
# base head headOid
39+
# checks checks_settled threads unresolved_threads
40+
# reviewDecision reviews_on_head has_review_on_head
41+
# has_copilot_review_on_head copilot_review_errored copilot_error_count
42+
# requested_reviewers
43+
# merge_methods auto_merge_allowed queue_active queue_entry
44+
# rulesets rules_fetched required_contexts undispatched unsigned
45+
# next
46+
#
47+
# `checks` is {total,pass,fail,pending,skip,...} and `next` is
48+
# {action,why,cmd} — the same two things the prose rendering leads with.
49+
#
50+
# Before writing a jq filter against any of these, consider whether --watch
51+
# already answers the question; it usually does, and a hand-rolled poll loop is
52+
# how the wrong field name gets guessed in the first place.
53+
#
54+
# tests/test_pr_status_json_contract.sh fails when this list and the emitted
55+
# object drift apart, in either direction.
2756
set -uo pipefail
2857

2958
REPO=""; PR=""; JSON=0; WATCH=0; INTERVAL=20; MAXWAIT=3600
@@ -39,7 +68,10 @@ while [ $# -gt 0 ]; do
3968
--watch) WATCH=1; shift ;;
4069
--interval) need "$@"; INTERVAL="$2"; shift 2 ;;
4170
--max-wait) need "$@"; MAXWAIT="$2"; shift 2 ;;
42-
-h|--help) sed -n '2,30p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;;
71+
# Prints the whole leading comment block rather than a fixed line range:
72+
# a hard-coded range silently truncates --help the moment the header
73+
# grows, which is how a documented contract stops being visible.
74+
-h|--help) awk 'NR>1 && /^#/ {sub(/^# ?/,""); print; next} NR>1 {exit}' "$0"; exit 0 ;;
4375
-*) die "unknown flag: $1" ;;
4476
*) PR="$1"; shift ;;
4577
esac

‎tests/test_pr_status_json_contract.sh‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
#!/usr/bin/env bash
2+
# Regression test: the --json key list in pr-status.sh's header must match what
3+
# --json actually emits.
4+
#
5+
# Why this is a test and not a comment: a caller who guesses a field name gets
6+
# `null` from jq and no warning, so a watcher polling `.mergeStateStatus`
7+
# (the GraphQL name, not this script's) never fires and reads as "still
8+
# running" forever. The header exists to stop that guess — which only works
9+
# while it is true, and a documented contract that nobody checks rots at the
10+
# first new field.
11+
#
12+
# Runs pr-status.sh against a stubbed `gh`, so it needs no network and no repo.
13+
14+
set -euo pipefail
15+
16+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
17+
SCRIPT="$ROOT/skills/git-workflow/scripts/pr-status.sh"
18+
STUB_DIR="$(mktemp -d)"
19+
trap 'rm -rf "$STUB_DIR"' EXIT
20+
21+
fail=0
22+
23+
# --- stub `gh`: one rules payload, one GraphQL payload -----------------------
24+
25+
printf '%s\n' '[{"type":"copilot_code_review","parameters":{}}]' > "$STUB_DIR/rules.json"
26+
27+
cat > "$STUB_DIR/gh" <<STUB
28+
#!/usr/bin/env bash
29+
for a in "\$@"; do
30+
case "\$a" in
31+
repos/*/rules/branches/*) cat "$STUB_DIR/rules.json"; exit 0 ;;
32+
esac
33+
done
34+
cat "$STUB_DIR/graphql.json"
35+
STUB
36+
chmod +x "$STUB_DIR/gh"
37+
38+
python3 - "$STUB_DIR/graphql.json" <<'PY'
39+
import sys, json
40+
head = "deadbeefcafe"
41+
json.dump({"data": {"repository": {
42+
"nameWithOwner": "o/r",
43+
"mergeCommitAllowed": True, "rebaseMergeAllowed": False, "squashMergeAllowed": False,
44+
"pullRequest": {
45+
"number": 1, "title": "t", "state": "OPEN", "isDraft": False,
46+
"mergeable": "MERGEABLE", "mergeStateStatus": "CLEAN", "reviewDecision": None,
47+
"author": {"login": "someone"},
48+
"baseRefName": "main", "headRefName": "f", "headRefOid": head,
49+
"isCrossRepository": False,
50+
"reviews": {"nodes": []},
51+
"reviewRequests": {"nodes": []},
52+
"reviewThreads": {"nodes": []},
53+
"commits": {"nodes": [{"commit": {"oid": head, "statusCheckRollup": {
54+
"state": "SUCCESS", "contexts": {"nodes": [
55+
{"__typename": "CheckRun", "name": "CI", "conclusion": "SUCCESS",
56+
"status": "COMPLETED", "detailsUrl": "u",
57+
"startedAt": "2026-01-01T00:00:00Z"}]}}}}]},
58+
"allCommits": {"nodes": [{"commit": {"oid": head,
59+
"signature": {"isValid": True}}}]},
60+
}}}}, open(sys.argv[1], "w"))
61+
PY
62+
63+
# --- the two lists -----------------------------------------------------------
64+
65+
emitted="$(PATH="$STUB_DIR:$PATH" bash "$SCRIPT" -R o/r 1 --json | jq -r 'keys[]' | sort)"
66+
67+
# The documented block: the indented run of key names between the "--json
68+
# contract" heading and the first line that is not part of it. Matching on
69+
# indentation rather than on a marker keeps the header readable as prose.
70+
documented="$(
71+
awk '
72+
/^# --json contract$/ { inblock = 1; next }
73+
!inblock { next }
74+
/^# [a-z]/ { sub(/^# /, ""); print; next }
75+
/^# `checks` is/ { exit }
76+
' "$SCRIPT" | tr " " "\n" | sed "/^$/d" | sort
77+
)"
78+
79+
# --- compare both directions -------------------------------------------------
80+
81+
if [ -z "$emitted" ]; then
82+
echo " FAIL the stubbed run emitted no JSON keys at all"
83+
exit 1
84+
fi
85+
if [ -z "$documented" ]; then
86+
echo " FAIL no key list found under the '--json contract' header"
87+
exit 1
88+
fi
89+
90+
undocumented="$(comm -23 <(printf '%s\n' "$emitted") <(printf '%s\n' "$documented"))"
91+
stale="$(comm -13 <(printf '%s\n' "$emitted") <(printf '%s\n' "$documented"))"
92+
93+
if [ -n "$undocumented" ]; then
94+
echo " FAIL --json emits keys the header does not list:"
95+
printf ' %s\n' "$undocumented"
96+
echo " Add them to the '--json contract' block in pr-status.sh."
97+
fail=1
98+
else
99+
echo " ok every emitted key is documented"
100+
fi
101+
102+
if [ -n "$stale" ]; then
103+
echo " FAIL the header lists keys --json does not emit:"
104+
printf ' %s\n' "$stale"
105+
echo " Remove them from the '--json contract' block in pr-status.sh."
106+
fail=1
107+
else
108+
echo " ok every documented key is emitted"
109+
fi
110+
111+
# --help must show the block; a fixed line range used to truncate it.
112+
if bash "$SCRIPT" --help | grep -q '^--json contract$'; then
113+
echo " ok --help shows the contract"
114+
else
115+
echo " FAIL --help does not show the '--json contract' block"
116+
fail=1
117+
fi
118+
119+
exit $fail

0 commit comments

Comments
 (0)