Add AI-powered PR review system with domain-specific personas#197
Merged
DiamondDagger590 merged 10 commits intoMar 4, 2026
Conversation
Four review personas (GUI/UX, Server Owner, Extensibility, Testing) for both Cursor (.cursor/rules/persona-*.mdc) and Claude Code (.claude/commands/review-*.md). A GitHub Actions workflow (pr-review.yml) automatically invokes the relevant personas on PRs targeting main/develop using file-change detection, calls the Anthropic API (Haiku by default), and posts a consolidated comment — only when concerns are found. CLAUDE.md maintenance table updated with persona sync reminders. https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
Four review personas (GUI/UX, Server Owner, Extensibility, Testing) for both Cursor (.cursor/rules/persona-*.mdc) and Claude Code (.claude/commands/review-*.md). A GitHub Actions workflow (pr-review.yml) automatically invokes the relevant personas on PRs targeting main/develop using file-change detection, calls the Anthropic API (Haiku by default), and posts a consolidated comment — only when concerns are found. Persona details: - GUI/UX: command-driven navigation, localization manager (not direct MiniMessage), nuanced onClick(false) handling, visual+sound feedback - Server Owner: file readability/cross-file complexity checks, reload safety, migration guard, permission node conventions - Extensibility: cancellable events, NamespacedKey stability, third-party extension opportunity check, backward-compat contracts - Testing: TimeProvider for time-dependent logic, McRPGBaseTest nuance, givenWhenThen naming + @DisplayName requirement CLAUDE.md maintenance table updated with persona sync reminders. https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
The secrets context is not available in step-level if: expressions in GitHub Actions. Replace the invalid secrets.ANTHROPIC_API_KEY != '' condition with a dedicated check-key step that passes the secret via env: (which is valid) and writes an output flag used by downstream steps. https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
Configures CodeRabbit to use the assertive review profile, disables the poem, and adds Java path instructions that accept intentional Bukkit/Paper patterns (async scheduling, event handlers, localization manager usage) without false-positive flagging. Explicitly defers GUI, config, extensibility, and test review to the Anthropic persona CI workflow. https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds multi-persona reviewer documentation, a CodeRabbit config, an AI PR review Python script, and two GitHub Actions workflows to classify changed domains, produce diff artifacts, run persona-driven Anthropic reviews, and optionally post consolidated PR comments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Detect as "GH Actions: detect-changes"
participant Prep as "GH Actions: prepare-review"
participant Storage as "Artifact Storage"
participant Runner as "GH Actions: pr-review-run"
participant Script as "pr-review.py"
participant Anthropic as "Anthropic Messages API"
participant GHPR as "GitHub PR (comment)"
Detect->>Prep: classify changed files, set domain flags
Prep->>Storage: upload pr.diff + review-meta.json
Runner->>Storage: download pr.diff + review-meta.json
Runner->>Script: run script with env (NEEDS_*, REVIEW_MODEL, API key, DIFF_FILE)
Script->>Anthropic: send persona prompts + sanitized diff
Anthropic-->>Script: persona review responses
Script-->>Storage: write review_comment.txt
Runner->>Storage: read review_comment.txt
Runner->>GHPR: post review comment (if not suppressed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/review-extensibility.md:
- Line 1: Add a top-level H1 heading to the start of the markdown to satisfy
MD041 (`first-line-heading`)—insert a single line like "# Extensibility Persona"
(or another descriptive title) at the very top of the file so the
first-line-heading lint rule is satisfied and the document begins with an
explicit top-level heading.
In @.claude/commands/review-server-owner.md:
- Line 1: The file .claude/commands/review-server-owner.md is missing a
top-level heading which triggers MD041; add a single H1 line at the very start
of the file (for example "# Server Owner Review" or "# Review Server Owner") so
the first line is a top-level markdown heading and the linter stops flagging
MD041; ensure the heading is the first non-empty line and leaves the existing
content unchanged.
In @.claude/commands/review-testing.md:
- Line 1: Add a top-level Markdown heading at the very start of the file to
satisfy MD041; prepend a H1 line (e.g., "# Testing Auditor Persona" or similar)
immediately before the existing first-line text "Adopt the Testing Auditor
Persona. You are a test engineer reviewing..." so the file begins with a heading
and the rest of the content remains unchanged.
In @.coderabbit.yaml:
- Around line 24-25: The YAML contains overly broad "Do not flag" suppression
text (e.g., the directive containing "Async scheduling via BukkitRunnable,
Bukkit.getScheduler(), or CompletableFuture chains is intentional. Do not flag
as thread-safety issues.") that may hide real defects; replace this blanket
suppression with a scoped exception: remove the free‑form "Do not flag" line and
instead add targeted rules that whitelist specific check IDs or patterns and/or
explicit symbols (e.g., class/method names like BukkitRunnable,
Bukkit.getScheduler, CompletableFuture) only where intentional, include a short
rationale comment for each exception, and apply the same scoped approach to the
other broad directive referenced around lines 38-40 so only known safe usages
are excluded rather than suppressing whole categories.
In @.github/scripts/pr-review.py:
- Around line 114-116: The retry logic in the exception handling around e.code
currently only retries on 429 and 529, causing transient 5xx upstream errors to
be treated as permanent; update the condition in the exception handling block
(the branch that checks e.code) to also retry for common transient HTTP status
codes such as 500, 502, 503, and 504 (or generally any 5xx) so that the function
that checks PR reviews will continue retrying on these transient upstream
failures instead of returning None; locate the check referring to e.code and
expand the tuple or condition to include 500, 502, 503, 504 (or use a 500 <=
e.code < 600 range) to implement the fix.
- Line 153: The parsing of MAX_DIFF_CHARS should be made robust: wrap the
int(os.environ.get("MAX_DIFF_CHARS", DEFAULT_MAX_DIFF_CHARS)) logic used to set
max_diff_chars in a try/except (catch ValueError/TypeError) and validate the
resulting value is a positive integer; on failure, fall back to
DEFAULT_MAX_DIFF_CHARS and emit a warning (e.g. via print or a logger) that
MAX_DIFF_CHARS was malformed, referencing the symbols max_diff_chars and
DEFAULT_MAX_DIFF_CHARS so you replace the direct int() call with safe parsing
and validation.
In @.github/workflows/pr-review.yml:
- Line 3: The workflow YAML has yamllint violations: the bare "on:" key triggers
a truthy warning and several mapping keys have incorrect colon spacing; change
the top-level "on:" to an explicit empty mapping (e.g., replace "on:" with "on:
{}" or list actual events) to satisfy the truthy rule, and fix all colon spacing
issues by ensuring no space before colons and exactly one space after colons for
the mappings referenced (the top-level "on" key and the mappings in the blocks
around the ranges that contained the errors — adjust the mappings in those
sections so keys use the correct "key: value" spacing).
- Around line 66-68: The workflow's job-level permissions block only specifies
pull-requests: write which causes all other permissions to default to none and
breaks actions/checkout; update the permissions block to include contents: read
alongside pull-requests: write so actions/checkout can read the repository
(i.e., add "contents: read" in the same permissions mapping that currently
contains "pull-requests: write").
- Around line 69-72: The PR workflow is exposing ANTHROPIC_API_KEY to untrusted
PR code (the step that runs .github/scripts/pr-review.py after uses:
actions/checkout@v4), so remove the secret from the pull_request workflow and
either (1) switch the secret usage to OIDC/short-lived token authentication for
the external call, or (2) move the secret-dependent step into a separate
workflow triggered by workflow_run that does not perform actions/checkout (so PR
head code is not executed); update the pr-review.yml steps to stop injecting
ANTHROPIC_API_KEY into steps that run .github/scripts/pr-review.py and instead
call the secure token or the isolated workflow.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
.claude/commands/review-extensibility.md.claude/commands/review-gui-ux.md.claude/commands/review-server-owner.md.claude/commands/review-testing.md.coderabbit.yaml.cursor/rules/persona-extensibility.mdc.cursor/rules/persona-gui-ux.mdc.cursor/rules/persona-server-owner.mdc.cursor/rules/persona-testing.mdc.github/scripts/pr-review.py.github/workflows/pr-review.ymlCLAUDE.md
| @@ -0,0 +1,118 @@ | |||
| name: AI PR Review | |||
|
|
|||
| on: | |||
There was a problem hiding this comment.
Fix current yamllint violations in workflow YAML.
Line 3 (on) triggers truthy warning, and Lines 16-19 and 100-105 have colons spacing errors.
Proposed fix
-on:
+'on':
@@
- needs_gui: ${{ steps.detect.outputs.gui }}
+ needs_gui: ${{ steps.detect.outputs.gui }}
@@
- needs_api: ${{ steps.detect.outputs.api }}
- needs_test: ${{ steps.detect.outputs.test }}
+ needs_api: ${{ steps.detect.outputs.api }}
+ needs_test: ${{ steps.detect.outputs.test }}
@@
- REVIEW_MODEL: ${{ vars.REVIEW_MODEL || 'claude-haiku-4-5-20251001' }}
- NEEDS_GUI: ${{ needs.detect-changes.outputs.needs_gui }}
- NEEDS_CONFIG: ${{ needs.detect-changes.outputs.needs_config }}
- NEEDS_API: ${{ needs.detect-changes.outputs.needs_api }}
- NEEDS_TEST: ${{ needs.detect-changes.outputs.needs_test }}
- DIFF_FILE: /tmp/pr.diff
+ REVIEW_MODEL: ${{ vars.REVIEW_MODEL || 'claude-haiku-4-5-20251001' }}
+ NEEDS_GUI: ${{ needs.detect-changes.outputs.needs_gui }}
+ NEEDS_CONFIG: ${{ needs.detect-changes.outputs.needs_config }}
+ NEEDS_API: ${{ needs.detect-changes.outputs.needs_api }}
+ NEEDS_TEST: ${{ needs.detect-changes.outputs.needs_test }}
+ DIFF_FILE: /tmp/pr.diffAlso applies to: 16-19, 100-105
🧰 Tools
🪛 YAMLlint (1.38.0)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-review.yml at line 3, The workflow YAML has yamllint
violations: the bare "on:" key triggers a truthy warning and several mapping
keys have incorrect colon spacing; change the top-level "on:" to an explicit
empty mapping (e.g., replace "on:" with "on: {}" or list actual events) to
satisfy the truthy rule, and fix all colon spacing issues by ensuring no space
before colons and exactly one space after colons for the mappings referenced
(the top-level "on" key and the mappings in the blocks around the ranges that
contained the errors — adjust the mappings in those sections so keys use the
correct "key: value" spacing).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (9)
.claude/commands/review-extensibility.md (1)
1-1:⚠️ Potential issue | 🟡 MinorInsert a top-level heading before the opening sentence.
Line 1 currently triggers MD041 (
first-line-heading).Suggested fix
+# Extensibility Review Command + Adopt the Third-Party Extensibility Persona. You are a developer building an addon plugin that hooks into McRPG. Evaluate this diff from the perspective of: can you safely hook in? Will your existing addon break? Is the API surface documented well enough to extend without reading implementation code?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/review-extensibility.md at line 1, Add a top-level Markdown heading before the opening sentence "Adopt the Third-Party Extensibility Persona. You are a developer building an addon plugin that hooks into McRPG..." to satisfy MD041; insert a single H1 (e.g., "# Third-Party Extensibility Persona" or similar) as the very first line so the document begins with a heading above that sentence..claude/commands/review-server-owner.md (1)
1-1:⚠️ Potential issue | 🟡 MinorAdd a top-level H1 at file start.
Line 1 violates markdown first-line-heading (MD041), which will keep docs lint noisy.
Suggested fix
+# Server Owner Review Command + Adopt the Server Owner Review Persona. You are a server administrator who has never read Java source — you evaluate changes by reading config YAMLs, `plugin.yml`, and upgrade notes. You care about your server not breaking on update, players not losing data, and configs being navigable without a manual.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/review-server-owner.md at line 1, This markdown file is missing a top-level H1 which triggers MD041; add a single top-level heading as the very first line of the file (e.g., a concise title reflecting the document purpose) so the file begins with "# Your Title", followed by a blank line and then the existing content; ensure the heading is unique and descriptive so linters stop flagging MD041 and docs remain navigable..claude/commands/review-testing.md (1)
1-1:⚠️ Potential issue | 🟡 MinorAdd a top-level H1 to satisfy MD041.
Line 1 begins with plain text, so markdown lint will keep reporting first-line-heading.
Suggested fix
+# Testing Review Command + Adopt the Testing Auditor Persona. You are a test engineer reviewing whether this change is adequately tested and whether tests are structured correctly. Flag coverage gaps and structural problems — not style preferences.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/review-testing.md at line 1, Add a top-level H1 as the first line of the document to satisfy MD041 so the current plain text at the top becomes a subheading/paragraph; update .claude/commands/review-testing.md by inserting a descriptive H1 (for example "Testing Auditor Persona" or similar) as the very first line and move the existing first-line text below it so markdown-lint stops flagging the missing first-line heading..github/workflows/pr-review.yml (3)
66-67:⚠️ Potential issue | 🟠 MajorAdd
contents: readwhen permissions are explicitly scoped.Line [66]–Line [67] set job permissions but omit
contents: read; with explicit scopes, unspecified permissions becomenone, which can breakactions/checkout.Suggested fix
permissions: + contents: read pull-requests: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-review.yml around lines 66 - 67, The workflow explicitly sets job permissions (the permissions: block with pull-requests: write) but omits contents: read which causes unspecified permissions to default to none and can break actions/checkout; update the permissions block to include contents: read alongside pull-requests: write so actions/checkout can read the repository contents and other steps depending on repo content succeed.
69-72:⚠️ Potential issue | 🔴 Critical
ANTHROPIC_API_KEYis still exposed to PR-head code execution.Line [69]–Line [72] checks out PR code, and Line [96]–Line [107] runs repository script with
ANTHROPIC_API_KEY. For same-repo PRs, this remains a credential-exfiltration path.Use a split-workflow design: untrusted PR workflow produces metadata/diff only, and a trusted follow-up workflow (default branch context) performs secret-dependent API calls without executing PR-head code.
Also applies to: 96-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-review.yml around lines 69 - 72, The workflow currently checks out PR-head code with actions/checkout@v4 and then runs repository scripts using the ANTHROPIC_API_KEY secret, exposing secrets to untrusted PR code; to fix, split into two workflows: (1) an untrusted PR workflow that does NOT checkout or execute PR code (remove actions/checkout@v4 or set fetch-depth to 0 but avoid running repo scripts) and only produces metadata/diffs/artifacts, and (2) a trusted workflow triggered by workflow_run (or a manual/merge-to-default trigger) running in default-branch context that checks out the trusted default branch and executes the script with ANTHROPIC_API_KEY set; ensure the job that uses ANTHROPIC_API_KEY runs only in the trusted workflow (no PR-head checkout) and reference the existing actions/checkout@v4 usage and the job that sets env ANTHROPIC_API_KEY to relocate it into the trusted workflow.
3-3:⚠️ Potential issue | 🟡 MinorWorkflow still contains current yamllint violations (
truthy,colons).Line [3] and spacing at Line [16], Line [18]–Line [19], and Line [100]–Line [105] still violate the reported lint rules.
Suggested fix
-on: +'on': @@ - needs_gui: ${{ steps.detect.outputs.gui }} + needs_gui: ${{ steps.detect.outputs.gui }} @@ - needs_api: ${{ steps.detect.outputs.api }} - needs_test: ${{ steps.detect.outputs.test }} + needs_api: ${{ steps.detect.outputs.api }} + needs_test: ${{ steps.detect.outputs.test }} @@ - REVIEW_MODEL: ${{ vars.REVIEW_MODEL || 'claude-haiku-4-5-20251001' }} - NEEDS_GUI: ${{ needs.detect-changes.outputs.needs_gui }} - NEEDS_CONFIG: ${{ needs.detect-changes.outputs.needs_config }} - NEEDS_API: ${{ needs.detect-changes.outputs.needs_api }} - NEEDS_TEST: ${{ needs.detect-changes.outputs.needs_test }} - DIFF_FILE: /tmp/pr.diff + REVIEW_MODEL: ${{ vars.REVIEW_MODEL || 'claude-haiku-4-5-20251001' }} + NEEDS_GUI: ${{ needs.detect-changes.outputs.needs_gui }} + NEEDS_CONFIG: ${{ needs.detect-changes.outputs.needs_config }} + NEEDS_API: ${{ needs.detect-changes.outputs.needs_api }} + NEEDS_TEST: ${{ needs.detect-changes.outputs.needs_test }} + DIFF_FILE: /tmp/pr.diffAlso applies to: 16-16, 18-19, 100-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-review.yml at line 3, Fix the yamllint "truthy" and "colons" violations by replacing any bare truthy tokens with explicit booleans or quoted strings and ensuring a single space after each colon in mappings; specifically review the top-level on key and the mappings around the regions reported (keys at or near "on", and the blocks referenced around lines 16, 18–19, 100–105) and change bare values like yes/no/on/off to true/false (or quote them if they must be a string) and add proper spacing after ":" in keys/values so all mappings conform to the colon rule..coderabbit.yaml (1)
24-41:⚠️ Potential issue | 🟠 MajorNarrow blanket suppression rules to avoid review blind spots.
Line [24]–Line [41] still use broad “Do not flag / Do NOT review” directives. This can hide legitimate high-severity defects when persona runs are skipped (e.g., missing secret, workflow failure). Keep these as de-prioritization rules, but still allow clear critical/major issues.
Suggested adjustment
- Accept the following patterns without flagging: + Treat the following patterns as common in this codebase; do not flag by default: @@ - CompletableFuture chains is intentional. Do not flag as thread-safety issues. + CompletableFuture chains is intentional. Still flag clear shared-mutable-state + hazards, Bukkit main-thread-only API misuse, or ordering/visibility race risks. @@ - Do NOT review for: GUI slot design, server config readability, API extension - points, or test structure. Those domains are covered by dedicated Anthropic - AI persona reviewers that run on every PR. + De-prioritize: GUI slot design, server config readability, API extension points, + and test structure when corresponding persona output is present. Still flag + clear high-severity defects if detected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.coderabbit.yaml around lines 24 - 41, Update the broad "Do not flag / Do NOT review" directives in the suppression block so they become de-prioritization rules that still allow detection of critical/major issues: replace absolute phrases like "Do not flag" and "Do NOT review" with conditional wording (e.g., "Avoid flagging by default; still surface Critical/Major issues such as secrets, build/workflow failures, or security vulnerabilities") and add explicit exceptions for severity classes (Critical/Major) in the same block that lists patterns (the Async scheduling, Listener/@EventHandler, InventoryClickEvent, MiniMessage, Registry/builder/abstract, generic type parameters entries) so automated reviewers will skip low-priority findings but will still report high-severity problems..github/scripts/pr-review.py (2)
114-116:⚠️ Potential issue | 🟠 MajorExpand retryable HTTP status handling for transient failures.
Line [114] currently retries only
429/529; transient upstream errors like500/502/503/504are still treated as permanent and can suppress reviews unnecessarily.Suggested fix
- if e.code in (429, 529): # rate limit / overload — retry + if e.code in (408, 425, 429, 500, 502, 503, 504, 529): # transient continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/pr-review.py around lines 114 - 116, The exception handling currently only retries on e.code in (429, 529) and treats other HTTP errors as permanent; update the check in the error-handling block that examines e.code (the branch that currently returns None for "other HTTP errors") to also treat transient 5xx server errors as retryable — e.g., include 500, 502, 503, 504 (or a 500–599 range) in the retry condition so that temporary upstream failures are retried instead of returning None.
153-153:⚠️ Potential issue | 🟠 MajorHarden
MAX_DIFF_CHARSparsing to prevent hard-fail on bad env values.Line [153] can raise
ValueError(or accept non-positive values) and abort the workflow step.Suggested fix
- max_diff_chars = int(os.environ.get("MAX_DIFF_CHARS", DEFAULT_MAX_DIFF_CHARS)) + raw_max_diff_chars = os.environ.get("MAX_DIFF_CHARS", str(DEFAULT_MAX_DIFF_CHARS)) + try: + max_diff_chars = int(raw_max_diff_chars) + if max_diff_chars <= 0: + raise ValueError + except (TypeError, ValueError): + print( + f"Invalid MAX_DIFF_CHARS={raw_max_diff_chars!r}; using {DEFAULT_MAX_DIFF_CHARS}.", + file=sys.stderr, + ) + max_diff_chars = DEFAULT_MAX_DIFF_CHARS
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/pr-review.py:
- Around line 109-110: The current code directly returns
data["content"][0]["text"] which raises KeyError/IndexError when "content" is
missing, empty, or lacks "text"; update the parsing in the part that reads and
returns the Anthropic response (where data is assigned from
json.loads(resp.read().decode("utf-8"))) to defensively validate the schema: use
data.get("content") and verify it's a non-empty list, then check the first item
is a dict with a "text" key; handle JSON decode errors and any missing-field
cases by logging a clear message (or returning a safe fallback like an empty
string) and raising or returning a controlled error instead of letting
KeyError/IndexError propagate. Ensure these checks happen before the return so
callers (the review job) won’t crash on unexpected API responses.
---
Duplicate comments:
In @.claude/commands/review-extensibility.md:
- Line 1: Add a top-level Markdown heading before the opening sentence "Adopt
the Third-Party Extensibility Persona. You are a developer building an addon
plugin that hooks into McRPG..." to satisfy MD041; insert a single H1 (e.g., "#
Third-Party Extensibility Persona" or similar) as the very first line so the
document begins with a heading above that sentence.
In @.claude/commands/review-server-owner.md:
- Line 1: This markdown file is missing a top-level H1 which triggers MD041; add
a single top-level heading as the very first line of the file (e.g., a concise
title reflecting the document purpose) so the file begins with "# Your Title",
followed by a blank line and then the existing content; ensure the heading is
unique and descriptive so linters stop flagging MD041 and docs remain navigable.
In @.claude/commands/review-testing.md:
- Line 1: Add a top-level H1 as the first line of the document to satisfy MD041
so the current plain text at the top becomes a subheading/paragraph; update
.claude/commands/review-testing.md by inserting a descriptive H1 (for example
"Testing Auditor Persona" or similar) as the very first line and move the
existing first-line text below it so markdown-lint stops flagging the missing
first-line heading.
In @.coderabbit.yaml:
- Around line 24-41: Update the broad "Do not flag / Do NOT review" directives
in the suppression block so they become de-prioritization rules that still allow
detection of critical/major issues: replace absolute phrases like "Do not flag"
and "Do NOT review" with conditional wording (e.g., "Avoid flagging by default;
still surface Critical/Major issues such as secrets, build/workflow failures, or
security vulnerabilities") and add explicit exceptions for severity classes
(Critical/Major) in the same block that lists patterns (the Async scheduling,
Listener/@EventHandler, InventoryClickEvent, MiniMessage,
Registry/builder/abstract, generic type parameters entries) so automated
reviewers will skip low-priority findings but will still report high-severity
problems.
In @.github/scripts/pr-review.py:
- Around line 114-116: The exception handling currently only retries on e.code
in (429, 529) and treats other HTTP errors as permanent; update the check in the
error-handling block that examines e.code (the branch that currently returns
None for "other HTTP errors") to also treat transient 5xx server errors as
retryable — e.g., include 500, 502, 503, 504 (or a 500–599 range) in the retry
condition so that temporary upstream failures are retried instead of returning
None.
In @.github/workflows/pr-review.yml:
- Around line 66-67: The workflow explicitly sets job permissions (the
permissions: block with pull-requests: write) but omits contents: read which
causes unspecified permissions to default to none and can break
actions/checkout; update the permissions block to include contents: read
alongside pull-requests: write so actions/checkout can read the repository
contents and other steps depending on repo content succeed.
- Around line 69-72: The workflow currently checks out PR-head code with
actions/checkout@v4 and then runs repository scripts using the ANTHROPIC_API_KEY
secret, exposing secrets to untrusted PR code; to fix, split into two workflows:
(1) an untrusted PR workflow that does NOT checkout or execute PR code (remove
actions/checkout@v4 or set fetch-depth to 0 but avoid running repo scripts) and
only produces metadata/diffs/artifacts, and (2) a trusted workflow triggered by
workflow_run (or a manual/merge-to-default trigger) running in default-branch
context that checks out the trusted default branch and executes the script with
ANTHROPIC_API_KEY set; ensure the job that uses ANTHROPIC_API_KEY runs only in
the trusted workflow (no PR-head checkout) and reference the existing
actions/checkout@v4 usage and the job that sets env ANTHROPIC_API_KEY to
relocate it into the trusted workflow.
- Line 3: Fix the yamllint "truthy" and "colons" violations by replacing any
bare truthy tokens with explicit booleans or quoted strings and ensuring a
single space after each colon in mappings; specifically review the top-level on
key and the mappings around the regions reported (keys at or near "on", and the
blocks referenced around lines 16, 18–19, 100–105) and change bare values like
yes/no/on/off to true/false (or quote them if they must be a string) and add
proper spacing after ":" in keys/values so all mappings conform to the colon
rule.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
.claude/commands/review-extensibility.md.claude/commands/review-gui-ux.md.claude/commands/review-server-owner.md.claude/commands/review-testing.md.coderabbit.yaml.cursor/rules/persona-extensibility.mdc.cursor/rules/persona-gui-ux.mdc.cursor/rules/persona-server-owner.mdc.cursor/rules/persona-testing.mdc.github/scripts/pr-review.py.github/workflows/pr-review.ymlCLAUDE.md
- Add MD041 H1 headings to review-extensibility.md, review-server-owner.md, review-testing.md - Scope .coderabbit.yaml suppression directives from blanket "do not flag" to targeted per-pattern exceptions with rationale; soften "Do NOT review for" to a deprioritize directive with fallback - pr-review.py: add 5xx retry (500/502/503/504) alongside existing 429/529 - pr-review.py: add sanitize_diff() to redact secrets before sending to API - pr-review.py: guard API response parsing against missing/wrong-typed keys - pr-review.py: tighten is_no_findings to require exact standalone sentence match - pr-review.py: wrap MAX_DIFF_CHARS int() in try/except with safe fallback - pr-review.yml: fix yamllint truthy warning by quoting "on:" key; fix colon spacing; add contents: read permission; restructure to detect+prepare only (no secrets) — uploads diff + metadata as artifact - pr-review-run.yml: new workflow_run-triggered workflow that downloads the artifact and runs the AI review without ever checking out PR head code, isolating ANTHROPIC_API_KEY from untrusted PR changes https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
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 @.github/scripts/pr-review.py:
- Around line 69-78: The current _SENSITIVE_PATTERNS only matches unquoted
key[:=]value forms (see _SENSITIVE_PATTERNS) and thus can miss
JSON/YAML/camelCase keys like "apiKey": "secret"; add additional regex entries
that match quoted and camelCase key names (e.g., "apiKey", 'apiKey', apiKey,
api_key, api-key, privateKey, token, secret, password) followed by optional
whitespace, a JSON/YAML separator (":" or "="), optional quotes around the
value, and capture the secret to replace with a redaction token; ensure these
new patterns are inserted before the broad long-base64/hex pattern so explicit
key-based redactions run first and reuse the same replacement tokens (e.g.,
[REDACTED_API_KEY], [REDACTED_TOKEN], [REDACTED_PASSWORD],
[REDACTED_PRIVATE_KEY]) as used by the existing entries.
- Around line 130-138: The current logic validates the Anthropic API response
but only returns data["content"][0]["text"], dropping other text blocks; modify
the return path to iterate over data["content"] and concatenate all items where
each item is a dict and item.get("text") is a str (join with "" or "\n" as
appropriate) so the function returns the full assembled string instead of just
the first block; keep the same safety checks (isinstance(data, dict),
isinstance(data.get("content"), list)) and ensure empty or invalid content still
returns None.
In @.github/workflows/pr-review-run.yml:
- Around line 16-18: The workflow is missing the actions: read permission
required for the actions/download-artifact@v4 step that uses run-id and
github-token; update the permissions block (where contents: read and
pull-requests: write are set) to include actions: read so the token can access
artifacts from other runs, ensuring the download step that uses github-token:
${{ secrets.GITHUB_TOKEN }} succeeds when calling actions/download-artifact@v4.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.claude/commands/review-extensibility.md.claude/commands/review-server-owner.md.claude/commands/review-testing.md.coderabbit.yaml.github/scripts/pr-review.py.github/workflows/pr-review-run.yml.github/workflows/pr-review.yml
- _SENSITIVE_PATTERNS: add five quoted-key variants ("apiKey": "val",
'api-key': 'val', etc.) that run before the broad base64/hex catch-all;
existing bare-key patterns remain for unquoted forms so both sets cover
all key spellings (api_key, api-key, apiKey, camelCase, etc.)
- call_api: replace single data["content"][0]["text"] return with an
iteration over all content blocks, joining every item where item is a
dict and item["text"] is a str, so multi-block responses are not truncated
- pr-review-run.yml: add actions: read permission required by
actions/download-artifact@v4 when fetching artifacts via run-id + github-token
https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/pr-review.py:
- Around line 84-86: The broad regex tuple
(re.compile(r"(?<![A-Za-z0-9])[A-Za-z0-9+/=_-]{32,}(?![A-Za-z0-9])"),
"[REDACTED_TOKEN]") can false-positive redact hex-only values like commit
SHAs/UUIDs; tighten it by requiring evidence of non-hex/base64 padding or a
token-like prefix/suffix: replace the compiled pattern with one that either (a)
requires at least one base64-specific char (e.g. +,/ or =) or a non-hex
character (use a positive lookahead like (?=.*[+/=]) or (?=.*[^0-9a-fA-F])), or
(b) anchors to common secret labels (e.g. lookbehind for
"api_key|secret|token"). Update the tuple where re.compile(...) appears to use
the new pattern so legitimate hex IDs are not redacted while preserving token
redaction.
- Around line 130-133: Replace the list concatenation in the enumerate call with
unpacking to follow idiomatic style: change the iterable from "[0] +
RETRY_DELAYS" to use unpacking like "[0, *RETRY_DELAYS]" where RETRY_DELAYS is
referenced in the for loop (the enumerate([...]) expression) so the loop
behavior is unchanged but uses the more idiomatic form.
- Around line 235-237: The loop over PERSONAS uses an unused variable name key;
to satisfy static analysis (B007) and clarify intent, rename the unused loop
variable to _key (for example: for _key, persona in PERSONAS.items():) or
iterate over values directly (for persona in PERSONAS.values():) and keep the
existing body that checks os.environ.get(persona["env"], "false").lower() !=
"true": continue; update any references accordingly (use PERSONAS.items() or
PERSONAS.values() as chosen) to remove the unused variable warning.
- Around line 96-101: The read_file function currently calls open(path, "r",
encoding="utf-8") with an unnecessary "r" mode; change the open invocation in
read_file to open(path, encoding="utf-8") so it relies on the default read mode,
keeping the try/except and return behavior unchanged.
- Around line 186-193: The current heuristic builds "others" and treats any
sentence starting with the conjunction regex
re.match(r"^(and|but|however|also)\b", ...) as non-substantive, which can
incorrectly ignore sentences like "However, consider X." Update the check in the
any(...) so it first strips a leading conjunction and punctuation (e.g., remove
r"^(and|but|however|also)[,;:\s-]*" case-insensitively) from each sentence in
"others" and then test whether the remaining text contains substantive content
(e.g., non-whitespace/alphanumeric characters); replace the existing
re.match-based filter with this strip-then-check logic using the same "others"
variable and regex symbol to locate the code.
In @.github/workflows/pr-review-run.yml:
- Around line 34-42: Replace the unclosed file handle used in
json.load(open("review-meta.json")) by opening "review-meta.json" with a context
manager; specifically, change the inline Python so you use with
open("review-meta.json") as f: d = json.load(f) (keep the rest of the script
that strips whitespace and writes to out using the existing with open(out, "a")
as f block).
- Around line 76-82: The workflow currently injects the variable BODY directly
into the gh pr comment command which can mis-handle or execute special shell
characters; change to a file-based invocation: write the comment to
review_comment.txt (or a temp file) and pass that file with gh pr comment
--body-file (or equivalent) instead of using the "$BODY" variable, ensuring you
still preserve the existing suppression check that reads review_comment.txt into
BODY and then call gh pr comment ${{ steps.meta.outputs.pr_number }} --body-file
review_comment.txt (or the temp file) to avoid shell quoting/injection issues.
pr-review.py: - Tighten broad base64 regex to require at least one +/= char, preventing false-positive redaction of commit SHAs and hex UUIDs (line 85) - Use idiomatic list unpacking [0, *RETRY_DELAYS] instead of concatenation (line 130) - Replace unused loop variable `key` with PERSONAS.values() iteration (line 235) - Remove redundant explicit "r" mode from open() in read_file (line 98) - Fix conjunction-stripping heuristic: strip leading conjunction prefix then test remaining text for substantive content, so "However, consider X." is correctly treated as substantive rather than ignored (line 188) pr-review-run.yml: - Wrap json.load(open(...)) in a context manager to ensure file is closed (line 36) - Replace --body "$BODY" with --body-file to avoid shell injection/quoting issues with large or special-character comment content (line 77) https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/pr-review.py:
- Around line 84-85: The current redaction rule that matches long base64-like
tokens (the tuple with
re.compile(r"(?<![A-Za-z0-9])(?=[A-Za-z0-9+/=_-]*[+/=])[A-Za-z0-9+/=_-]{32,}(?![A-Za-z0-9])"),
"[REDACTED_TOKEN]") misses base64url-only tokens and common provider prefixes;
update the redaction rules to (1) add a JWT pattern (three dot-separated
base64url segments) to catch JWTs, (2) add a base64url-only fallback that
permits - and _ and does not require + or /, matching length >=32, and (3) add
explicit provider/token-prefix patterns for common prefixes like ghp_, gho_,
sk-, pk_live_, etc.; ensure these new patterns are added alongside the existing
tuple in the redaction rules list and ordered so specific prefix patterns and
JWTs are checked before generic long-token fallbacks.
In @.github/workflows/pr-review-run.yml:
- Around line 35-42: Validate each output key from review-meta.json (the keys in
d) against a safe pattern (e.g., only alphanumerics, underscores, hyphens) and
skip or normalize any invalid keys, then write values to GITHUB_OUTPUT (variable
out) using the GitHub multiline delimiter format when the value contains
newlines or control characters: emit a "key<<EOF" line, the raw value, then a
terminating "EOF" line to prevent injection of new output keys; for simple
single-line safe values you may continue to write "key=value". Apply this logic
where the code iterates "for k, v in d.items()" and writes to the file opened
from out.
pr-review.py: - Add explicit provider-prefix patterns (ghp_/gho_/ghs_/ghr_/github_pat_, sk-, pk_live_) checked before generic fallbacks - Add JWT pattern anchored on the eyJ header common to all JWTs - Add a base64url-alphabet fallback (A-Za-z0-9_-, 32+ chars) to catch URL-safe tokens that lack the + / = chars required by the previous rule All new patterns are ordered most-specific → most-general pr-review-run.yml: - Validate each key from review-meta.json against ^[A-Za-z0-9_-]+$; skip any key that fails to prevent injection into GITHUB_OUTPUT - Write multiline values using the key<<EOF...EOF delimiter format so newlines and control characters in values cannot inject new output keys https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
DiamondDagger590
added a commit
that referenced
this pull request
Mar 12, 2026
* Add specialized review persona contexts and automated CI PR review Four review personas (GUI/UX, Server Owner, Extensibility, Testing) for both Cursor (.cursor/rules/persona-*.mdc) and Claude Code (.claude/commands/review-*.md). A GitHub Actions workflow (pr-review.yml) automatically invokes the relevant personas on PRs targeting main/develop using file-change detection, calls the Anthropic API (Haiku by default), and posts a consolidated comment — only when concerns are found. CLAUDE.md maintenance table updated with persona sync reminders. https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd * Add specialized review persona contexts and automated CI PR review Four review personas (GUI/UX, Server Owner, Extensibility, Testing) for both Cursor (.cursor/rules/persona-*.mdc) and Claude Code (.claude/commands/review-*.md). A GitHub Actions workflow (pr-review.yml) automatically invokes the relevant personas on PRs targeting main/develop using file-change detection, calls the Anthropic API (Haiku by default), and posts a consolidated comment — only when concerns are found. Persona details: - GUI/UX: command-driven navigation, localization manager (not direct MiniMessage), nuanced onClick(false) handling, visual+sound feedback - Server Owner: file readability/cross-file complexity checks, reload safety, migration guard, permission node conventions - Extensibility: cancellable events, NamespacedKey stability, third-party extension opportunity check, backward-compat contracts - Testing: TimeProvider for time-dependent logic, McRPGBaseTest nuance, givenWhenThen naming + @DisplayName requirement CLAUDE.md maintenance table updated with persona sync reminders. https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd * fix: use step output to gate secrets-dependent steps in pr-review.yml The secrets context is not available in step-level if: expressions in GitHub Actions. Replace the invalid secrets.ANTHROPIC_API_KEY != '' condition with a dedicated check-key step that passes the secret via env: (which is valid) and writes an output flag used by downstream steps. https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd * chore: add CodeRabbit config with assertive profile and domain hints Configures CodeRabbit to use the assertive review profile, disables the poem, and adds Java path instructions that accept intentional Bukkit/Paper patterns (async scheduling, event handlers, localization manager usage) without false-positive flagging. Explicitly defers GUI, config, extensibility, and test review to the Anthropic persona CI workflow. https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd * fix: remove unrecognized 'version' field from .coderabbit.yaml * fix: update default branches to master/develop/recode https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd * fix: address CodeRabbit review feedback for McRPG - Add MD041 H1 headings to review-extensibility.md, review-server-owner.md, review-testing.md - Scope .coderabbit.yaml suppression directives from blanket "do not flag" to targeted per-pattern exceptions with rationale; soften "Do NOT review for" to a deprioritize directive with fallback - pr-review.py: add 5xx retry (500/502/503/504) alongside existing 429/529 - pr-review.py: add sanitize_diff() to redact secrets before sending to API - pr-review.py: guard API response parsing against missing/wrong-typed keys - pr-review.py: tighten is_no_findings to require exact standalone sentence match - pr-review.py: wrap MAX_DIFF_CHARS int() in try/except with safe fallback - pr-review.yml: fix yamllint truthy warning by quoting "on:" key; fix colon spacing; add contents: read permission; restructure to detect+prepare only (no secrets) — uploads diff + metadata as artifact - pr-review-run.yml: new workflow_run-triggered workflow that downloads the artifact and runs the AI review without ever checking out PR head code, isolating ANTHROPIC_API_KEY from untrusted PR changes https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd * fix: extend sensitive-pattern coverage and fix API response assembly - _SENSITIVE_PATTERNS: add five quoted-key variants ("apiKey": "val", 'api-key': 'val', etc.) that run before the broad base64/hex catch-all; existing bare-key patterns remain for unquoted forms so both sets cover all key spellings (api_key, api-key, apiKey, camelCase, etc.) - call_api: replace single data["content"][0]["text"] return with an iteration over all content blocks, joining every item where item is a dict and item["text"] is a str, so multi-block responses are not truncated - pr-review-run.yml: add actions: read permission required by actions/download-artifact@v4 when fetching artifacts via run-id + github-token https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd * fix: address code-review findings in pr-review scripts pr-review.py: - Tighten broad base64 regex to require at least one +/= char, preventing false-positive redaction of commit SHAs and hex UUIDs (line 85) - Use idiomatic list unpacking [0, *RETRY_DELAYS] instead of concatenation (line 130) - Replace unused loop variable `key` with PERSONAS.values() iteration (line 235) - Remove redundant explicit "r" mode from open() in read_file (line 98) - Fix conjunction-stripping heuristic: strip leading conjunction prefix then test remaining text for substantive content, so "However, consider X." is correctly treated as substantive rather than ignored (line 188) pr-review-run.yml: - Wrap json.load(open(...)) in a context manager to ensure file is closed (line 36) - Replace --body "$BODY" with --body-file to avoid shell injection/quoting issues with large or special-character comment content (line 77) https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd * fix: expand token redaction coverage and harden GITHUB_OUTPUT writes pr-review.py: - Add explicit provider-prefix patterns (ghp_/gho_/ghs_/ghr_/github_pat_, sk-, pk_live_) checked before generic fallbacks - Add JWT pattern anchored on the eyJ header common to all JWTs - Add a base64url-alphabet fallback (A-Za-z0-9_-, 32+ chars) to catch URL-safe tokens that lack the + / = chars required by the previous rule All new patterns are ordered most-specific → most-general pr-review-run.yml: - Validate each key from review-meta.json against ^[A-Za-z0-9_-]+$; skip any key that fails to prevent injection into GITHUB_OUTPUT - Write multiline values using the key<<EOF...EOF delimiter format so newlines and control characters in values cannot inject new output keys https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd --------- Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces an automated AI-powered pull request review system that analyzes code changes through multiple domain-specific personas. The system uses the Anthropic API to provide targeted feedback on GUI/UX, server configuration, API extensibility, and test coverage — complementing the existing CodeRabbit automated reviews.
Key Changes
GitHub Actions Workflow (
.github/workflows/pr-review.yml)ANTHROPIC_API_KEYis unavailablePython Review Script (
.github/scripts/pr-review.py)Review Personas (
.cursor/rules/persona-*.mdc)@NotNull/@Nullablecontracts, registry extension points, and backward compatibilityTimeProviderusage,McRPGBaseTeststructure, and edge case completenessClaude Commands (
.claude/commands/review-*.md)CodeRabbit Configuration (
.coderabbit.yaml)Documentation Update (
CLAUDE.md)Implementation Details
src/*/gui/*.java,resources/*.yml,src/*/event/*.java).mdcfile and updating thePERSONASdictThe system is designed to be non-blocking (failures don't fail the workflow) and transparent (all output is logged to stderr for debugging).
https://claude.ai/code/session_01Ckae4mH7sneaYK5xYUmtsd
Summary by CodeRabbit