t1386: Post-merge review feedback scanner#2786
Conversation
Scans merged PRs for unaddressed AI review bot suggestions (CodeRabbit, Gemini Code Assist, claude-review, gpt-review) and creates GitHub issues for follow-up. Supports dry-run mode, configurable lookback window via SCANNER_DAYS env var, and idempotent issue creation. Closes #2785
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new automation script designed to enhance code quality and ensure that valuable AI-generated review feedback is consistently addressed. By automatically scanning merged pull requests for unaddressed bot suggestions and creating corresponding GitHub issues, it streamlines the follow-up process and prevents important recommendations from being missed, thereby improving the overall maintainability and quality of the codebase. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdds a new Bash script Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Scanner as "post-merge-review-scanner.sh"
participant GitHubAPI as "GitHub API"
participant Issues as "GitHub Issues"
User->>Scanner: run scan / dry-run
Scanner->>GitHubAPI: list merged PRs since lookback
GitHubAPI-->>Scanner: merged PR list
loop For each PR (up to SCANNER_PR_LIMIT / SCANNER_MAX_ISSUES)
Scanner->>GitHubAPI: fetch reviews & review_comments for PR
GitHubAPI-->>Scanner: comments
Scanner->>Scanner: filter by bot users & actionable keywords
Scanner->>GitHubAPI: search for existing issue with SCANNER_LABEL referencing PR
GitHubAPI-->>Scanner: issue exists? (yes/no)
alt No existing issue & actionable items
Scanner->>Issues: create labeled issue (or log if dry-run)
Issues-->>Scanner: issue created
else Skip PR
end
end
Scanner-->>User: report summary / finished
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Tue Mar 3 16:14:26 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
Pulse triage (2026-03-03T16:18:47Z):
PR is blocked on CodeRabbit completing — expected within a few minutes. Will merge once all gates pass. |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Tue Mar 3 16:19:14 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.agents/scripts/post-merge-review-scanner.sh (1)
104-112: Preferprintfoverecho -efor summary rendering.
echo -einterprets escapes from comment text and can mutate output unexpectedly.Suggested refactor
- while IFS='|' read -r bot path snippet; do + while IFS='|' read -r bot path snippet; do local ref="" [[ -n "$path" ]] && ref=" (\`${path}\`)" - summary="${summary}- **${bot}**${ref}: ${snippet}...\n" + 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" "$(echo -e "$summary")" "$dry_run" + create_issue "$repo" "$pr" "$pr_title" "$summary" "$dry_run"As per coding guidelines,
.agents/scripts/*.sh: automation scripts should provide robust behavior and clear feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/post-merge-review-scanner.sh around lines 104 - 112, Replace the use of echo -e when passing the rendered summary into create_issue to avoid unintended escape interpretation; update the invocation that currently uses "$(echo -e "$summary")" to use printf with the "%b" format (e.g., pass the expanded summary via printf "%b" "$summary") so escapes are handled safely, keeping the same variables and call-site (summary and create_issue) otherwise unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/post-merge-review-scanner.sh:
- Around line 82-83: The gh pr list call that populates pr_numbers omits an
explicit pagination limit, so it only returns the default 30 results; update the
command used to populate pr_numbers (the gh pr list invocation) to add an
explicit --limit value (e.g., --limit 1000 or a shell variable like
PR_LIST_LIMIT) so the scan covers all merged PRs in the lookback window; ensure
the chosen limit is high enough for expected merge volume or make it
configurable via an environment variable used when constructing the gh pr list
command.
- Around line 49-51: The idempotency check in issue_exists() currently only
queries open issues and uses a generic search "PR #${pr}", so closed follow-ups
or unrelated matches will cause duplicates; change the gh query to include all
states (remove or replace --state open with --state all) and replace the generic
search with an exact title match built the same way as create_issue() does
(construct the TITLE the same way create_issue() forms the issue title and use
--search 'in:title "TITLE"' or the exact TITLE variable), keeping the same
output parsing (--json number --jq 'length') to determine existence; reference
issue_exists(), create_issue(), and SCANNER_LABEL to locate and update the call.
- Around line 35-44: The jq filters in variables jq_f and jq_r assume .body is
non-null which makes test() fail silently due to the trailing "|| true" and
redirected stderr; update both filters to coalesce nullable .body with // ""
before calling test() and gsub (e.g., use (.body // "") in place of .body) and
remove the broad error suppression (the 2>/dev/null || true on the gh api pipes)
so real jq errors surface; ensure you keep the existing selections on
.user.login and .path and preserve the truncation/gsub logic but applied to
(.body // "").
---
Nitpick comments:
In @.agents/scripts/post-merge-review-scanner.sh:
- Around line 104-112: Replace the use of echo -e when passing the rendered
summary into create_issue to avoid unintended escape interpretation; update the
invocation that currently uses "$(echo -e "$summary")" to use printf with the
"%b" format (e.g., pass the expanded summary via printf "%b" "$summary") so
escapes are handled safely, keeping the same variables and call-site (summary
and create_issue) otherwise unchanged.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new script to scan for unaddressed review comments on merged pull requests. The script is well-structured and makes good use of shell features for robustness. My review focuses on improving error handling and overall script resilience. The primary concern is the consistent use of 2>/dev/null to suppress error streams, which can hide critical issues like authentication failures or syntax errors, making the script difficult to debug. I've provided several high-severity suggestions to address this pattern, aligning with repository best practices. Additionally, I've included a medium-severity suggestion to improve the way multiline strings are constructed, making the script more robust against unexpected input from PR comments.
| { gh api "repos/${repo}/pulls/${pr}/comments" --paginate 2>/dev/null || echo '[]'; } | | ||
| jq -r "$jq_f" 2>/dev/null || true |
There was a problem hiding this comment.
Error suppression with 2>/dev/null for gh api and jq can hide important issues. For gh api, it can mask authentication or permission errors. For jq, it can hide syntax errors in the filter. This is against the repository's general rules, which advise against blanket error suppression to aid debugging. The || echo '[]' and || true constructs are sufficient to handle expected non-zero exit codes gracefully without hiding the underlying error messages.
| { gh api "repos/${repo}/pulls/${pr}/comments" --paginate 2>/dev/null || echo '[]'; } | | |
| jq -r "$jq_f" 2>/dev/null || true | |
| { gh api "repos/${repo}/pulls/${pr}/comments" --paginate || echo '[]'; } | | |
| jq -r "$jq_f" || true |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging. In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting when a command like 'jq' fails on an optional lookup. Do not suppress stderr with '2>/dev/null' so that actual syntax or system errors remain visible for debugging.
| { gh api "repos/${repo}/pulls/${pr}/reviews" --paginate 2>/dev/null || echo '[]'; } | | ||
| jq -r "$jq_r" 2>/dev/null || true |
There was a problem hiding this comment.
As with the comment fetching above, suppressing stderr with 2>/dev/null for gh api and jq should be avoided. This practice can hide critical errors like authentication failures or jq filter syntax errors, making the script harder to debug. Allowing stderr to be printed is aligned with the repository's general guidelines.
| { gh api "repos/${repo}/pulls/${pr}/reviews" --paginate 2>/dev/null || echo '[]'; } | | |
| jq -r "$jq_r" 2>/dev/null || true | |
| { gh api "repos/${repo}/pulls/${pr}/reviews" --paginate || echo '[]'; } | | |
| jq -r "$jq_r" || true |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging. In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting when a command like 'jq' fails on an optional lookup. Do not suppress stderr with '2>/dev/null' so that actual syntax or system errors remain visible for debugging.
| count=$(gh issue list --repo "$repo" --label "$SCANNER_LABEL" \ | ||
| --search "PR #${pr}" --state open --json number --jq 'length' 2>/dev/null || echo "0") |
There was a problem hiding this comment.
Suppressing stderr with 2>/dev/null here is risky. If the gh issue list command fails due to an authentication error, the command will silently fail, count will be set to "0", and issue_exists will incorrectly return false. This could lead to duplicate issues being created. It's better to let potential errors be visible for debugging, in line with repository guidelines.
| count=$(gh issue list --repo "$repo" --label "$SCANNER_LABEL" \ | |
| --search "PR #${pr}" --state open --json number --jq 'length' 2>/dev/null || echo "0") | |
| count=$(gh issue list --repo "$repo" --label "$SCANNER_LABEL" \ | |
| --search "PR #${pr}" --state open --json number --jq 'length' || echo "0") |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
| pr_numbers=$(gh pr list --state merged --search "merged:>${since_date}" \ | ||
| --repo "$repo" --json number --jq '.[].number' 2>/dev/null || echo "") |
There was a problem hiding this comment.
Suppressing stderr with 2>/dev/null can hide critical errors, such as authentication failures. If this gh pr list command fails, pr_numbers will be empty, and the script will incorrectly report "No merged PRs found" instead of surfacing the underlying problem. Please remove the error suppression to improve debuggability, as recommended by the repository's general rules.
| pr_numbers=$(gh pr list --state merged --search "merged:>${since_date}" \ | |
| --repo "$repo" --json number --jq '.[].number' 2>/dev/null || echo "") | |
| pr_numbers=$(gh pr list --state merged --search "merged:>${since_date}" \ | |
| --repo "$repo" --json number --jq '.[].number' || echo "") |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
| done <<<"$hits" | ||
| [[ -z "$summary" ]] && continue | ||
| log "PR #${pr}: creating issue" | ||
| create_issue "$repo" "$pr" "$pr_title" "$(echo -e "$summary")" "$dry_run" |
There was a problem hiding this comment.
Using echo -e can be risky as it interprets backslash escapes within the $summary variable. Since the summary contains content from PR comments (snippet), it could unintentionally alter the output if a comment contains sequences like \t or \c. A safer approach is to build the summary string with literal newlines and pass it directly to create_issue.
To do this, you can modify line 107 to append a true newline:
# In line 107
summary+="- **${bot}**${ref}: ${snippet}..."$'
'And then change this line to pass $summary directly, without echo -e.
| create_issue "$repo" "$pr" "$pr_title" "$(echo -e "$summary")" "$dry_run" | |
| create_issue "$repo" "$pr" "$pr_title" "$summary" "$dry_run" |
…(t1386) - Add --limit flag to gh pr list (configurable via SCANNER_PR_LIMIT, default 1000) - issue_exists() now uses --state all and exact title matching to catch closed issues - Null-coalesce .body with // "" in jq filters to handle null bodies safely - Remove broad 2>/dev/null || true on jq to surface real parse errors - Replace echo -e with printf '%b' for portable escape interpretation
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Tue Mar 3 16:24:04 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.agents/scripts/post-merge-review-scanner.sh (2)
37-37: Optional: Null-coalesce.user.loginfor complete defensive filtering.The
.bodyfields are now properly guarded with// "", but.user.loginisn't. While null logins are extremely rare (deleted user edge case), adding the guard would make the filter fully robust.♻️ Defensive hardening
- local jq_f='[.[] | select(.user.login | test("'"$BOT_RE"'";"i")) + local jq_f='[.[] | select((.user.login // "") | test("'"$BOT_RE"'";"i"))Same pattern applies to
jq_ron line 42.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/post-merge-review-scanner.sh at line 37, The jq filters jq_f and jq_r should defensively coalesce .user.login like the existing .body guards so null/absent logins don't break matching; update the filter expressions that currently use .user.login | test(...) to use (.user.login // "") | test(..., "i") in both the jq_f and jq_r definitions (preserving the case-insensitive flag and surrounding logic) so deleted/NULL user cases are safely ignored.
40-46: Silent API failures could mask missed feedback.When
gh apifails (rate limit, network error, auth issue), the2>/dev/null || echo '[]'pattern silently proceeds with empty results. The scanner won't know it missed PRs, undermining reliability.Consider logging API failures while still providing the fallback:
♻️ Proposed improvement for observability
- { gh api "repos/${repo}/pulls/${pr}/comments" --paginate 2>/dev/null || echo '[]'; } | + { gh api "repos/${repo}/pulls/${pr}/comments" --paginate 2>/dev/null || { log "WARN: API failed for PR #${pr} comments"; echo '[]'; }; } | jq -r "$jq_f" ... - { gh api "repos/${repo}/pulls/${pr}/reviews" --paginate 2>/dev/null || echo '[]'; } | + { gh api "repos/${repo}/pulls/${pr}/reviews" --paginate 2>/dev/null || { log "WARN: API failed for PR #${pr} reviews"; echo '[]'; }; } | jq -r "$jq_r"As per coding guidelines:
.agents/scripts/*.shautomation scripts should prioritize clear logging and feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/post-merge-review-scanner.sh around lines 40 - 46, The gh api calls currently swallow errors via the "2>/dev/null || echo '[]'" pattern (used around the gh api "repos/${repo}/pulls/${pr}/comments" and "repos/${repo}/pulls/${pr}/reviews" invocations and the jq_f/jq_r pipelines); change each call to capture stderr and the exit status, and when gh api fails emit a clear error log to stderr (include repo, pr and the captured error text) before falling back to "[]", e.g. run gh api into a temp variable or redirect stderr to a variable, test $? and print a descriptive message to >&2 so failures aren’t silent while preserving the empty-array fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.agents/scripts/post-merge-review-scanner.sh:
- Line 37: The jq filters jq_f and jq_r should defensively coalesce .user.login
like the existing .body guards so null/absent logins don't break matching;
update the filter expressions that currently use .user.login | test(...) to use
(.user.login // "") | test(..., "i") in both the jq_f and jq_r definitions
(preserving the case-insensitive flag and surrounding logic) so deleted/NULL
user cases are safely ignored.
- Around line 40-46: The gh api calls currently swallow errors via the
"2>/dev/null || echo '[]'" pattern (used around the gh api
"repos/${repo}/pulls/${pr}/comments" and "repos/${repo}/pulls/${pr}/reviews"
invocations and the jq_f/jq_r pipelines); change each call to capture stderr and
the exit status, and when gh api fails emit a clear error log to stderr (include
repo, pr and the captured error text) before falling back to "[]", e.g. run gh
api into a temp variable or redirect stderr to a variable, test $? and print a
descriptive message to >&2 so failures aren’t silent while preserving the
empty-array fallback.
…ew-scanner (t1386) - Remove all 2>/dev/null stderr suppression from gh api, jq, gh pr list, gh issue list, gh pr view, gh repo view, and gh label create calls. Keep || fallbacks (|| echo '[]', || true, || echo '') for graceful error recovery without hiding auth/syntax errors (Gemini high severity) - Add --limit 100 to gh issue list in issue_exists() to prevent default 30-item cap from missing existing follow-up issues (CodeRabbit major) - Harden jq filters with (.user.login // "") null coalescing to prevent test() failures on nullable fields (CodeRabbit major) - Replace echo -e with printf -v for summary rendering to avoid unintended escape interpretation from PR comment content (CodeRabbit nitpick) Closes #2785
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Tue Mar 3 16:29:28 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
Worker killed after 4+ hours with 0 commits (struggle_ratio: 7323). PR is APPROVED with CI nearly complete — merging directly once Framework Validation check passes. |
- Null-coalesce .user.login with // "" in both jq_f and jq_r filters to handle deleted-user edge case (CodeRabbit nitpick) - Replace echo-style \n string concatenation with printf format string to avoid unintended escape interpretation in comment text - ShellCheck: zero violations
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Tue Mar 3 16:32:10 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
2 participants
Summary
.agents/scripts/post-merge-review-scanner.sh— scans merged PRs for unaddressed AI review bot suggestions and creates GitHub issues for follow-upscan(creates issues) anddry-run(preview only) modesSCANNER_DAYS(lookback window, default 7),SCANNER_MAX_ISSUES(cap per run, default 10),SCANNER_LABEL(issue label, defaultreview-followup)Details
Detects actionable comments from CodeRabbit, Gemini Code Assist, claude-review, and gpt-review bots containing keywords: should, consider, fix, change, update, refactor, missing, add. Idempotent — skips PRs that already have a follow-up issue.
Verification:
helpcommand: worksdry-run marcusquinn/aidevops: found 10 PRs with unaddressed bot feedback (correctly capped at SCANNER_MAX_ISSUES)Closes #2785
Summary by CodeRabbit