Skip to content

Commit 08e70e8

Browse files
derekmislerderekmisler
committed
feat: auto-resolve stale bot review threads on re-review
Signed-off-by: Derek Misler <derek.misler@docker.com>
1 parent 99bfcb3 commit 08e70e8

File tree

4 files changed

+192
-5
lines changed

4 files changed

+192
-5
lines changed

‎CAGENT_VERSION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
v1.27.1
1+
v1.28.1

‎review-pr/action.yml‎

Lines changed: 186 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,191 @@ runs:
178178
fi
179179
fi
180180
181+
- name: Resolve stale review threads
182+
continue-on-error: true
183+
shell: bash
184+
env:
185+
GH_TOKEN: ${{ steps.resolve-token.outputs.token }}
186+
PR_NUMBER: ${{ steps.resolve-context.outputs.pr-number }}
187+
REPO: ${{ github.repository }}
188+
run: |
189+
echo "🔍 Checking for stale bot review threads to resolve..."
190+
191+
# A. Parse diff → file:line set
192+
# Tracks current file from "diff --git" headers, line numbers from @@ hunks,
193+
# and emits "file:line" for each added (+) line.
194+
if [ ! -f pr.diff ]; then
195+
echo "ℹ️ No pr.diff found — skipping stale thread resolution"
196+
exit 0
197+
fi
198+
199+
DIFF_LINES_FILE=$(mktemp)
200+
trap "rm -f '$DIFF_LINES_FILE'" EXIT
201+
202+
awk '
203+
/^\+\+\+ b\// {
204+
# Extract file path from "+++ b/foo" (unambiguous, unlike diff --git header)
205+
file = substr($0, 7)
206+
}
207+
/^@@ / {
208+
# Parse new file line number from "@@ -X,Y +Z,W @@"
209+
match($0, /\+([0-9]+)/, arr)
210+
line = arr[1]
211+
next
212+
}
213+
/^\+[^+]/ || /^\+$/ {
214+
# Added line (but not the +++ header)
215+
if (file != "" && line > 0) {
216+
print file ":" line
217+
}
218+
line++
219+
}
220+
/^ / { line++ }
221+
/^-/ { next }
222+
' pr.diff | sort -u > $DIFF_LINES_FILE
223+
224+
DIFF_LINE_COUNT=$(wc -l < $DIFF_LINES_FILE | tr -d ' ')
225+
echo "📄 Found $DIFF_LINE_COUNT unique file:line pairs in diff"
226+
227+
# B. Fetch all review threads via GraphQL (paginated)
228+
OWNER="${REPO%%/*}"
229+
REPO_NAME="${REPO##*/}"
230+
231+
QUERY='
232+
query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) {
233+
repository(owner: $owner, name: $repo) {
234+
pullRequest(number: $pr) {
235+
reviewThreads(first: 100, after: $cursor) {
236+
pageInfo { hasNextPage endCursor }
237+
nodes {
238+
id
239+
isResolved
240+
path
241+
line
242+
comments(first: 5) {
243+
nodes { body }
244+
}
245+
}
246+
}
247+
}
248+
}
249+
}'
250+
251+
ALL_THREADS="[]"
252+
CURSOR=""
253+
PAGE=0
254+
FETCH_OK=true
255+
256+
while true; do
257+
PAGE=$((PAGE + 1))
258+
259+
GQL_ARGS=(-f query="$QUERY" -f owner="$OWNER" -f repo="$REPO_NAME" -F pr="$PR_NUMBER")
260+
if [ -n "$CURSOR" ]; then
261+
GQL_ARGS+=(-f cursor="$CURSOR")
262+
fi
263+
264+
RESULT=$(gh api graphql "${GQL_ARGS[@]}" 2>&1) || {
265+
echo "::warning::GraphQL query failed (page $PAGE): $RESULT"
266+
FETCH_OK=false
267+
break
268+
}
269+
270+
# Check for GraphQL errors in response (HTTP 200 can still contain errors)
271+
if echo "$RESULT" | jq -e '.errors' > /dev/null 2>&1; then
272+
echo "::warning::GraphQL returned errors: $(echo "$RESULT" | jq -c '.errors')"
273+
FETCH_OK=false
274+
break
275+
fi
276+
277+
THREADS=$(echo "$RESULT" | jq '.data.repository.pullRequest.reviewThreads.nodes // []') || {
278+
echo "::warning::Failed to parse threads on page $PAGE"
279+
FETCH_OK=false
280+
break
281+
}
282+
ALL_THREADS=$(echo "$ALL_THREADS" "$THREADS" | jq -s '.[0] + .[1]') || {
283+
echo "::warning::Failed to merge threads on page $PAGE"
284+
FETCH_OK=false
285+
break
286+
}
287+
288+
HAS_NEXT=$(echo "$RESULT" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage')
289+
if [ "$HAS_NEXT" != "true" ]; then
290+
break
291+
fi
292+
CURSOR=$(echo "$RESULT" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.endCursor')
293+
done
294+
295+
if [ "$FETCH_OK" != "true" ]; then
296+
echo "::warning::Thread fetch incomplete — skipping resolution to avoid acting on partial data"
297+
exit 0
298+
fi
299+
300+
TOTAL_THREADS=$(echo "$ALL_THREADS" | jq 'length')
301+
echo "📋 Fetched $TOTAL_THREADS review threads (across $PAGE page(s))"
302+
303+
# C. Filter to unresolved bot threads (containing <!-- cagent-review --> marker)
304+
BOT_THREADS=$(echo "$ALL_THREADS" | jq '[
305+
.[] | select(
306+
.isResolved == false and
307+
(.comments.nodes | any(.body | contains("<!-- cagent-review -->")))
308+
)
309+
]')
310+
311+
BOT_COUNT=$(echo "$BOT_THREADS" | jq 'length')
312+
echo "🤖 Found $BOT_COUNT unresolved bot review thread(s)"
313+
314+
if [ "$BOT_COUNT" -eq 0 ]; then
315+
echo "✅ No stale bot threads to resolve"
316+
exit 0
317+
fi
318+
319+
# D. Resolve stale threads (file:line no longer in diff)
320+
RESOLVED=0
321+
KEPT=0
322+
323+
for i in $(seq 0 $((BOT_COUNT - 1))); do
324+
THREAD_ID=$(echo "$BOT_THREADS" | jq -r ".[$i].id")
325+
THREAD_PATH=$(echo "$BOT_THREADS" | jq -r ".[$i].path")
326+
THREAD_LINE=$(echo "$BOT_THREADS" | jq -r ".[$i].line")
327+
328+
# Skip threads with null path or line (outdated diff positions, file-level comments)
329+
if [ "$THREAD_PATH" = "null" ] || [ -z "$THREAD_PATH" ] || \
330+
[ "$THREAD_LINE" = "null" ] || [ -z "$THREAD_LINE" ]; then
331+
echo " ⏭️ Keeping open: thread $THREAD_ID (path or line is null/outdated)"
332+
KEPT=$((KEPT + 1))
333+
continue
334+
fi
335+
336+
FILE_LINE="${THREAD_PATH}:${THREAD_LINE}"
337+
338+
if grep -qF "$FILE_LINE" $DIFF_LINES_FILE; then
339+
echo " ⏭️ Keeping open: $FILE_LINE (still in diff)"
340+
KEPT=$((KEPT + 1))
341+
else
342+
MUTATION_RESULT=$(gh api graphql -f query='
343+
mutation($threadId: ID!) {
344+
resolveReviewThread(input: { threadId: $threadId }) {
345+
thread { id isResolved }
346+
}
347+
}
348+
' -f threadId="$THREAD_ID" 2>&1) || {
349+
echo " ⚠️ Failed to resolve thread $THREAD_ID (API error)"
350+
continue
351+
}
352+
353+
if echo "$MUTATION_RESULT" | jq -e '.errors' > /dev/null 2>&1; then
354+
echo " ⚠️ Failed to resolve thread $THREAD_ID: $(echo "$MUTATION_RESULT" | jq -c '.errors')"
355+
continue
356+
fi
357+
358+
echo " ✅ Resolved: $FILE_LINE (no longer in diff)"
359+
RESOLVED=$((RESOLVED + 1))
360+
fi
361+
done
362+
363+
echo ""
364+
echo "📋 Summary: Resolved $RESOLVED thread(s), kept $KEPT thread(s) open"
365+
181366
- name: Ensure cache directory exists
182367
shell: bash
183368
run: mkdir -p "${{ github.workspace }}/.cache"
@@ -324,7 +509,7 @@ runs:
324509
echo '3. **Verify**: For each hypothesis, delegate to `verifier` agent'
325510
echo '4. **Post**: Aggregate findings and post review via `gh api`'
326511
echo ""
327-
echo "Only report CONFIRMED and LIKELY findings. Approve if no issues found."
512+
echo "Only report CONFIRMED and LIKELY findings. Always post as COMMENT (never APPROVE or REQUEST_CHANGES)."
328513
} > review_context.md
329514
330515
# Append extra prompt if provided

‎review-pr/agents/pr-review.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ agents:
153153
- MINOR = everything else
154154
3. **Label the assessment** (informational only — does NOT change the event type):
155155
- ANY CRITICAL findings → label as "🔴 CRITICAL" in the summary
156-
- ANY NOTABLE findings (no CRITICAL) → label as "🟡 NEEDS_ATTENTION"
156+
- ANY NOTABLE findings (no CRITICAL) → label as "🟡 NEEDS ATTENTION"
157157
- Only MINOR or no findings → label as "🟢 APPROVE"
158158
4. **Post the review**: The GitHub review event is ALWAYS `COMMENT`,
159159
regardless of the assessment label. Never use `APPROVE` or `REQUEST_CHANGES`.
@@ -173,7 +173,7 @@ agents:
173173
174174
```
175175
## Review: COMMENT
176-
### Assessment: [🟢 APPROVE|🟡 NEEDS_ATTENTION|🔴 CRITICAL]
176+
### Assessment: [🟢 APPROVE|🟡 NEEDS ATTENTION|🔴 CRITICAL]
177177
### Summary
178178
<assessment>
179179
### Findings

‎tests/test-job-summary.sh‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
set -e
66

7+
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
8+
79
echo "=========================================="
810
echo "Testing Job Summary Format"
911
echo "=========================================="
@@ -35,7 +37,7 @@ echo "---"
3537
echo "| Agent | \`agents/security-scanner.yaml\` |"
3638
echo "| Exit Code | 0 |"
3739
echo "| Execution Time | 45s |"
38-
echo "| cagent Version | v1.27.1 |"
40+
echo "| cagent Version | $(cat "$SCRIPT_DIR/../CAGENT_VERSION" | tr -d '[:space:]') |"
3941
echo "| MCP Gateway | false |"
4042
echo ""
4143
echo "✅ **Status:** Success"

0 commit comments

Comments
 (0)