chore: improvement to PR review skill (#15040)

* chore: improvement to PR review skill

* fix help wanted

* skip for no changes

Author: shahednasser

.claude/skills/reviewing-prs/SKILL.md

@@ -16,7 +16,7 @@ Reviews GitHub pull requests for Medusa. Checks template compliance, contributio
 
 - **Checking contribution guidelines?** → MUST load `reference/contribution-types.md` first
 - **Verifying code conventions?** → MUST load `reference/conventions.md` first
-- **Writing a review comment?** → MUST load `reference/comment-guidelines.md` first
+- **Writing a review comment?** → MUST load `reference/comment-guidelines.md` first (includes bug reporting format)
 
 **Minimum requirement:** Load at least the relevant reference file(s) before completing the review.
 
@@ -72,6 +72,7 @@ For each previously raised issue or remark, determine whether it has been addres
 - If **all prior issues are resolved** — acknowledge the progress in the new review comment and focus only on any remaining or new issues.
 - If **some prior issues remain unresolved** — carry them forward into the new review. Do not re-explain them in detail; reference them briefly (e.g., *"The changeset message format is still incorrect"*).
 - If **this is the first review** (no prior bot comments) — skip this step.
+- If **there is a prior review and nothing has changed** — no new issues, no resolved issues, no new concerns — **do not post a new comment**. Stop here.
 
 > **CRITICAL:** Do not repeat the full explanation for issues already raised in a previous comment. Keep follow-up reviews concise — assume the contributor has read the prior feedback.
 
@@ -109,7 +110,7 @@ If the PR has more than 500 changed lines (additions + deletions) **or** more th
 ```bash
 bash scripts/get_linked_issues.sh <pr_number>
 ```
-Check whether any linked issue carries a `help wanted` label. If not, apply `requires-more` and comment explaining that large contributions should be scoped and pre-approved via an issue first (reference `CONTRIBUTING.md`).
+Check whether any linked issue carries a `help-wanted` label. If not, apply `requires-more` and comment explaining that large contributions should be scoped and pre-approved via an issue first (reference `CONTRIBUTING.md`).
 
 **4b. Security scan:**
 
@@ -150,7 +151,27 @@ For mixed PRs, apply all relevant types.
 
 Load `reference/conventions.md` and verify the changed files follow Medusa's conventions. Focus on the areas most relevant to the contribution type (e.g., API conventions for code changes, MDX structure for docs changes).
 
-### Step 8 — Contextual Assessment
+### Step 8 — Bug Detection
+
+Read the actual code diff carefully and look for potential bugs. Focus on:
+
+- **Logic errors** — off-by-one, wrong conditionals, inverted boolean checks
+- **Null / undefined access** — accessing properties on values that may be null/undefined without guards
+- **Async issues** — missing `await`, unhandled promise rejections, race conditions
+- **Type mismatches** — passing wrong types, unsafe casts, implicit coercions
+- **Resource leaks** — unclosed DB connections, missing transaction rollbacks, unhandled errors in cleanup paths
+- **Edge cases not handled** — empty arrays, zero values, missing input validation
+- **Mutation side-effects** — mutating shared state or function arguments unexpectedly
+- **Incorrect error handling** — swallowed errors, rethrowing without context, wrong error types
+
+For each potential bug found:
+- Note the **file and approximate location**
+- Briefly explain **why it's a bug or risk**
+- Suggest a **concrete fix** if one is obvious
+
+> **CRITICAL:** Distinguish between confirmed bugs (clear logic/runtime errors) and code smell / style issues. Only flag something as a bug if it would plausibly cause incorrect behaviour at runtime.
+
+### Step 9 — Contextual Assessment
 
 Before writing the review, assess whether the changes make sense in the broader context of the PR. Load `reference/comment-guidelines.md` (Contextual Assessment section) for the full checklist. Key questions:
 
@@ -162,7 +183,7 @@ Before writing the review, assess whether the changes make sense in the broader
 
 Note any concerns to include in the review comment.
 
-### Step 9 — Compose and Post Review
+### Step 10 — Compose and Post Review
 
 Load `reference/comment-guidelines.md` for comment templates and tone guidance.
 

.claude/skills/reviewing-prs/reference/comment-guidelines.md

@@ -73,6 +73,34 @@ If concerns are found, include them in the review comment under a **"Concerns"**
 
 Keep concerns concise and factual — describe the problem, not a lecture. If unsure whether something is actually a bug, phrase it as a question: *"Should `X` also handle the case where `Y` is null?"*
 
+## Potential Bugs Section
+
+When bugs are found in Step 8, include a **"Potential Bugs"** section in the review comment. This section is **always required** when bugs are found — for both `initial-approval` and `requires-more`.
+
+- Confirmed bugs (will cause incorrect runtime behaviour) → add to **Required changes** and apply `requires-more`
+- Uncertain / possible bugs → include under **"Potential Bugs"** as a flagged concern, even on `initial-approval`
+
+**Format:**
+
+```
+**Potential Bugs:**
+
+⚠️ **`path/to/file.ts`** — <one-line description of the bug>
+```typescript
+// the problematic snippet
+```
+<Explanation of why this is a problem and what could go wrong. If a fix is obvious, suggest it.>
+
+⚠️ **`path/to/other-file.ts`** — <another bug>
+...
+```
+
+**Rules:**
+- Always quote the relevant code snippet in a fenced code block so the author knows exactly what line is flagged
+- Be specific about the failure scenario — *"this will throw if `items` is empty"* not *"this looks wrong"*
+- If unsure, phrase as a question: *"Should this also handle the case where X is undefined?"*
+- Do not flag style issues or code smell here — only actual correctness/runtime concerns
+
 ## Security Risk Comment Template
 
 Use when a potential (non-malicious) security risk is found. Include it as part of the review, not as a standalone comment.