feat: add pr-batch-report skill#92
Conversation
Summary by CodeRabbitRelease Notes
WalkthroughAdded a new skill for batch reporting of GitHub Pull Requests. The skill consists of documentation describing functionality and a Python script that fetches PR metadata and diffs, aggregates results, and outputs formatted reports via multiple formats and input methods. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script as pr_batch_report.py
participant GH as gh CLI
participant API as GitHub API
participant Output
User->>Script: PR URLs (args/file/stdin)
Script->>Script: Normalize & deduplicate URLs
Note over Script: Parallel processing (--jobs)
loop For each PR
Script->>GH: gh pr view --json (URL, title, body, state, checks)
GH->>API: Fetch PR metadata
API-->>GH: PR data + statusCheckRollup
GH-->>Script: JSON response
Script->>Script: Classify CI status (pending/failing/passing)
alt --no-diff not set
Script->>GH: gh pr diff
GH->>API: Fetch unified diff
API-->>GH: Diff output
GH-->>Script: Diff text
Script->>Script: Parse diff headers for paths
end
Script->>Script: Aggregate into Row (metadata + diff + stats)
end
Script->>Output: Format & render (--format markdown|json)
Output-->>User: Formatted report
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/pr-batch-report/scripts/pr_batch_report.py`:
- Around line 54-64: The current logic reads from stdin whenever urls is empty,
which causes stdin to be consumed even if args.file was explicitly provided;
update the flow in the block using args.file, urls, normalize_pr_url and
sys.stdin.isatty() so that stdin is only read when no --file was provided (i.e.,
args.file is falsy) and stdin is not a TTY; ensure that when args.file is set we
do not enter the stdin-reading loop even if the file produced zero URLs.
- Around line 75-97: The _gh_json and _gh_diff functions call subprocess.run
without timeouts which can hang; add a timeout argument (e.g., timeout=30) to
both subprocess.run invocations and catch subprocess.TimeoutExpired around each
call: for _gh_json return a dict like {"url": url, "error": "gh pr view timed
out"} (or include timeout seconds) and for _gh_diff return a string indicating
the diff command timed out, ensuring you keep the existing behavior for
non-timeout failures and still use r.stdout/r.stderr when available and preserve
GH_JSON_FIELDS and the function signatures.
- Around line 117-123: The StatusContext branch currently only flags "FAILURE"
and "PENDING" but ignores "ERROR" and "EXPECTED", causing them to be treated as
passing; update the conditional in the tn == "StatusContext" block (the code
that sets ctx, state and appends to failing or pending lists) so that state
values "ERROR" and "EXPECTED" are treated as non-passing (e.g., include them
with "FAILURE" in the failing.append branch) and keep "PENDING" in the
pending.append branch; adjust the checks on the state variable used in that
block (ctx, state, failing, pending) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 71e57b04-02df-4854-b9ce-c1f3614c5cbc
📒 Files selected for processing (2)
skills/pr-batch-report/SKILL.mdskills/pr-batch-report/scripts/pr_batch_report.py
| if args.file: | ||
| with open(args.file, encoding="utf-8") as f: | ||
| for line in f: | ||
| n = normalize_pr_url(line) | ||
| if n: | ||
| urls.append(n) | ||
| if not urls and not sys.stdin.isatty(): | ||
| for line in sys.stdin: | ||
| n = normalize_pr_url(line) | ||
| if n: | ||
| urls.append(n) |
There was a problem hiding this comment.
Honor --file precedence before falling back to stdin.
Line 60 still reads stdin whenever the accumulated list is empty, even if --file was explicitly supplied. An empty or comment-only file will therefore silently consume piped input and report on the wrong PR set.
Suggested fix
- if not urls and not sys.stdin.isatty():
+ if not urls and not args.file and not sys.stdin.isatty():
for line in sys.stdin:
n = normalize_pr_url(line)
if n:
urls.append(n)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/pr-batch-report/scripts/pr_batch_report.py` around lines 54 - 64, The
current logic reads from stdin whenever urls is empty, which causes stdin to be
consumed even if args.file was explicitly provided; update the flow in the block
using args.file, urls, normalize_pr_url and sys.stdin.isatty() so that stdin is
only read when no --file was provided (i.e., args.file is falsy) and stdin is
not a TTY; ensure that when args.file is set we do not enter the stdin-reading
loop even if the file produced zero URLs.
| def _gh_json(url: str) -> dict[str, Any]: | ||
| r = subprocess.run( | ||
| ["gh", "pr", "view", url, "--json", GH_JSON_FIELDS], | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| if r.returncode != 0: | ||
| err = (r.stderr or r.stdout or "").strip() | ||
| return {"url": url, "error": err or f"gh exit {r.returncode}"} | ||
| return json.loads(r.stdout) | ||
|
|
||
|
|
||
| def _gh_diff(url: str) -> str: | ||
| r = subprocess.run( | ||
| ["gh", "pr", "diff", url], | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| if r.returncode != 0: | ||
| return f"(gh pr diff failed: {(r.stderr or '').strip()})" | ||
| return r.stdout |
There was a problem hiding this comment.
Add timeouts around the gh subprocesses.
Both gh pr view and gh pr diff can hang on network stalls or auth/environment issues. In that case the worker future never completes and the whole batch report blocks instead of returning a per-PR error.
Suggested fix
def _gh_json(url: str) -> dict[str, Any]:
- r = subprocess.run(
- ["gh", "pr", "view", url, "--json", GH_JSON_FIELDS],
- capture_output=True,
- text=True,
- check=False,
- )
+ try:
+ r = subprocess.run(
+ ["gh", "pr", "view", url, "--json", GH_JSON_FIELDS],
+ capture_output=True,
+ text=True,
+ check=False,
+ timeout=60,
+ )
+ except subprocess.TimeoutExpired:
+ return {"url": url, "error": "gh pr view timed out"}
if r.returncode != 0:
err = (r.stderr or r.stdout or "").strip()
return {"url": url, "error": err or f"gh exit {r.returncode}"}
return json.loads(r.stdout)
def _gh_diff(url: str) -> str:
- r = subprocess.run(
- ["gh", "pr", "diff", url],
- capture_output=True,
- text=True,
- check=False,
- )
+ try:
+ r = subprocess.run(
+ ["gh", "pr", "diff", url],
+ capture_output=True,
+ text=True,
+ check=False,
+ timeout=60,
+ )
+ except subprocess.TimeoutExpired:
+ return "(gh pr diff timed out)"
if r.returncode != 0:
return f"(gh pr diff failed: {(r.stderr or '').strip()})"
return r.stdout📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _gh_json(url: str) -> dict[str, Any]: | |
| r = subprocess.run( | |
| ["gh", "pr", "view", url, "--json", GH_JSON_FIELDS], | |
| capture_output=True, | |
| text=True, | |
| check=False, | |
| ) | |
| if r.returncode != 0: | |
| err = (r.stderr or r.stdout or "").strip() | |
| return {"url": url, "error": err or f"gh exit {r.returncode}"} | |
| return json.loads(r.stdout) | |
| def _gh_diff(url: str) -> str: | |
| r = subprocess.run( | |
| ["gh", "pr", "diff", url], | |
| capture_output=True, | |
| text=True, | |
| check=False, | |
| ) | |
| if r.returncode != 0: | |
| return f"(gh pr diff failed: {(r.stderr or '').strip()})" | |
| return r.stdout | |
| def _gh_json(url: str) -> dict[str, Any]: | |
| try: | |
| r = subprocess.run( | |
| ["gh", "pr", "view", url, "--json", GH_JSON_FIELDS], | |
| capture_output=True, | |
| text=True, | |
| check=False, | |
| timeout=60, | |
| ) | |
| except subprocess.TimeoutExpired: | |
| return {"url": url, "error": "gh pr view timed out"} | |
| if r.returncode != 0: | |
| err = (r.stderr or r.stdout or "").strip() | |
| return {"url": url, "error": err or f"gh exit {r.returncode}"} | |
| return json.loads(r.stdout) | |
| def _gh_diff(url: str) -> str: | |
| try: | |
| r = subprocess.run( | |
| ["gh", "pr", "diff", url], | |
| capture_output=True, | |
| text=True, | |
| check=False, | |
| timeout=60, | |
| ) | |
| except subprocess.TimeoutExpired: | |
| return "(gh pr diff timed out)" | |
| if r.returncode != 0: | |
| return f"(gh pr diff failed: {(r.stderr or '').strip()})" | |
| return r.stdout |
🧰 Tools
🪛 Ruff (0.15.10)
[error] 76-76: subprocess call: check for execution of untrusted input
(S603)
[error] 77-77: Starting a process with a partial executable path
(S607)
[error] 89-89: subprocess call: check for execution of untrusted input
(S603)
[error] 90-90: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/pr-batch-report/scripts/pr_batch_report.py` around lines 75 - 97, The
_gh_json and _gh_diff functions call subprocess.run without timeouts which can
hang; add a timeout argument (e.g., timeout=30) to both subprocess.run
invocations and catch subprocess.TimeoutExpired around each call: for _gh_json
return a dict like {"url": url, "error": "gh pr view timed out"} (or include
timeout seconds) and for _gh_diff return a string indicating the diff command
timed out, ensuring you keep the existing behavior for non-timeout failures and
still use r.stdout/r.stderr when available and preserve GH_JSON_FIELDS and the
function signatures.
| elif tn == "StatusContext": | ||
| ctx = c.get("context") or "status" | ||
| state = (c.get("state") or "").upper() | ||
| if state == "FAILURE": | ||
| failing.append(f"{ctx} ({state})") | ||
| elif state == "PENDING": | ||
| pending.append(f"{ctx} ({state})") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the possible values of GitHub GraphQL StatusState/StatusContext.stateinstatusCheckRollup, and what do ERRORandEXPECTED mean for PR check status?
💡 Result:
The possible values of GitHub GraphQL StatusState (used for StatusContext.state in statusCheckRollup) are ERROR, EXPECTED, FAILURE, PENDING, and SUCCESS. ERROR means "Status is errored." EXPECTED means "Status is expected." These states apply to legacy commit statuses (StatusContext objects) aggregated in statusCheckRollup for PRs. statusCheckRollup.contexts can be StatusContext or CheckRun; StatusContext.state uses StatusState, while CheckRun uses CheckState (e.g., FAILURE, NEUTRAL, PENDING, SUCCESS).
Citations:
- 1: https://docs.github.com/enterprise/2.12/developer/v4/enum/statusstate/
- 2: https://docs.github.com/enterprise/2.16/developer/v4/enum/statusstate/
- 3: https://docs.github.com/enterprise/2.15/developer/v4/object/statuscontext/
- 4:
gh pr statusGraphQL query requests moreStatusCheckRollupdata than required, causing timeouts in large repositories cli/cli#7421 - 5: Use new GQL fields that support CheckRun and StatusContext counts by state cli/cli#7462
StatusContext states ERROR and EXPECTED are currently misreported as passing.
GitHub's StatusState enum includes ERROR and EXPECTED values, but the current code only checks for FAILURE (line 120) and PENDING (line 122). When a status context has state ERROR or EXPECTED, it falls through to the passing category, under-reporting broken or incomplete checks.
Suggested fix
elif tn == "StatusContext":
ctx = c.get("context") or "status"
state = (c.get("state") or "").upper()
- if state == "FAILURE":
+ if state in {"FAILURE", "ERROR"}:
failing.append(f"{ctx} ({state})")
- elif state == "PENDING":
+ elif state in {"PENDING", "EXPECTED"}:
pending.append(f"{ctx} ({state})")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif tn == "StatusContext": | |
| ctx = c.get("context") or "status" | |
| state = (c.get("state") or "").upper() | |
| if state == "FAILURE": | |
| failing.append(f"{ctx} ({state})") | |
| elif state == "PENDING": | |
| pending.append(f"{ctx} ({state})") | |
| elif tn == "StatusContext": | |
| ctx = c.get("context") or "status" | |
| state = (c.get("state") or "").upper() | |
| if state in {"FAILURE", "ERROR"}: | |
| failing.append(f"{ctx} ({state})") | |
| elif state in {"PENDING", "EXPECTED"}: | |
| pending.append(f"{ctx} ({state})") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/pr-batch-report/scripts/pr_batch_report.py` around lines 117 - 123,
The StatusContext branch currently only flags "FAILURE" and "PENDING" but
ignores "ERROR" and "EXPECTED", causing them to be treated as passing; update
the conditional in the tn == "StatusContext" block (the code that sets ctx,
state and appends to failing or pending lists) so that state values "ERROR" and
"EXPECTED" are treated as non-passing (e.g., include them with "FAILURE" in the
failing.append branch) and keep "PENDING" in the pending.append branch; adjust
the checks on the state variable used in that block (ctx, state, failing,
pending) accordingly.
Description
This skill lets you quickly view in your terminal the diffs and summary of a list of PRs.
It's purely generated by Cursor.
Type of change
Testing steps
I tried it locally and this is the result:
Details
PR batch report
bots-automerge.ymlworkflow file - This workflow is superseded by the newbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of perso….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.yamlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/dependabot-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-reviews.yamlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlbots-auto-merge.yamlworkflow synced from processing-tools - The new workflow uses the obsint-processing GitHub app instead of personal a….github/workflows/bots-automerge.ymlCI notes
Unified diffs (
gh pr diff)https://github.com/RedHatInsights/ccx-notification-service/pull/1101 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/ccx-notification-writer/pull/803 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/data-pipeline/pull/59 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/insights-ccx-messaging/pull/708 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/insights-content-template-renderer/pull/280 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/insights-operator-gathering-conditions-service/pull/637 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/insights-operator-utils/pull/840 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/insights-results-aggregator/pull/2450 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/insights-results-aggregator-exporter/pull/580 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/insights-results-aggregator-mock/pull/535 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/insights-results-smart-proxy/pull/1704 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/obsint-mocks/pull/201 — Remove old bots auto-merge workflow
https://github.com/RedHatInsights/parquet-factory/pull/57 — Remove old bots auto-merge workflow