Skip to content

GH#18538: emit worker-actionable review-followup issue bodies #18607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcusquinn merged 1 commit into from
Apr 13, 2026
+143 −40
Merged

GH#18538: emit worker-actionable review-followup issue bodies #18607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 142 additions & 38 deletions .agents/scripts/post-merge-review-scanner.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,19 @@
# Assist, claude-review, gpt-review) on recently merged PRs and creates
# GitHub issues for follow-up. Idempotent — skips PRs with existing issues.
#
# Issue bodies are worker-actionable per the t1901 mandatory rule: each
# includes file:line refs, a Worker Guidance section, and direct links to
# the inline review comments. Workers must be able to fix the issue without
# re-fetching the source PR's review API.
#
# Usage: post-merge-review-scanner.sh {scan|dry-run|help} [REPO]
# Env: SCANNER_DAYS (default 7), SCANNER_MAX_ISSUES (default 10),
# SCANNER_LABEL (default review-followup),
# SCANNER_PR_LIMIT (default 1000)
# SCANNER_PR_LIMIT (default 1000),
# SCANNER_MAX_COMMENTS (default 10) — cap per issue body
#
# t1386: https://github.com/marcusquinn/aidevops/issues/2785
# GH#18538: workers timed out on review-followup issues with truncated bodies.
set -euo pipefail

# Source shared-constants for gh_create_issue wrapper (t1756)
Expand All @@ -24,6 +31,7 @@ SCANNER_DAYS="${SCANNER_DAYS:-7}"
SCANNER_MAX_ISSUES="${SCANNER_MAX_ISSUES:-10}"
SCANNER_LABEL="${SCANNER_LABEL:-review-followup}"
SCANNER_PR_LIMIT="${SCANNER_PR_LIMIT:-1000}"
SCANNER_MAX_COMMENTS="${SCANNER_MAX_COMMENTS:-10}"
BOT_RE="coderabbitai|gemini-code-assist|claude-review|gpt-review"
ACT_RE="should|consider|fix|change|update|refactor|missing|add"

Expand All @@ -38,19 +46,125 @@ get_lookback_date() {
fi
}

# Fetch actionable bot comments for a PR. Output: "bot|path|snippet" per line.
fetch_actionable() {
# Fetch inline review comments and format each as a markdown block. Each
# block includes the bot login, file:line, a direct link to the comment, and
# the full body (capped at 2000 chars) as a markdown blockquote so workers
# can read the bot's full reasoning without re-fetching the API.
fetch_inline_comments_md() {
local repo="$1" pr="$2"
gh api "repos/${repo}/pulls/${pr}/comments" --paginate 2>/dev/null |
jq -r --arg bots "$BOT_RE" --arg acts "$ACT_RE" --argjson cap "$SCANNER_MAX_COMMENTS" '
[.[]
| select((.user.login // "") | test($bots; "i"))
| select((.body // "") | test($acts; "i"))
]
| if length == 0 then "" else
.[:$cap] | map(
"#### \(.user.login) on `\(.path // "<no path>"):\(.line // .original_line // "?")`\n\n"
+ "[View inline comment](\(.html_url))\n\n"
+ ((.body // "")[:2000] | split("\n") | map("> " + .) | join("\n"))
+ "\n"
) | join("\n")
end
' 2>/dev/null || true
}

# Fetch top-level PR review summaries (Gemini's "Code Review" body, etc.)
# and format each as a markdown block. Same shape as inline comments but
# without file:line refs.
fetch_review_summaries_md() {
local repo="$1" pr="$2"
gh api "repos/${repo}/pulls/${pr}/reviews" --paginate 2>/dev/null |
jq -r --arg bots "$BOT_RE" --arg acts "$ACT_RE" --argjson cap "$SCANNER_MAX_COMMENTS" '
[.[]
| select((.user.login // "") | test($bots; "i"))
| select((.body // "") | length > 0)
| select((.body // "") | test($acts; "i"))
]
| if length == 0 then "" else
.[:$cap] | map(
"#### \(.user.login) review summary\n\n"
+ "[View review](\(.html_url))\n\n"
+ ((.body // "")[:2000] | split("\n") | map("> " + .) | join("\n"))
+ "\n"
) | join("\n")
end
' 2>/dev/null || true
}

# Deduped list of `path:line` refs from inline comments, formatted as a
# markdown bullet list. Used in the Worker Guidance section so workers can
# see the full set of files to read at a glance.
fetch_file_refs_md() {
local repo="$1" pr="$2"
local jq_f='[.[] | select((.user.login // "") | test("'"$BOT_RE"'";"i"))
| select((.body // "") | test("'"$ACT_RE"'";"i"))
| "\((.user.login // ""))|\(.path // "")|\((.body // "") | gsub("\n";" ") | .[:200])"] | .[]'
{ gh api "repos/${repo}/pulls/${pr}/comments" --paginate || echo '[]'; } |
jq -r "$jq_f"
local jq_r='[.[] | select((.user.login // "") | test("'"$BOT_RE"'";"i"))
| select((.body // "") | test("'"$ACT_RE"'";"i"))
| "\((.user.login // ""))||\((.body // "") | gsub("\n";" ") | .[:200])"] | .[]'
{ gh api "repos/${repo}/pulls/${pr}/reviews" --paginate || echo '[]'; } |
jq -r "$jq_r"
gh api "repos/${repo}/pulls/${pr}/comments" --paginate 2>/dev/null |
jq -r --arg bots "$BOT_RE" --arg acts "$ACT_RE" '
[.[]
| select((.user.login // "") | test($bots; "i"))
| select((.body // "") | test($acts; "i"))
| select(.path != null)
| "- `\(.path):\(.line // .original_line // "?")`"
] | unique | .[]
' 2>/dev/null || true
}

# Build a worker-actionable issue body for a PR's unaddressed bot feedback.
# Returns 1 (and prints nothing) if there's no actionable content.
build_pr_followup_body() {
local repo="$1" pr="$2"
local inline review file_refs refs_section
inline=$(fetch_inline_comments_md "$repo" "$pr")
review=$(fetch_review_summaries_md "$repo" "$pr")
file_refs=$(fetch_file_refs_md "$repo" "$pr")

if [[ -z "$inline" && -z "$review" ]]; then
return 1
fi

if [[ -n "$file_refs" ]]; then
refs_section="$file_refs"
else
refs_section="- _(No file paths in inline comments — see PR review summaries below for context)_"
fi

cat <<MD
## Unaddressed review bot suggestions

PR #${pr} was merged with unaddressed review bot feedback. Each comment
below includes its file path, line number, and a direct link to the
inline review comment. Read the relevant lines, decide whether the
suggestion is correct, and either apply the fix or close this issue
with a wontfix rationale.

**Source PR:** https://github.com/${repo}/pull/${pr}

### Worker Guidance

**Files to modify:**

${refs_section}

**Implementation steps:**

1. Read each file at the specified \`:line\` (read ~20 lines around for context).
2. Read the bot's full comment below — it contains the rationale and suggested change.
3. Apply the change if it's correct. If you disagree, close this issue with an explanation rather than burning iterations trying to satisfy a wrong suggestion.
4. If multiple comments target the same file, group your edits into one logical commit.
5. Run \`shellcheck\` / \`markdownlint-cli2\` / project tests as appropriate.

**Verification:**

- Open the new PR with \`Resolves #<this-issue>\` so this followup is auto-closed on merge.
- If the bot's suggestion was incorrect, leave a comment on this issue explaining why before closing — that comment trains the next session reading this thread.

### Inline comments

${inline:-_(none)_}

### PR review summaries

${review:-_(none)_}
MD
}

issue_exists() {
Expand All @@ -63,34 +177,28 @@ issue_exists() {
}

create_issue() {
local repo="$1" pr="$2" pr_title="$3" summary="$4" dry_run="$5"
local repo="$1" pr="$2" pr_title="$3" body="$4" dry_run="$5"
local title="Review followup: PR #${pr} — ${pr_title}"
if [[ "$dry_run" == "true" ]]; then
log "[DRY-RUN] Would create: $title"
log "[DRY-RUN] Body length: ${#body} chars"
return 0
fi
gh label create "$SCANNER_LABEL" --repo "$repo" \
--description "Unaddressed review bot feedback" --color "D4C5F9" || true
gh label create "source:review-scanner" --repo "$repo" \
--description "Auto-created by post-merge-review-scanner.sh" --color "C2E0C6" --force || true
local body
# Build signature footer
local sig_footer=""
local sig_helper

# Append signature footer
local sig_footer="" sig_helper
sig_helper="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)/gh-signature-helper.sh"
if [[ -x "$sig_helper" ]]; then
sig_footer=$("$sig_helper" footer 2>/dev/null || echo "")
fi

body="## Unaddressed review bot suggestions

PR #${pr} was merged with unaddressed review bot feedback.
**Source PR:** https://github.com/${repo}/pull/${pr}

### Actionable comments

${summary}${sig_footer}"
gh_create_issue --repo "$repo" --title "$title" --label "$SCANNER_LABEL,source:review-scanner" --body "$body"
gh_create_issue --repo "$repo" --title "$title" \
--label "$SCANNER_LABEL,source:review-scanner" \
--body "${body}${sig_footer}"
}

do_scan() {
Expand All @@ -115,19 +223,15 @@ do_scan() {
log "PR #${pr}: issue exists, skip"
continue
fi
local hits
hits=$(fetch_actionable "$repo" "$pr")
[[ -z "$hits" ]] && continue
local pr_title summary=""
local body
body=$(build_pr_followup_body "$repo" "$pr") || {
log "PR #${pr}: no actionable bot feedback, skip"
continue
}
local pr_title
pr_title=$(gh pr view "$pr" --repo "$repo" --json title --jq '.title' || echo "Unknown")
while IFS='|' read -r bot path snippet; do
local ref=""
[[ -n "$path" ]] && ref=" (\`${path}\`)"
printf -v summary '%s- **%s**%s: %s...\n' "$summary" "$bot" "$ref" "$snippet"
done <<<"$hits"
[[ -z "$summary" ]] && continue
log "PR #${pr}: creating issue"
create_issue "$repo" "$pr" "$pr_title" "$summary" "$dry_run"
create_issue "$repo" "$pr" "$pr_title" "$body" "$dry_run"
issues_created=$((issues_created + 1))
done <<<"$pr_numbers"
log "Done. Issues created: ${issues_created}"
Expand Down
3 changes: 1 addition & 2 deletions TODO.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ Tasks with no open blockers - ready to work on. Use `/ready` to refresh this lis

- [x] t1985 test: extend stub-harness pattern from t1969 to issue-sync-lib.sh — t1983's P0 bug would have been caught on day one by a stub-based test covering add_gh_ref_to_todo. Same pattern as test-privacy-guard.sh (t1969), applied to issue-sync-lib.sh with ~10 tests covering 6 functions (add_gh_ref_to_todo, fix_gh_ref_in_todo, add_pr_ref_to_todo, strip_code_fences, _escape_ere, parse_task_line). Include explicit regression test for t1983. #test #auto-dispatch #interactive ~2h tier:standard ref:GH#18404 logged:2026-04-12 blocked-by:t1983 -> [todo/tasks/t1985-brief.md] pr:#18430 completed:2026-04-12

- [x] t1990 feat(pre-edit-check): canonical stays on main in interactive sessions — no main-branch planning exception for interactive. is_main_allowlisted_path() short-circuits FALSE when no headless env var is set (FULL_LOOP_HEADLESS / AIDEVOPS_HEADLESS / OPENCODE_HEADLESS / GITHUB_ACTIONS). Every interactive edit — TODO.md, todo/**, README.md, code — goes through a linked worktree. Headless sessions keep the allowlist for routine bookkeeping. Docs updated in AGENTS.md + build.txt. #enhancement #interactive ~45m tier:standard logged:2026-04-12 -> [todo/tasks/t1990-brief.md] pr:#18414 completed:2026-04-12

- [x] t1995 feat(git-hooks): post-checkout hook warning when canonical goes off main — complements t1990's edit-time check by catching the branch switch itself. Warn-only by default (strict mode opt-in via AIDEVOPS_CANONICAL_GUARD=strict), bypass via AIDEVOPS_CANONICAL_GUARD=bypass. Fail-open on headless sessions, worktrees, main/master, detached HEAD, missing repos.json. Detects and migrates legacy "t1988 session, local install" hook. Auto-installs across all initialized repos via setup.sh. 8-test harness. #security #interactive ~1h tier:standard logged:2026-04-12 -> [todo/tasks/t1995-brief.md] pr:#18427 completed:2026-04-12

- [x] t1998 fix(pulse-dispatch-core): skip-if-already-labeled short-circuit neutered the simplification re-eval loop — `_issue_targets_large_files` returned 0 immediately on any `needs-simplification`-labeled issue, which meant `_reevaluate_simplification_labels` could never see a cleared case. Stale labels persisted forever even after target files were simplified below threshold. Trigger: #18346 referenced pulse-wrapper.sh (was 13,797 lines, now 1,352 after t1962 decomposition) but label survived. Fix: add force_recheck parameter, re-eval passes true. Normal dispatch path unchanged. #bug #pulse #interactive ~30m tier:simple logged:2026-04-12 -> [todo/tasks/t1998-brief.md] pr:#18438 completed:2026-04-12
Expand Down Expand Up @@ -2167,6 +2165,7 @@ t165,Provider-agnostic task claiming via TODO.md,marcusquinn,orchestration archi

## Done

- [x] t1990 feat(pre-edit-check): canonical stays on main in interactive sessions — no main-branch planning exception for interactive. is_main_allowlisted_path() short-circuits FALSE when no headless env var is set (FULL_LOOP_HEADLESS / AIDEVOPS_HEADLESS / OPENCODE_HEADLESS / GITHUB_ACTIONS). Every interactive edit — TODO.md, todo/**, README.md, code — goes through a linked worktree. Headless sessions keep the allowlist for routine bookkeeping. Docs updated in AGENTS.md + build.txt. #enhancement #interactive ~45m tier:standard logged:2026-04-12 -> [todo/tasks/t1990-brief.md] pr:#18414 completed:2026-04-12
- [x] t1387 Add conversational memory lookup guidance to build.txt harness — three-tier progressive discovery (local cached → local indexed → remote) covering 8 knowledge sources so agents proactively search when users reference past work from prior sessions #feature #agent #memory ~30m model:opus ref:GH#2798 logged:2026-03-04 pr:#2797 completed:2026-03-04
- [x] t205 YouTube competitor research and content automation agent #feature #youtube #content ~6h actual:4h logged:2026-02-09 started:2026-02-09 completed:2026-02-09 verified:2026-02-09 pr:#811
- Notes: Post-merge verification (2026-02-09): ShellCheck clean (SC1091 info only), markdownlint clean after PR #828 fixed 12 MD022 violations, all cross-references resolve, live API tests pass (auth-test, channel @mkbhd, quota tracking). 9/10000 API units used.
Expand Down
Loading