Skip to content

Commit 4641be5

Browse files
ThomasK33ThomasK33
committed
🤖 fix: address Codex feedback on PR review tooling
- paginate Codex comment + review thread queries in check_codex_comments.sh - paginate review threads in check_pr_reviews.sh - treat numeric input as PR when it resolves in extract_pr_logs.sh Validation: - bash -n scripts/*.sh - make verify-vendor - make test - make build --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.82`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=0.82 -->
1 parent 5ae8f52 commit 4641be5

3 files changed

Lines changed: 171 additions & 44 deletions

File tree

‎scripts/check_codex_comments.sh‎

Lines changed: 119 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,31 @@ fi
99
PR_NUMBER=$1
1010
BOT_LOGIN_GRAPHQL="chatgpt-codex-connector"
1111

12+
if ! [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]; then
13+
echo "❌ PR number must be numeric. Got: '$PR_NUMBER'"
14+
exit 1
15+
fi
16+
1217
echo "Checking for unresolved Codex comments in PR #${PR_NUMBER}..."
1318

14-
# Use GraphQL to get all comments (including minimized status)
15-
# shellcheck disable=SC2016 # Single quotes are intentional - this is a GraphQL query, not shell expansion
16-
GRAPHQL_QUERY='query($owner: String!, $repo: String!, $pr: Int!) {
19+
REPO_INFO=$(gh repo view --json owner,name --jq '{owner: .owner.login, name: .name}')
20+
OWNER=$(echo "$REPO_INFO" | jq -r '.owner')
21+
REPO=$(echo "$REPO_INFO" | jq -r '.name')
22+
23+
# Depot runners sometimes hit transient network timeouts to api.github.com.
24+
# Retry the GraphQL request a few times before failing the required check.
25+
MAX_ATTEMPTS=5
26+
BACKOFF_SECS=2
27+
28+
# shellcheck disable=SC2016 # Single quotes are intentional - these are GraphQL queries.
29+
COMMENTS_QUERY='query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) {
1730
repository(owner: $owner, name: $repo) {
1831
pullRequest(number: $pr) {
19-
comments(first: 100) {
32+
comments(first: 100, after: $cursor) {
33+
pageInfo {
34+
hasNextPage
35+
endCursor
36+
}
2037
nodes {
2138
id
2239
author { login }
@@ -25,7 +42,19 @@ GRAPHQL_QUERY='query($owner: String!, $repo: String!, $pr: Int!) {
2542
isMinimized
2643
}
2744
}
28-
reviewThreads(first: 100) {
45+
}
46+
}
47+
}'
48+
49+
# shellcheck disable=SC2016 # Single quotes are intentional - these are GraphQL queries.
50+
THREADS_QUERY='query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) {
51+
repository(owner: $owner, name: $repo) {
52+
pullRequest(number: $pr) {
53+
reviewThreads(first: 100, after: $cursor) {
54+
pageInfo {
55+
hasNextPage
56+
endCursor
57+
}
2958
nodes {
3059
id
3160
isResolved
@@ -45,50 +74,111 @@ GRAPHQL_QUERY='query($owner: String!, $repo: String!, $pr: Int!) {
4574
}
4675
}'
4776

48-
REPO_INFO=$(gh repo view --json owner,name --jq '{owner: .owner.login, name: .name}')
49-
OWNER=$(echo "$REPO_INFO" | jq -r '.owner')
50-
REPO=$(echo "$REPO_INFO" | jq -r '.name')
77+
fetch_graphql_with_retry() {
78+
local query="$1"
79+
shift
80+
81+
local attempt
82+
local backoff
83+
backoff="$BACKOFF_SECS"
84+
85+
for ((attempt = 1; attempt <= MAX_ATTEMPTS; attempt++)); do
86+
if gh api graphql \
87+
-f query="$query" \
88+
-F owner="$OWNER" \
89+
-F repo="$REPO" \
90+
-F pr="$PR_NUMBER" \
91+
"$@"; then
92+
return 0
93+
fi
94+
95+
if [ "$attempt" -eq "$MAX_ATTEMPTS" ]; then
96+
echo "❌ GraphQL query failed after ${MAX_ATTEMPTS} attempts"
97+
return 1
98+
fi
99+
100+
echo "⚠️ GraphQL query failed (attempt ${attempt}/${MAX_ATTEMPTS}); retrying in ${backoff}s..."
101+
sleep "$backoff"
102+
backoff=$((backoff * 2))
103+
done
104+
}
105+
106+
COMMENTS_CURSOR=""
107+
ALL_COMMENTS='[]'
108+
109+
while true; do
110+
if [ -n "$COMMENTS_CURSOR" ]; then
111+
COMMENTS_RESULT=$(fetch_graphql_with_retry "$COMMENTS_QUERY" -F cursor="$COMMENTS_CURSOR")
112+
else
113+
COMMENTS_RESULT=$(fetch_graphql_with_retry "$COMMENTS_QUERY")
114+
fi
51115

52-
# Depot runners sometimes hit transient network timeouts to api.github.com.
53-
# Retry the GraphQL request a few times before failing the required check.
54-
MAX_ATTEMPTS=5
55-
BACKOFF_SECS=2
116+
if [ "$(echo "$COMMENTS_RESULT" | jq -r '.data.repository.pullRequest == null')" = "true" ]; then
117+
echo "❌ PR #${PR_NUMBER} does not exist in ${OWNER}/${REPO}."
118+
exit 1
119+
fi
120+
121+
PAGE_COMMENTS=$(echo "$COMMENTS_RESULT" | jq '.data.repository.pullRequest.comments.nodes')
122+
ALL_COMMENTS=$(jq -cn --argjson all "$ALL_COMMENTS" --argjson page "$PAGE_COMMENTS" '$all + $page')
56123

57-
for ((attempt = 1; attempt <= MAX_ATTEMPTS; attempt++)); do
58-
if RESULT=$(gh api graphql \
59-
-f query="$GRAPHQL_QUERY" \
60-
-F owner="$OWNER" \
61-
-F repo="$REPO" \
62-
-F pr="$PR_NUMBER"); then
124+
HAS_NEXT=$(echo "$COMMENTS_RESULT" | jq -r '.data.repository.pullRequest.comments.pageInfo.hasNextPage')
125+
if [ "$HAS_NEXT" != "true" ]; then
63126
break
64127
fi
65128

66-
if [ "$attempt" -eq "$MAX_ATTEMPTS" ]; then
67-
echo "❌ GraphQL query failed after ${MAX_ATTEMPTS} attempts"
129+
COMMENTS_CURSOR=$(echo "$COMMENTS_RESULT" | jq -r '.data.repository.pullRequest.comments.pageInfo.endCursor')
130+
if [ -z "$COMMENTS_CURSOR" ] || [ "$COMMENTS_CURSOR" = "null" ]; then
131+
echo "❌ Assertion failed: comments pagination cursor missing while hasNextPage=true"
132+
exit 1
133+
fi
134+
done
135+
136+
THREADS_CURSOR=""
137+
ALL_THREADS='[]'
138+
139+
while true; do
140+
if [ -n "$THREADS_CURSOR" ]; then
141+
THREADS_RESULT=$(fetch_graphql_with_retry "$THREADS_QUERY" -F cursor="$THREADS_CURSOR")
142+
else
143+
THREADS_RESULT=$(fetch_graphql_with_retry "$THREADS_QUERY")
144+
fi
145+
146+
if [ "$(echo "$THREADS_RESULT" | jq -r '.data.repository.pullRequest == null')" = "true" ]; then
147+
echo "❌ PR #${PR_NUMBER} does not exist in ${OWNER}/${REPO}."
68148
exit 1
69149
fi
70150

71-
echo "⚠️ GraphQL query failed (attempt ${attempt}/${MAX_ATTEMPTS}); retrying in ${BACKOFF_SECS}s..."
72-
sleep "$BACKOFF_SECS"
73-
BACKOFF_SECS=$((BACKOFF_SECS * 2))
151+
PAGE_THREADS=$(echo "$THREADS_RESULT" | jq '.data.repository.pullRequest.reviewThreads.nodes')
152+
ALL_THREADS=$(jq -cn --argjson all "$ALL_THREADS" --argjson page "$PAGE_THREADS" '$all + $page')
153+
154+
HAS_NEXT=$(echo "$THREADS_RESULT" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage')
155+
if [ "$HAS_NEXT" != "true" ]; then
156+
break
157+
fi
158+
159+
THREADS_CURSOR=$(echo "$THREADS_RESULT" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.endCursor')
160+
if [ -z "$THREADS_CURSOR" ] || [ "$THREADS_CURSOR" = "null" ]; then
161+
echo "❌ Assertion failed: review thread pagination cursor missing while hasNextPage=true"
162+
exit 1
163+
fi
74164
done
75165

76166
# Filter regular comments from bot that aren't minimized, excluding:
77167
# - "Didn't find any major issues" (no issues found)
78168
# - "usage limits have been reached" (rate limit error, not a real review)
79-
REGULAR_COMMENTS=$(echo "$RESULT" | jq "[.data.repository.pullRequest.comments.nodes[] | select(.author.login == \"${BOT_LOGIN_GRAPHQL}\" and .isMinimized == false and (.body | test(\"Didn't find any major issues|usage limits have been reached\") | not))]")
169+
REGULAR_COMMENTS=$(echo "$ALL_COMMENTS" | jq "[.[] | select(.author.login == \"${BOT_LOGIN_GRAPHQL}\" and .isMinimized == false and (.body | test(\"Didn't find any major issues|usage limits have been reached\") | not))]")
80170
REGULAR_COUNT=$(echo "$REGULAR_COMMENTS" | jq 'length')
81171

82172
# Filter unresolved review threads from bot
83-
UNRESOLVED_THREADS=$(echo "$RESULT" | jq "[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false and .comments.nodes[0].author.login == \"${BOT_LOGIN_GRAPHQL}\")]")
173+
UNRESOLVED_THREADS=$(echo "$ALL_THREADS" | jq "[.[] | select(.isResolved == false and .comments.nodes[0].author.login == \"${BOT_LOGIN_GRAPHQL}\")]")
84174
UNRESOLVED_COUNT=$(echo "$UNRESOLVED_THREADS" | jq 'length')
85175

86176
TOTAL_UNRESOLVED=$((REGULAR_COUNT + UNRESOLVED_COUNT))
87177

88178
echo "Found ${REGULAR_COUNT} unminimized regular comment(s) from bot"
89179
echo "Found ${UNRESOLVED_COUNT} unresolved review thread(s) from bot"
90180

91-
if [ $TOTAL_UNRESOLVED -gt 0 ]; then
181+
if [ "$TOTAL_UNRESOLVED" -gt 0 ]; then
92182
echo ""
93183
echo "❌ Found ${TOTAL_UNRESOLVED} unresolved comment(s) from Codex in PR #${PR_NUMBER}"
94184
echo ""
@@ -107,7 +197,7 @@ if [ $TOTAL_UNRESOLVED -gt 0 ]; then
107197
echo ""
108198
echo "Please address or resolve all Codex comments before merging."
109199
exit 1
110-
else
111-
echo "✅ No unresolved Codex comments found"
112-
exit 0
113200
fi
201+
202+
echo "✅ No unresolved Codex comments found"
203+
exit 0

‎scripts/check_pr_reviews.sh‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,24 @@ if [ $# -eq 0 ]; then
1111
fi
1212

1313
PR_NUMBER="$1"
14+
if ! [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]; then
15+
echo "❌ PR number must be numeric. Got: '$PR_NUMBER'"
16+
exit 1
17+
fi
1418

1519
REPO_INFO=$(gh repo view --json owner,name --jq '{owner: .owner.login, name: .name}')
1620
OWNER=$(echo "$REPO_INFO" | jq -r '.owner')
1721
REPO=$(echo "$REPO_INFO" | jq -r '.name')
1822

19-
# shellcheck disable=SC2016 # Single quotes are intentional - this is a GraphQL query, not shell expansion
20-
GRAPHQL_QUERY='query($owner: String!, $repo: String!, $pr: Int!) {
23+
# shellcheck disable=SC2016 # Single quotes are intentional - this is a GraphQL query.
24+
GRAPHQL_QUERY='query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) {
2125
repository(owner: $owner, name: $repo) {
2226
pullRequest(number: $pr) {
23-
reviewThreads(first: 100) {
27+
reviewThreads(first: 100, after: $cursor) {
28+
pageInfo {
29+
hasNextPage
30+
endCursor
31+
}
2432
nodes {
2533
id
2634
isResolved
@@ -38,18 +46,46 @@ GRAPHQL_QUERY='query($owner: String!, $repo: String!, $pr: Int!) {
3846
}
3947
}'
4048

41-
RESULT=$(gh api graphql \
42-
-f query="$GRAPHQL_QUERY" \
43-
-F owner="$OWNER" \
44-
-F repo="$REPO" \
45-
-F pr="$PR_NUMBER")
49+
THREAD_CURSOR=""
50+
ALL_THREADS='[]'
4651

47-
if [ "$(echo "$RESULT" | jq -r '.data.repository.pullRequest == null')" = "true" ]; then
48-
echo "❌ PR #$PR_NUMBER was not found in ${OWNER}/${REPO}."
49-
exit 1
50-
fi
52+
while true; do
53+
if [ -n "$THREAD_CURSOR" ]; then
54+
RESULT=$(gh api graphql \
55+
-f query="$GRAPHQL_QUERY" \
56+
-F owner="$OWNER" \
57+
-F repo="$REPO" \
58+
-F pr="$PR_NUMBER" \
59+
-F cursor="$THREAD_CURSOR")
60+
else
61+
RESULT=$(gh api graphql \
62+
-f query="$GRAPHQL_QUERY" \
63+
-F owner="$OWNER" \
64+
-F repo="$REPO" \
65+
-F pr="$PR_NUMBER")
66+
fi
67+
68+
if [ "$(echo "$RESULT" | jq -r '.data.repository.pullRequest == null')" = "true" ]; then
69+
echo "❌ PR #$PR_NUMBER was not found in ${OWNER}/${REPO}."
70+
exit 1
71+
fi
72+
73+
PAGE_THREADS=$(echo "$RESULT" | jq '.data.repository.pullRequest.reviewThreads.nodes')
74+
ALL_THREADS=$(jq -cn --argjson all "$ALL_THREADS" --argjson page "$PAGE_THREADS" '$all + $page')
75+
76+
HAS_NEXT=$(echo "$RESULT" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage')
77+
if [ "$HAS_NEXT" != "true" ]; then
78+
break
79+
fi
80+
81+
THREAD_CURSOR=$(echo "$RESULT" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.endCursor')
82+
if [ -z "$THREAD_CURSOR" ] || [ "$THREAD_CURSOR" = "null" ]; then
83+
echo "❌ Assertion failed: review thread cursor missing while hasNextPage=true"
84+
exit 1
85+
fi
86+
done
5187

52-
UNRESOLVED=$(echo "$RESULT" | jq -c '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {thread_id: .id, user: (.comments.nodes[0].author.login // "unknown"), body: (.comments.nodes[0].body // ""), diff_hunk: (.comments.nodes[0].diffHunk // ""), commit_id: (.comments.nodes[0].commit.oid // "")}')
88+
UNRESOLVED=$(echo "$ALL_THREADS" | jq -c '.[] | select(.isResolved == false) | {thread_id: .id, user: (.comments.nodes[0].author.login // "unknown"), body: (.comments.nodes[0].body // ""), diff_hunk: (.comments.nodes[0].diffHunk // ""), commit_id: (.comments.nodes[0].commit.oid // "")}')
5389

5490
if [ -n "$UNRESOLVED" ]; then
5591
echo "❌ Unresolved review comments found:"

‎scripts/extract_pr_logs.sh‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ if [[ -z "$INPUT" ]]; then
3636
exit 1
3737
fi
3838

39-
# Detect if input is PR number or run ID (run IDs are much longer)
40-
if [[ "$INPUT" =~ ^[0-9]{1,5}$ ]]; then
39+
# Detect if input is a PR number or a run ID.
40+
# Prefer PR interpretation whenever the number resolves to an existing PR.
41+
if [[ "$INPUT" =~ ^[0-9]+$ ]] && gh pr view "$INPUT" --json number --jq '.number' >/dev/null 2>&1; then
4142
PR_NUMBER="$INPUT"
4243
echo "🔍 Finding latest failed run for PR #$PR_NUMBER..." >&2
4344

0 commit comments

Comments
 (0)