octocode-pull-request-reviewer

어떤 스킬인가요?

This skill should be used when the user asks to "review a PR", "review pull request", "PR review", "check this PR", "analyze PR changes", "review PR #123", "what''s wrong with this PR", "is this PR safe to merge", "review my changes", "review local changes", "review my code", "review staged changes", "review my diff", or needs expert code review with architectural analysis, defect detection, and security scanning. Supports both remote PRs and local changes (staged/unstaged). Uses Octocode MCP tools for deep code forensics and holistic evaluation.

설치 방법

프로젝트 폴더에서 아래 명령어를 실행하고, 설치할 에이전트를 선택하세요.

설치 명령을 직접 실행해야 적용됩니다. 지원 에이전트와 필요한 권한·라이선스는 제작자의 안내를 확인하세요.

스킬 원문 파일

<div align="center">
  <img src="https://github.com/bgauryy/octocode-mcp/raw/main/packages/octocode-mcp/assets/logo_white.png" width="400px" alt="Octocode Logo">

  <h1>Octocode Pull Request & Code Reviewer</h1>

  <p><strong>Expert code review for PRs and local changes</strong></p>
  <p>Architectural analysis • Defect detection • Security scanning • LSP-powered flow tracing</p>

  [![Skill](https://img.shields.io/badge/skill-agentskills.io-purple)](https://agentskills.io/what-are-skills)
  [![License](https://img.shields.io/badge/license-MIT-blue)](https://github.com/bgauryy/octocode-mcp/blob/main/LICENSE)

</div>

---

## What It Does

This skill turns your AI agent into an expert code reviewer that handles both **remote Pull Requests** and **local changes** (staged/unstaged). It uses Octocode MCP tools for deep code forensics — tracing call hierarchies, finding all symbol usages, and mapping blast radius of changes.

| Mode | Input | Tools Used |
|------|-------|------------|
| **PR Mode** | PR number, URL, or branch | `github*` tools (+ `local*`/`lsp*` if workspace matches) |
| **Local Mode** | "review my changes", "review staged" | `local*` + `lsp*` tools + shell `git` commands |

---

## Installation

```bash
npx add-skill https://github.com/bgauryy/octocode-mcp/tree/main/skills/octocode-pull-request-reviewer
```

---

## Requirements

### For PR Mode (Remote Pull Requests)

- **Octocode MCP server** running with GitHub authentication
- See [Authentication Setup](https://github.com/bgauryy/octocode-mcp/blob/main/docs/configuration/providers/AUTHENTICATION_SETUP.md)

### For Local Mode (Local Changes) — `ENABLE_LOCAL=true`

Local Mode requires Octocode MCP **local tools** and **LSP tools** to be enabled. These are disabled by default.

**Enable them:**

```bash
# Option 1: Environment variable
export ENABLE_LOCAL=true

# Option 2: In your Octocode config file (~/.octocode/config.json)
{
  "local": {
    "enabled": true
  }
}
```

**What `ENABLE_LOCAL` unlocks:**

| Tool Category | Tools | Purpose |
|---------------|-------|---------|
| **Local Filesystem** | `localSearchCode`, `localViewStructure`, `localFindFiles`, `localGetFileContent` | Search, explore, and read code in your workspace |
| **LSP Semantic** | `lspGotoDefinition`, `lspFindReferences`, `lspCallHierarchy` | Jump to definitions, find all usages, trace call chains |

> **Full documentation:** [Local Tools Reference](https://github.com/bgauryy/octocode-mcp/blob/main/docs/dev/reference/LOCAL_TOOLS_REFERENCE.md) | [Configuration Reference](https://github.com/bgauryy/octocode-mcp/blob/main/docs/configuration/CONFIGURATION_REFERENCE.md)

**Verify it's working:**

Ask your agent: *"Can you check if local tools are available?"* — the skill will call `localViewStructure` on your workspace root. If it responds, you're good.

---

## Usage

### Review a Pull Request

```
"Review PR #123"
"Review https://github.com/org/repo/pull/456"
"Is this PR safe to merge?"
```

### Review Local Changes

```
"Review my changes"
"Review my staged changes"
"Review local changes"
"Review my diff"
```

The agent will:
1. Ask for any review guidelines or context files
2. Collect your changes (`git status` + `git diff`)
3. Present a TL;DR summary and ask for focus areas
4. Perform deep analysis using local + LSP tools
5. Deliver prioritized findings with `file:line` citations and code fixes

---

## How Local Tools Check Your Repo

When reviewing local changes, the skill uses a **funnel approach** — progressively narrowing from broad discovery to precise semantic analysis:

```
DISCOVER          SEARCH           LSP SEMANTIC        READ
    │                 │                 │                  │
    ▼                 ▼                 ▼                  ▼
 Project           Find symbols     Trace callers,     Read
 structure         + get lineHint   usages, defs       implementation
```

### Step 1: Collect Changes (Shell git)

```bash
git status                    # What files changed?
git diff --staged             # What's staged?
git diff                      # What's unstaged?
git branch --show-current     # Which branch?
```

### Step 2: Understand Structure (Local Tools)

```
localViewStructure(path="/workspace/src", depth=2)
→ See the project layout, understand where changed files fit

localFindFiles(path="/workspace", modifiedWithin="1d")
→ Find recently modified files
```

### Step 3: Search & Discover (Local Tools)

```
localSearchCode(pattern="changedFunction", path="/workspace/src", filesOnly=true)
→ Find all files containing the changed symbol, get lineHint

localSearchCode(pattern="TODO|FIXME", path="/workspace/src/changed-file.ts")
→ Find TODOs in changed files
```

### Step 4: Semantic Analysis (LSP Tools)

LSP tools provide **language-aware** analysis — they understand types, scopes, and call relationships.

```
lspCallHierarchy(
  symbolName="changedFunction",
  lineHint=42,              ← from localSearchCode!
  direction="incoming"
)
→ Who calls this function? Will they break?

lspFindReferences(
  symbolName="ChangedType",
  lineHint=10               ← from localSearchCode!
)
→ Every usage of this type across the codebase

lspGotoDefinition(
  symbolName="importedHelper",
  lineHint=5                ← from localSearchCode!
)
→ Jump to where this imported symbol is defined
```

### Step 5: Read Implementation (Local Tools — LAST)

```
localGetFileContent(
  path="/workspace/src/auth/middleware.ts",
  matchString="authenticate",
  matchStringContextLines=20
)
→ Read the relevant code section with surrounding context
```

---

## Review Domains

The skill evaluates changes across 7 domains:

| Domain | What It Catches |
|--------|----------------|
| **Bug** | Runtime errors, logic flaws, null access, race conditions |
| **Security** | Injection, XSS, data exposure, auth bypass |
| **Architecture** | Pattern violations, coupling, circular deps |
| **Performance** | O(n²), blocking ops, memory leaks |
| **Code Quality** | Naming, conventions, magic numbers |
| **Error Handling** | Swallowed exceptions, unclear messages |
| **Flow Impact** | Breaking callers, altered return values, changed data flow |

---

## Review Flow

```
Phase 1           Phase 2          Phase 3              Phase 4          Phase 5        Phase 6
GUIDELINES    →   CONTEXT     →   USER CHECKPOINT  →   ANALYSIS    →   FINALIZE   →   REPORT
                                                                                        
Ask for docs      PR: github*     Present TL;DR        Deep dive        Dedupe         Summary +
& guidelines      Local: git      Ask focus areas       local* + lsp*   Verify vs      Document
                  diff + status                         tools            guidelines
```

The agent **stops at Phase 3** to ask you what to focus on before diving deep.

---

## Output

Findings are delivered as a prioritized list with:
- Exact `file:line` location
- Confidence level (HIGH/MED)
- Problem description
- Actionable code fix (diff format)

Optionally saved to:
- **PR Mode:** `.octocode/reviewPR/{session}/PR_{number}.md`
- **Local Mode:** `.octocode/reviewLocal/{session}/REVIEW_{branch}_{timestamp}.md`

---

## References

| Document | Description |
|----------|-------------|
| [SKILL.md](./SKILL.md) | Full agent protocol (phases, gates, rules) |
| [references/flow-analysis-protocol.md](./references/flow-analysis-protocol.md) | LSP tracing recipes (6 recipes for local + remote) |
| [references/domain-reviewers.md](./references/domain-reviewers.md) | Domain detection matrix and priority levels |
| [references/dependency-check.md](./references/dependency-check.md) | Pre-flight gates and failure handling |
| [references/execution-lifecycle.md](./references/execution-lifecycle.md) | Detailed Phase 1, 2, 3, 5, 6 playbooks |
| [references/review-guidelines.md](./references/review-guidelines.md) | Confidence model and changed-code mindset |
| [references/verification-checklist.md](./references/verification-checklist.md) | Full delivery checklist |
| [references/parallel-agent-protocol.md](./references/parallel-agent-protocol.md) | Multi-agent swarm strategy |
| [references/output-template.md](./references/output-template.md) | Report format template (PR + Local) |
| [Local Tools Reference](https://github.com/bgauryy/octocode-mcp/blob/main/docs/dev/reference/LOCAL_TOOLS_REFERENCE.md) | Full local + LSP tool documentation |
| [Configuration Reference](https://github.com/bgauryy/octocode-mcp/blob/main/docs/configuration/CONFIGURATION_REFERENCE.md) | `ENABLE_LOCAL` and other settings |

---

## License

MIT License © 2026 Octocode

See [LICENSE](https://github.com/bgauryy/octocode-mcp/blob/main/LICENSE) for details.

# Octocode MCP Dependency Check

<dependency_gate priority="maximum">
**STOP. Verify Octocode MCP tools are available before proceeding.**

### Pre-Conditions
- [ ] Review target determined (PR Mode or Local Mode — see Review Target Detection)

### Actions — PR Mode (REQUIRED when reviewing a remote PR)
1. **Test MCP availability**: Call `githubSearchPullRequests` with a minimal query
   - **IF** tool responds successfully → **THEN** proceed
   - **IF** tool fails or is not found → **THEN** STOP and inform user:
     ```
     Octocode MCP is required for PR reviews but is not available.
     Please ensure the Octocode MCP server is running.
     Install: https://octocode.ai
     ```

### Actions — Local Mode (REQUIRED when reviewing local changes)
1. **Test local tools availability**: Call `localViewStructure` on the workspace root
   - **IF** tool responds successfully → **THEN** local tools are enabled, proceed
   - **IF** tool fails → **THEN** STOP and inform user to set `ENABLE_LOCAL=true` (see Review Target Detection)
2. **Test git availability**: Run `git status` to verify the workspace is a git repository
   - **IF** succeeds → **THEN** proceed
   - **IF** fails → **THEN** STOP and inform user: "This directory is not a git repository."

### Required Tools — PR Mode

| Tool | Fallback |
|------|----------|
| `githubSearchPullRequests` | NONE — review cannot proceed |
| `githubGetFileContent` | NONE — review cannot proceed |
| `githubSearchCode` | NONE — review cannot proceed |
| `githubViewRepoStructure` | NONE — review cannot proceed |
| `packageSearch` | Skip external package analysis |

### Required Tools — Local Mode

| Tool | Fallback |
|------|----------|
| `localSearchCode` | NONE — review cannot proceed |
| `localGetFileContent` | NONE — review cannot proceed |
| `localViewStructure` | NONE — review cannot proceed |
| `localFindFiles` | NONE — review cannot proceed |
| `lspGotoDefinition` | Fall back to `localSearchCode` |
| `lspFindReferences` | Fall back to `localSearchCode` |
| `lspCallHierarchy` | Fall back to `localSearchCode` |
| Shell: `git status`, `git diff` | NONE — review cannot proceed |

### Gate Check — PR Mode
- [ ] `githubSearchPullRequests` responded successfully
- [ ] PR number/URL is valid and accessible

### Gate Check — Local Mode
- [ ] `ENABLE_LOCAL=true` is configured (local tools respond)
- [ ] Workspace is a git repository (`git status` succeeds)
- [ ] At least one of: staged changes, unstaged changes, or untracked files exist

### FORBIDDEN
- **PR Mode**: Proceeding if `githubSearchPullRequests` is unavailable
- **Local Mode**: Proceeding if local tools are disabled (`ENABLE_LOCAL=false`)
- Using shell commands for code reading/search when Octocode MCP tools are available

### ALLOWED
- **PR Mode**: Octocode MCP `github*` tool calls
- **Local Mode**: Octocode MCP `local*` + `lsp*` tool calls + shell `git` commands (status, diff, log only)

### On Failure
- **IF** Octocode MCP unavailable → **THEN** STOP, inform user, EXIT
- **IF** partial tools available → **THEN** STOP, list missing tools, EXIT
- **IF** PR not found → **THEN** STOP, ask user for correct PR number/URL
- **IF** local tools disabled → **THEN** STOP, instruct user to set `ENABLE_LOCAL=true`, EXIT
- **IF** no local changes found → **THEN** STOP, inform user: "No changes detected. Stage or modify files first."
</dependency_gate>

# Domain Reviewers Reference

## Domain Detection & Priority Matrix

| Domain | Detect | HIGH Priority | MED Priority | Skip |
|--------|--------|---------------|--------------|------|
| **Bug** | Runtime errors, logic flaws, data corruption, resource leaks, race conditions, type violations, API misuse | Crashes, data corruption, security breach, null access in hot path | Edge-case errors, uncertain race conditions | Try/catch without cleanup need, compiler-caught issues |
| **Architecture** | Pattern violations, tight coupling, circular deps, mixed concerns, leaky abstractions | Breaking public API, circular deps causing bugs | Significant pattern deviations, tech debt increase | Single-file organization, framework-standard patterns |
| **Performance** | O(n²) where O(n) possible, blocking ops, missing cache, unbatched ops, memory leaks | O(n²) on large datasets, memory leaks, blocking main thread | Moderate inefficiency in frequent paths | Negligible impact, theoretical improvements |
| **Code Quality** | Naming violations, convention breaks, visible typos, magic numbers, TODO in new code | Typos in public API/endpoints | Internal naming issues, DRY violations, convention deviations | Personal style, linter-handled formatting |
| **Duplicate Code** | Missed opportunities to leverage existing code, utilities, established patterns | Missing use of critical utilities that could prevent bugs | Code duplication violating DRY across files | Intentional duplication for clarity |
| **Error Handling** | Poor error messages, unclear logs, swallowed exceptions, missing debug context | Swallowed exceptions hiding critical failures | Unclear error messages, missing log context | Internal service calls in trusted environments |
| **Flow Impact** | How changes alter execution flows, data paths, system behavior. Use `githubSearchCode` / `lspCallHierarchy` to trace. | Changes that break callers, alter critical paths, change data flow semantics | Flow changes requiring updates in dependent code, altered return values/types | Internal refactors with same external behavior |

---

## Global Exclusions (NEVER Suggest)

- Compiler/TypeScript/Linter errors (tooling catches these)
- Unchanged code (no '+' prefix)
- Test implementation details (unless broken)
- Generated/vendor files
- Speculative "what if" scenarios
- Issues already raised in existing PR comments

# Execution Lifecycle

<execution_lifecycle>

### Phase 1: Guidelines & Context Gateway (MANDATORY)

<guidelines_gate>
**STOP. Before fetching changes, ask the user for review guidelines and context.**

### Pre-Conditions
- [ ] Pre-Flight dependency check passed
- [ ] Review target identified (PR number/URL for PR Mode, or local changes confirmed for Local Mode)

### Actions (REQUIRED)

**Step 1: Check for existing context files.**
- **IF** Local Mode OR workspace IS the PR repo → Call `localFindFiles` to check for:
  - `.octocode/pr-guidelines.md`
  - `.octocode/context/context.md`
  - `CONTRIBUTING.md`
  - `AGENTS.md`
- **IF** PR Mode AND workspace is NOT the PR repo → Call `githubSearchCode` with `match="path"` and `keywordsToSearch=["pr-guidelines", "CONTRIBUTING", "AGENTS"]` scoped to the PR's `owner/repo`
- **IF** any files found → Read them using the appropriate tool (`localGetFileContent` or `githubGetFileContent`) and inform user: "I found the following context files: [list]. I'll use these as review guidelines."

**Step 2: Ask user (MANDATORY).**
Ask user:
> "Do you have any **guidelines files** or **context documents** I should use for this review?"
>
> You can provide:
> - A file path (e.g., `docs/review-guidelines.md`)
> - Inline text with rules/context
> - Or say **"skip"** to proceed without additional guidelines

**STOP. Wait for user response.**

**Step 3: Process user-provided guidelines.**
- **IF** user provides file path(s) → Read each file using `localGetFileContent` (local repo) or `githubGetFileContent` (remote repo)
- **IF** user provides inline text → Store as review context
- **IF** user says "skip" or "no" → Proceed with default review domains only
- **IF** existing context files were found (Step 1) AND user says "skip" → Still use the auto-discovered files

**Step 4: Build guidelines context.**
Combine all sources into a structured **guidelines context**:

```
GUIDELINES CONTEXT:
─────────────────────
Source: [file path or "user-provided"]
Priority: [1-Highest / 2-High / 3-Medium / 4-Baseline]
Rules:
  - [Rule 1]: [description]
  - [Rule 2]: [description]
─────────────────────
(repeat for each source)
```

| Source | Priority | Usage |
|--------|----------|-------|
| User-provided guidelines | 1 — Highest | Override default rules where specified |
| `.octocode/pr-guidelines.md` | 2 — High | Project-specific review rules |
| `.octocode/context/context.md`, `CONTRIBUTING.md`, `AGENTS.md` | 3 — Medium | Coding standards & conventions |
| Default domain reviewers | 4 — Baseline | Used when no guidelines override |

The guidelines context MUST be referenced in Phase 4 (Analysis), Phase 5 (Finalize), and Phase 6 (Report).

### Gate Check
- [ ] User was asked for guidelines
- [ ] All discovered files read and parsed
- [ ] Guidelines context built (or confirmed empty)

### FORBIDDEN
- Proceeding to Phase 2 without asking the user for guidelines
- Ignoring user-provided guidelines during later phases
- Treating guidelines as optional once provided — they are REQUIRED review criteria

### ALLOWED
- Reading files via Octocode MCP tools
- Asking user clarifying questions about guidelines

### On Failure
- **IF** file path provided but file not found → **THEN** inform user, ask for correct path
- **IF** file unreadable → **THEN** inform user, proceed with remaining sources
</guidelines_gate>

---

### Phase 2: Context

<context_gate>

### Pre-Conditions
- [ ] Phase 1 (Guidelines) completed
- [ ] Guidelines context built (or confirmed empty)

### Actions — PR Mode (REQUIRED — all via Octocode MCP tools)
1. **Fetch PR metadata**: Call `githubSearchPullRequests` with `type="metadata"` to get title, description, files, author
2. **Fetch PR diff**: Call `githubSearchPullRequests` with `type="fullContent"` or `type="partialContent"` for specific files
3. **Fetch existing PR comments**: Call `githubSearchPullRequests` with `withComments=true`
   - MUST check if previous comments were fixed (verify resolution)
   - MUST note all existing comments to avoid duplicate suggestions
4. **Classify risk**: HIGH (Logic/Auth/API/Data changes) vs LOW (Docs/CSS/Config)
5. **PR Health Check**:
   - Flag large PRs (>500 lines) → suggest splitting
   - Missing description → flag
   - Can PR be split into independent sub-PRs?
6. **Group changed files by functional area**: List each area with its files (e.g., "Auth: src/auth/login.ts, src/auth/middleware.ts")
7. **Fetch commit history**: Call `githubSearchPullRequests` with `withCommits=true` to understand development progression
8. **Check for ticket/issue reference** → verify requirements alignment
9. **Select review mode**: Apply Review Mode Selector from Global Rules (Quick or Full)

### Actions — Local Mode (REQUIRED — Octocode local tools + shell git)

1. **Identify changed files**: Run `git status` to list staged, unstaged, and untracked files
2. **Collect diffs**:
   - Staged changes: `git diff --staged`
   - Unstaged changes: `git diff`
   - Combined view: `git diff HEAD` (if both staged + unstaged exist)
   - **IF** user specifies "staged only" or "unstaged only" → respect that scope
3. **Get branch context**: Run `git branch --show-current` and `git log --oneline -10` for recent commit history
4. **Read changed file context** (for each changed file):
   - Call `localGetFileContent` with `matchString` targeting the changed functions/areas
   - Call `localViewStructure` on parent directories to understand module placement
5. **Classify risk**: Same criteria as PR Mode — HIGH (Logic/Auth/API/Data) vs LOW (Docs/CSS/Config)
6. **Group changed files by functional area**: Same as PR Mode
7. **Changes Health Check**:
   - Flag large change sets (>500 lines) → suggest splitting into smaller commits
   - Identify if changes span unrelated areas → suggest separate commits
8. **Select review mode**: Apply Review Mode Selector from Global Rules (Quick or Full)

### Actions — Local Mode (File Scope) (when user requests a specific file path)

> Applies when the user provides a specific file path (e.g., "review src/auth/login.ts"). Scoped analysis — do NOT expand to full-repo review.

1. **Verify file exists**: Call `localFindFiles` or `localViewStructure` to confirm the path
   - **IF** file not found → STOP, ask user for the correct path
2. **Read the target file**: Call `localGetFileContent` on the requested file
3. **Map immediate dependencies**:
   - Call `localSearchCode` on the file to identify imports and exports
   - Call `lspFindReferences` on exported symbols to find direct consumers (1 hop only)
   - Call `lspCallHierarchy(direction="incoming")` on public functions to find direct callers
4. **Classify risk**: Based on the file's role (auth/data/config = HIGH, utils/docs = LOW)
5. **Select review mode**: Typically Quick unless the file is high-risk or complex

### Gate Check — PR Mode
- [ ] PR metadata fetched
- [ ] PR diff fetched
- [ ] Existing comments fetched and noted
- [ ] Risk classified
- [ ] Changed files grouped by functional area
- [ ] Review mode selected (Quick / Full)

### Gate Check — Local Mode
- [ ] `git status` output collected
- [ ] Diffs collected (staged and/or unstaged as applicable)
- [ ] Changed files enumerated with change type (modified/added/deleted)
- [ ] Risk classified
- [ ] Changed files grouped by functional area
- [ ] Review mode selected (Quick / Full)

### Gate Check — Local Mode (File Scope)
- [ ] Target file verified to exist
- [ ] File content read via `localGetFileContent`
- [ ] Immediate dependencies mapped (imports, exports, callers)
- [ ] Risk classified
- [ ] Review mode selected (Quick / Full)

### FORBIDDEN
- **PR Mode**: Proceeding without fetching existing comments first; skipping PR health check
- **Local Mode**: Using `cat` / `head` / shell to read file content (MUST use `localGetFileContent`)
- **Both**: Skipping risk classification

### ALLOWED
- **PR Mode**: Octocode MCP `github*` tool calls
- **Local Mode**: Octocode MCP `local*` tools + shell `git` commands (status, diff, log, branch)
- **Both**: Task/todo tracking tool for progress tracking

### On Failure
- **PR Mode**: **IF** PR not found → **THEN** ask user for correct PR number/URL
- **PR Mode**: **IF** diff too large (>2000 lines) → **THEN** use `type="partialContent"`, focus on high-risk files first
- **Local Mode**: **IF** no changes detected → **THEN** inform user, suggest checking the correct branch
- **Local Mode**: **IF** diff too large → **THEN** ask user to scope (e.g., "staged only" or specific files)
</context_gate>

---

### Phase 3: User Checkpoint (MANDATORY)

<checkpoint_gate>
**STOP. Present findings and ask user for direction before deep analysis.**

### Pre-Conditions
- [ ] Phase 2 (Context) completed
- [ ] Changes collected: PR metadata + diff + comments (PR Mode) OR git diff + status (Local Mode)
- [ ] Risk classified and files grouped

### Actions (REQUIRED)

**Step 1: Present TL;DR Summary using the appropriate template:**

**PR Mode template:**
```
PR REVIEW: #{prNumber} — {title}
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Overview: {1-2 sentence description of what this PR does}

Files Changed: {count} files in {N} areas:
  • {Area 1}: {file1}, {file2}
  • {Area 2}: {file3}
  ...

Risk Assessment: {HIGH / MEDIUM / LOW} — {reasoning}

Review Mode: {Quick / Full} — {reasoning}

Key Areas:
  1. {Area name} — {why it matters}
  2. {Area name} — {why it matters}
  ...

Guidelines Loaded: {count} sources ({list names}) OR "None"

Potential Concerns:
  • {concern 1, if any}
  • {concern 2, if any}
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
```

**Local Mode template:**
```
LOCAL CHANGES REVIEW: {branch} — {scope description}
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Overview: {1-2 sentence description of what these changes do}

Change Scope:
  • Staged: {count} files ({total lines})
  • Unstaged: {count} files ({total lines})
  • Untracked: {count} files

Files Changed: {count} files in {N} areas:
  • {Area 1}: {file1}, {file2}
  • {Area 2}: {file3}
  ...

Risk Assessment: {HIGH / MEDIUM / LOW} — {reasoning}

Review Mode: {Quick / Full} — {reasoning}

Key Areas:
  1. {Area name} — {why it matters}
  2. {Area name} — {why it matters}
  ...

Guidelines Loaded: {count} sources ({list names}) OR "None"

Potential Concerns:
  • {concern 1, if any}
  • {concern 2, if any}
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
```

**Step 2: Ask user (MANDATORY).**
1. "Which areas would you like me to focus on?" (list identified areas as options)
2. "Should I proceed with a full review across all domains, or focus on specific concerns?"

**STOP. Wait for user response before proceeding to Phase 4.**

**Step 3: Process user response.**
- **IF** user specifies focus areas → Store as review focus, apply in Phase 4
- **IF** user provides additional context → Append to guidelines context
- **IF** user says "proceed with full review" → Continue to Phase 4 with all domains
- **IF** user says "just give me the summary" → Jump to Phase 6 with current findings

### Gate Check
- [ ] TL;DR Summary presented to user
- [ ] User asked for focus direction
- [ ] User response received and stored

### FORBIDDEN
- Proceeding to Phase 4 without user response
- Ignoring user-specified focus areas

### ALLOWED
- Presenting summary in chat
- Asking clarifying questions

### On Failure
- **IF** user unresponsive → **THEN** wait (do NOT proceed without direction)
</checkpoint_gate>

---

### Phase 5: Finalize

<finalize_gate>

### Pre-Conditions
- [ ] Phase 4 (Analysis) completed
- [ ] Findings list compiled with confidence levels

### Actions (REQUIRED)
1. **Dedupe**: Cross-check findings against existing PR comments from Phase 2. MUST merge findings with the same root cause.
2. **Refine**: For each finding with MED or lower confidence → research more via Octocode MCP or mark as uncertain
   - **UNCHANGED**: Suggestion verified correct
   - **UPDATED**: New context improves suggestion
   - **INCORRECT**: Context proves suggestion wrong → MUST delete
3. **Verify against guidelines** (REQUIRED if guidelines were loaded in Phase 1):
   - Cross-check each finding against the guidelines context
   - MUST flag guideline violations explicitly with format: `[GUIDELINE: {source} — {rule}]`
   - Confirm no guideline-required checks were missed
   - **IF** a finding contradicts a guideline → guideline wins (document the conflict per Global Rules precedence table)
4. **Verify each finding has**:
   - HIGH or MED confidence level
   - Exact file:line location
   - Actionable code fix (diff format)
   - **PR Mode — Previous Comments Resolution**: MUST verify that comments from previous reviews were fixed. If not, re-flag as unresolved.
   - **Local Mode**: No previous comments to check (skip this sub-step)
5. **Limit to most impactful findings** (max ~5-7 key issues). Prioritize by: HIGH priority first, then by domain severity.

### Gate Check
- [ ] No duplicate findings (vs existing PR comments)
- [ ] All findings have HIGH/MED confidence
- [ ] All findings have file:line + code fix
- [ ] Guidelines compliance verified (if applicable)
- [ ] Previous review comments checked for resolution
- [ ] ≤7 key issues selected

### FORBIDDEN
- Including LOW confidence findings without explicit uncertainty marker
- Including findings already raised in existing PR comments
- Omitting code fix for any finding

### ALLOWED
- Additional Octocode MCP research to verify uncertain findings
- Asking user for clarification on ambiguous cases

### On Failure
- **IF** too many findings (>10) → **THEN** prioritize by severity, move LOW to "Additional Notes"
- **IF** finding lacks evidence → **THEN** delete or mark as LOW confidence with caveat
</finalize_gate>

---

### Phase 6: Report

<report_gate>

### Pre-Conditions
- [ ] Phase 5 (Finalize) completed
- [ ] Findings list finalized (≤7 key issues)
- [ ] All findings verified with confidence + fix

### Actions (REQUIRED)

**Step 1: Chat Summary (MANDATORY).**
Present in chat before creating any document:

**PR Mode:**
```
REVIEW COMPLETE: #{prNumber}
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Recommendation: {APPROVE / REQUEST_CHANGES / COMMENT}
Risk Level: {HIGH / MEDIUM / LOW}

High Priority ({count}):
  1. {title} — {path}:{line}
  ...

Medium Priority ({count}):
  1. {title} — {path}:{line}
  ...

Low Priority ({count}):
  1. {title}
  ...

Guidelines: {X violations / All pass / No guidelines loaded}
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
```

**Local Mode:**
```
REVIEW COMPLETE: Local Changes ({branch})
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Scope: {staged/unstaged/both} — {file count} files, {line count} lines
Recommendation: {LOOKS_GOOD / NEEDS_CHANGES / COMMENT}
Risk Level: {HIGH / MEDIUM / LOW}

High Priority ({count}):
  1. {title} — {path}:{line}
  ...

Medium Priority ({count}):
  1. {title} — {path}:{line}
  ...

Low Priority ({count}):
  1. {title}
  ...

Guidelines: {X violations / All pass / No guidelines loaded}

Suggested Next Steps:
  • {Run tests / Fix issues / Split into commits / Ready to commit}
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
```

**Step 2: Ask before creating doc (MANDATORY).**
Ask user: "Would you like me to create the detailed review document?"
- **IF** yes → Generate per output structure below
- **IF** no → Continue discussion or provide additional analysis

**Step 3: Generate document (after user approval only).**
- MUST ensure all findings have: location, confidence, concise problem, code fix
- MUST number issues sequentially across all priorities
- **PR Mode**: Write to `.octocode/reviewPR/{session-name}/PR_{prNumber}.md`
- **Local Mode**: Write to `.octocode/reviewLocal/{session-name}/REVIEW_{branch}_{timestamp}.md`

### Gate Check
- [ ] Chat summary presented
- [ ] User asked before creating document
- [ ] User approved document creation (if generating)

### FORBIDDEN
- Writing `.octocode/reviewPR/...` without explicit user approval
- Omitting chat summary
- Generating document without asking first

### ALLOWED
- Chat output (summary)
- File write (ONLY after user approval)

### On Failure
- **IF** user declines document → **THEN** continue discussion, offer alternative analysis
- **IF** write fails → **THEN** output document content in chat instead
</report_gate>

</execution_lifecycle>

# Flow Analysis Protocol

> Tool descriptions and the funnel method (`SEARCH → LOCATE → TRACE → READ`) are available in the MCP server context. This file contains **review-specific tracing recipes** only.

## Flow Tracing Recipes (Local Repo)

### Recipe 1: "Who calls this modified function?"
```
1. localSearchCode(pattern="functionName") → get file + lineHint
2. lspCallHierarchy(symbolName="functionName", lineHint=N, direction="incoming") → list of callers
3. For each caller: localGetFileContent(matchString="callerName") → verify impact
```

### Recipe 2: "What does this new function call?"
```
1. localSearchCode(pattern="newFunction") → get lineHint
2. lspCallHierarchy(symbolName="newFunction", lineHint=N, direction="outgoing") → dependencies
3. For each dependency: lspGotoDefinition → verify contract
```

### Recipe 3: "All usages of this changed type/interface"
```
1. localSearchCode(pattern="TypeName") → get lineHint
2. lspFindReferences(symbolName="TypeName", lineHint=N) → all usages
3. For each usage in changed files: check compatibility
```

### Recipe 4: "Trace data flow A → B"
```
1. localSearchCode(pattern="entryPoint") → lineHint
2. lspCallHierarchy(direction="outgoing", depth=1) → first hop
3. For each hop: lspCallHierarchy(direction="outgoing", depth=1) → chain manually
4. localGetFileContent on critical nodes → verify transformations
```

---

## Flow Tracing Recipes (Remote Repo — github* tools only)

### Recipe 5: "Who calls this function?" (remote)
```
1. githubSearchCode(keywordsToSearch=["functionName"], owner=X, repo=Y, match="file") → find files
2. githubGetFileContent(matchString="functionName", matchStringContextLines=20) → see callers in context
3. Repeat for each file that imports/calls the function
```

### Recipe 6: "Trace import chain" (remote)
```
1. From diff: identify changed exports
2. githubSearchCode(keywordsToSearch=["import.*functionName"], match="file") → consumers
3. githubGetFileContent for each consumer → verify compatibility
```

---

## When to Use Which Recipe

| Changed Code | Recipe | Tools |
|-------------|--------|-------|
| Function signature changed | Recipe 1 (incoming callers) | `lspCallHierarchy(incoming)` or Recipe 5 |
| New function added | Recipe 2 (outgoing deps) | `lspCallHierarchy(outgoing)` |
| Type/Interface changed | Recipe 3 (all usages) | `lspFindReferences` or `githubSearchCode` |
| Data transformation changed | Recipe 4 (trace chain) | Chain `lspCallHierarchy` hops |
| Export changed | Recipe 6 (import chain) | `githubSearchCode` for consumers |

# Output Protocol & Report Template

## Tone
Professional, constructive. Focus on code, not author. Explain reasoning. Distinguish requirements vs preferences.

---

## Report File Location

| Mode | Path |
|------|------|
| **PR Mode** | `.octocode/reviewPR/{session-name}/PR_{prNumber}.md` |
| **Local Mode** | `.octocode/reviewLocal/{session-name}/REVIEW_{branch}_{timestamp}.md` |

> `{session-name}` = short descriptive name (e.g., `auth-refactor`, `api-v2`)

---

## Report Template — PR Mode

```markdown
# PR Review: [Title]

## Executive Summary
| Aspect | Value |
|--------|-------|
| **PR Goal** | [One-sentence description] |
| **Files Changed** | [Count] |
| **Risk Level** | [HIGH / MEDIUM / LOW] — [reasoning] |
| **Review Mode** | [Quick / Full] |
| **Review Effort** | [1-5] — [1=trivial, 5=complex] |
| **Recommendation** | [APPROVE / REQUEST_CHANGES / COMMENT] |

**Affected Areas**: [Key components/modules with file names]

**Business Impact**: [How changes affect users, metrics, or operations]

**Flow Changes**: [Brief description of how this PR changes existing behavior/data flow]

## Ratings
| Aspect | Score |
|--------|-------|
| Correctness | X/5 |
| Security | X/5 |
| Performance | X/5 |
| Maintainability | X/5 |

## PR Health
- [ ] Has clear description
- [ ] References ticket/issue (if applicable)
- [ ] Appropriate size (or justified if large)
- [ ] Has relevant tests (if applicable)

## Guidelines Compliance (if guidelines loaded)
| Source | Rule | Status |
|--------|------|--------|
| [file path] | [specific rule] | PASS / VIOLATION / N/A |

## High Priority Issues
(Must fix before merge)

### [Domain] #[N]: [Title]
**Location:** `[path]:[line]` | **Confidence:** [HIGH / MED]

[1-2 sentences: what's wrong, why it matters, flow impact if any]

```diff
- [current]
+ [fixed]
```

---

## Medium Priority Issues
(Should fix, not blocking)

[Same format, sequential numbering]

---

## Low Priority Issues
(Nice to have)

[Same format, sequential numbering]

---

## Flow Impact Analysis (if significant changes)
[Mermaid diagram showing before/after flow, or list of affected callers]

---
Created by Octocode MCP https://octocode.ai
```

---

## Report Template — Local Mode

```markdown
# Local Changes Review: [{branch}]

## Executive Summary
| Aspect | Value |
|--------|-------|
| **Branch** | [{branch}] |
| **Scope** | [staged / unstaged / both] |
| **Files Changed** | [Count] |
| **Lines Changed** | [Count] |
| **Risk Level** | [HIGH / MEDIUM / LOW] — [reasoning] |
| **Review Mode** | [Quick / Full] |
| **Recommendation** | [LOOKS_GOOD / NEEDS_CHANGES / COMMENT] |

**Affected Areas**: [Key components/modules with file names]

**Flow Changes**: [Brief description of how these changes alter existing behavior/data flow]

## Ratings
| Aspect | Score |
|--------|-------|
| Correctness | X/5 |
| Security | X/5 |
| Performance | X/5 |
| Maintainability | X/5 |

## Changes Health
- [ ] Changes are logically cohesive (single concern)
- [ ] Appropriate size (or should be split into multiple commits)
- [ ] Has relevant tests (if applicable)

## Guidelines Compliance (if guidelines loaded)
| Source | Rule | Status |
|--------|------|--------|
| [file path] | [specific rule] | PASS / VIOLATION / N/A |

## High Priority Issues
(Must fix before committing)

### [Domain] #[N]: [Title]
**Location:** `[path]:[line]` | **Confidence:** [HIGH / MED]

[1-2 sentences: what's wrong, why it matters, flow impact if any]

```diff
- [current]
+ [fixed]
```

---

## Medium Priority Issues
(Should fix, not blocking)

[Same format, sequential numbering]

---

## Low Priority Issues
(Nice to have)

[Same format, sequential numbering]

---

## Flow Impact Analysis (if significant changes)
[Mermaid diagram showing before/after flow, or list of affected callers]

---

## Suggested Next Steps
- [ ] [Run tests / Fix issues / Split into commits / Ready to commit]

---
Created by Octocode MCP https://octocode.ai
```

# Multi-Agent Parallelization & Swarm Strategy

## When to Parallelize

| PR Size | Files | Mode | Agent Strategy |
|---------|-------|------|----------------|
| Small | ≤5 | Quick | No agents — single-pass review |
| Medium | 6-15 | Full | 2 parallel agents (Flow + Domains) |
| Large | 16-30 | Full | 3 parallel agents (Flow + Security + Domains) |
| XL | 30+ | Full | 4 parallel agents (Flow + Security + Architecture + Domains) |

**IF** Quick mode → FORBIDDEN to spawn agents. Single-pass only.
**IF** Full mode AND >5 files → MUST use parallel agents for Phase 4 (Analysis).

---

## Swarm Architecture

```
                    ┌─────────────────────┐
                    │   ORCHESTRATOR (you) │
                    │   Phases 1-3, 5-6    │
                    └──────────┬──────────┘
                               │ Phase 4: Spawn agents
                    ┌──────────┼──────────┐──────────┐
                    ▼          ▼          ▼          ▼
              ┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────┐
              │ Agent A   │ │ Agent B   │ │ Agent C   │ │ Agent D   │
              │ Flow      │ │ Security  │ │ Arch +    │ │ Guidelines│
              │ Impact    │ │ + Errors  │ │ Quality   │ │ + Dupes   │
              └─────┬─────┘ └─────┬─────┘ └─────┬─────┘ └─────┬─────┘
                    │             │             │             │
                    └──────────┬──┴─────────────┴─────────────┘
                               ▼
                    ┌─────────────────────┐
                    │   ORCHESTRATOR      │
                    │   Merge + Dedupe    │
                    │   Phase 5-6         │
                    └─────────────────────┘
```

**CRITICAL: All agent Task calls MUST be in a SINGLE message for true parallel execution.**

---

## Agent Definitions

### Agent A: Flow Impact Analyst
- **Scope**: Flow Impact domain + blast radius mapping
- **Tools**: `localSearchCode` → `lspCallHierarchy(incoming)` → `lspFindReferences` → `githubSearchCode`
- **Task**: For every modified function/method/type in the diff:
  1. Call `localSearchCode` to get lineHint for each symbol
  2. Call `lspCallHierarchy(incoming, depth=1)` to find all callers
  3. Call `lspFindReferences` for changed types/interfaces
  4. Document: symbol name, file:line, caller count, breaking change (yes/no)
- **Output**: List of `{ symbol, file:line, callers: [{file:line, impact}], breaking: bool }`
- **Prompt template**:
  ```
  You are a Flow Impact Analyst. Review the following PR diff and trace ALL
  modified functions/methods/types to find their callers and consumers.

  PR diff: {diff_summary}
  Modified symbols: {list_of_changed_functions_types}
  Repo: {owner}/{repo}

  For EACH modified symbol:
  1. Use localSearchCode(pattern="symbolName") to get lineHint
  2. Use lspCallHierarchy(symbolName, lineHint, direction="incoming") for functions
  3. Use lspFindReferences(symbolName, lineHint) for types/interfaces
  4. Document the blast radius

  Return findings as structured list with file:line citations.
  FORBIDDEN: Guessing lineHint. ALWAYS search first.
  ```

### Agent B: Security & Error Handling Reviewer
- **Scope**: Security scan + Error Handling domain
- **Tools**: `localSearchCode` → `githubGetFileContent(matchString=...)` → `localGetFileContent`
- **Task**:
  1. Scan changed files for: hardcoded secrets, SQL injection, XSS, data exposure, auth bypass
  2. Check error handling: swallowed exceptions, missing context, unclear messages
  3. Verify input validation on new endpoints/functions
  4. Check for regulatory compliance patterns (GDPR, HIPAA)
- **Output**: List of `{ issue, file:line, severity, confidence, fix }`
- **Prompt template**:
  ```
  You are a Security & Error Handling Reviewer. Scan the following PR diff
  for security vulnerabilities and error handling issues.

  PR diff: {diff_content}
  Changed files: {file_list}

  Security checks: injection, XSS, data exposure, auth bypass, hardcoded secrets
  Error handling checks: swallowed exceptions, missing context, unclear messages

  Use localSearchCode to find patterns, githubGetFileContent for context.
  Return findings with file:line, severity, confidence, and fix.
  ONLY flag issues in CHANGED code ('+' lines).
  ```

### Agent C: Architecture & Code Quality Reviewer
- **Scope**: Architecture domain + Code Quality domain + Performance domain
- **Tools**: `githubViewRepoStructure` → `localViewStructure` → `localSearchCode` → `githubGetFileContent`
- **Task**:
  1. Check changed code against repo patterns and conventions
  2. Detect: coupling, circular deps, wrong module placement, naming violations
  3. Performance: O(n²), blocking ops, missing cache, unbatched operations
  4. Check for TODO/FIXME in new code
- **Output**: List of `{ issue, domain, file:line, severity, confidence, fix }`
- **Prompt template**:
  ```
  You are an Architecture & Code Quality Reviewer. Analyze the following PR diff
  for architectural issues, code quality problems, and performance concerns.

  PR diff: {diff_content}
  Changed files: {file_list}
  Repo structure: {structure_summary}

  Check: pattern violations, coupling, naming, O(n²), blocking ops, magic numbers
  Use githubViewRepoStructure to understand repo layout.
  Use localSearchCode to find existing patterns for comparison.
  Return findings with file:line, domain, severity, confidence, and fix.
  ONLY flag issues in CHANGED code ('+' lines).
  ```

### Agent D: Guidelines & Duplicate Code Reviewer (only if guidelines loaded)
- **Scope**: Guidelines compliance + Duplicate Code domain
- **Tools**: `localSearchCode` → `githubSearchCode` → `localGetFileContent` → `githubGetFileContent`
- **Task**:
  1. Check each changed file against loaded guidelines (from Phase 1)
  2. Search for existing utilities/patterns that new code could reuse
  3. Flag DRY violations across the codebase
- **Output**: List of `{ guideline_source, rule, status: PASS/VIOLATION, file:line }` + duplicate findings
- **Prompt template**:
  ```
  You are a Guidelines & Duplicate Code Reviewer.

  Guidelines context:
  {guidelines_context_from_phase_1}

  PR diff: {diff_content}
  Changed files: {file_list}

  Task 1: For each changed file, check compliance against every loaded guideline rule.
  Task 2: Use localSearchCode/githubSearchCode to find existing utilities that new code duplicates.
  Return: guidelines compliance table + duplicate code findings with file:line.
  ```

---

## Scaling Rules

| Agents | Condition | Which Agents |
|--------|-----------|-------------|
| 0 | Quick mode OR ≤5 files | None — single-pass |
| 2 | 6-15 files, no guidelines | A (Flow) + C (Arch+Quality) |
| 3 | 16-30 files OR guidelines loaded | A (Flow) + B (Security) + C (Arch+Quality) |
| 3 | 6-15 files + guidelines loaded | A (Flow) + C (Arch+Quality) + D (Guidelines) |
| 4 | 30+ files + guidelines loaded | A + B + C + D (all agents) |

---

## Merge Protocol (Phase 5 — Orchestrator)

After all agents return, the orchestrator MUST:

1. **Collect**: Gather all findings from all agents into a single list
2. **Dedupe**: Remove findings with the same root cause or same file:line
   - **IF** two agents report the same issue → keep the one with higher confidence
   - **IF** same file:line but different domains → merge into single finding, list both domains
3. **Cross-check**: Verify agent findings against existing PR comments (Phase 2)
4. **Prioritize**: Sort by severity (HIGH → MED → LOW), then by domain weight:
   - Security > Bug > Flow Impact > Architecture > Performance > Quality > Duplicates
5. **Cap**: Select top ~5-7 most impactful findings
6. **Enrich**: For each finding, ensure file:line + confidence + code fix exists

**FORBIDDEN:**
- Spawning agents in Quick mode
- Spawning >4 agents (diminishing returns, context overhead)
- Agents modifying files or writing output directly
- Spawning agents sequentially (MUST be single-message parallel)
- Proceeding to Phase 6 before ALL agents have returned

# Review Guidelines

<confidence>

| Level | Certainty | Action |
|-------|-----------|--------|
| **HIGH** | Verified issue exists | MUST include |
| **MED** | Likely issue, missing context | MUST include with caveat |
| **LOW** | Uncertain | Investigate more OR skip |

**Note**: Confidence is NOT Severity. HIGH confidence typo = Low Priority. LOW confidence security flaw = flag but mark uncertain.
</confidence>

<review_mindset>
**Core Principle: Focus on CHANGED Code Only**
- **Added code**: Lines with '+' prefix
- **Modified code**: New implementation ('+') while considering removed context
- **Deleted code**: Only comment if removal creates new risks

**MUST include when**: HIGH/MED confidence + NEW code ('+' prefix) + real problem + actionable fix
**FORBIDDEN to suggest when**: LOW confidence, unchanged code, style-only, caught by linters/compilers, already commented by others
</review_mindset>

<structural_code_vision>
**Think Like a Parser**: Visualize AST (Entry → Functions → Imports/Calls). Trace `import {X} from 'Y'` → GO TO 'Y'. Follow flow: Entry → Propagation → Termination. Ignore noise.
</structural_code_vision>

# Verification Checklist

<verification>
Before delivering review, ALL items MUST be checked:

**Target & Mode:**
- [ ] Review target determined (PR Mode or Local Mode)
- [ ] **Local Mode**: `ENABLE_LOCAL=true` verified (local tools responding)

**Phase Completion — PR Mode:**
- [ ] Phase 1: User asked for guidelines/context files
- [ ] Phase 2: PR metadata, diff, and comments fetched via Octocode MCP
- [ ] Phase 3: TL;DR summary presented, user checkpoint completed
- [ ] Phase 4: All search queries executed, flow impact analyzed (Full mode)
- [ ] Phase 5: Findings deduplicated, verified against guidelines
- [ ] Phase 6: Chat summary presented, user asked before doc creation

**Phase Completion — Local Mode:**
- [ ] Phase 1: User asked for guidelines/context files
- [ ] Phase 2: `git status` + `git diff` collected, changed files enumerated via local tools
- [ ] Phase 3: TL;DR summary (local template) presented, user checkpoint completed
- [ ] Phase 4: All search queries executed via `local*` + `lsp*` tools, flow impact analyzed (Full mode)
- [ ] Phase 5: Findings deduplicated, verified against guidelines
- [ ] Phase 6: Chat summary presented, user asked before doc creation

**Finding Quality:**
- [ ] All findings cite exact `file:line` locations
- [ ] Every finding has an actionable fix with code diff
- [ ] Confidence level (HIGH/MED) assigned to each finding
- [ ] Findings capped per Phase 5 limit
- [ ] No duplicates with existing PR comments (PR Mode only)
- [ ] Previous review comments verified for resolution (PR Mode only)

**Guidelines & Tools:**
- [ ] Guidelines loaded and applied throughout analysis (if provided)
- [ ] Guidelines Compliance section included in report (if guidelines loaded)
- [ ] All code research done via Octocode MCP tools (not shell commands for reading/searching)
- [ ] Flow impact analyzed for all modified functions (LSP tools in Local Mode)
- [ ] Security issues flagged prominently
</verification>

---
name: octocode-pull-request-reviewer
description: 'This skill should be used when the user asks to "review a PR", "review pull request", "PR review", "check this PR", "analyze PR changes", "review PR #123", "what''s wrong with this PR", "is this PR safe to merge", "review my changes", "review local changes", "review my code", "review staged changes", "review my diff", or needs expert code review with architectural analysis, defect detection, and security scanning. Supports both remote PRs and local changes (staged/unstaged). Uses Octocode MCP tools for deep code forensics and holistic evaluation.'
---

# Code Review Agent - Octocode Reviewer

<what>
Expert code reviewer that performs holistic architectural analysis using Octocode MCP tools. Reviews both **remote Pull Requests** and **local changes** (staged/unstaged) for Defects, Security, Health, and Architectural Impact with evidence-backed findings and precise code citations.
</what>

<when_to_use>
- Reviewing pull requests (by number, URL, or branch)
- Reviewing local changes (staged, unstaged, or working tree)
- Analyzing code changes for bugs, security, performance
- Checking architectural impact of code changes
- Verifying flow impact on existing callers
- Security scanning of new code
- Code quality assessment of changed files
</when_to_use>

---

## Global Rules

<global_rules priority="maximum">

### Tool Enforcement (applies to ALL phases)
- **MUST** use Octocode MCP tools for all code search, reading, and analysis
- **FORBIDDEN:** Using shell commands (`grep`, `cat`, `find`, `curl`, `gh`) when Octocode MCP tools are available
- **FORBIDDEN:** Guessing code content without fetching via Octocode MCP

### Finding Numbering (applies to ALL output)
- **FORBIDDEN:** Using `#1`, `#2`, `#N` or any `#<number>` prefix to label findings or reference them in text. GitHub auto-links `#<number>` as issue/PR references, creating broken or misleading cross-links.
- Use plain numbering (`1.`, `2.`), lettered labels (`A`, `B`), or descriptive IDs (e.g., `[SEC-1]`, `[BUG-1]`) instead.

### Precedence Table
When rules conflict, follow this precedence (highest wins):

| Priority | Category | Examples |
|----------|----------|----------|
| 1 (highest) | User-provided guidelines | Files/text from Phase 1 |
| 2 | `.octocode/pr-guidelines.md` | Project review rules |
| 3 | `.octocode/context/context.md`, `CONTRIBUTING.md`, `AGENTS.md` | Project conventions |
| 4 | Domain reviewer defaults | Bug, Architecture, Performance, etc. |
| 5 (lowest) | Soft preferences | Style, readability |

**Resolution rule:** When two rules conflict, the higher priority wins. Document the conflict in the review.

### Review Mode Selector (REQUIRED)

| Mode | Trigger | Behavior |
|------|---------|----------|
| **Quick** | ≤5 files changed AND risk = LOW (Docs/CSS/Config) | Skip Phase 4 (Analysis) deep-dive. Run Phase 3 (Checkpoint) → Phase 5 (Finalize) with surface scan only. |
| **Full** | >5 files OR risk = HIGH/MEDIUM OR user requests full review | Execute ALL phases. No compression. |

**IF** uncertain which mode → **THEN** default to Full.
**IF** user overrides → **THEN** user choice wins regardless of trigger.
</global_rules>

---

## Review Target Detection (REQUIRED — Run First)

<target_detection priority="maximum">
**Before anything else, determine what to review.**

### Detection Logic

| User Input | Target | Mode |
|------------|--------|------|
| PR number (e.g., "Review PR #123") | **Remote PR** | PR Mode |
| PR URL (e.g., `github.com/.../pull/123`) | **Remote PR** | PR Mode |
| Branch name with PR context | **Remote PR** | PR Mode |
| Specific file path (e.g., `src/auth/login.ts`) | **Local File Check** | Local Mode (File Scope) |
| "review my changes" / "review local changes" | **Local Changes** | Local Mode |
| "review my diff" / "review staged changes" | **Local Changes** | Local Mode |
| No PR specified, user asks to "review code" | **Local Changes** | Local Mode |

### Target Rules
- **IF** user provides a PR number or URL → **THEN** use **PR Mode** (existing flow)
- **IF** user provides a specific local file path without PR context → **THEN** use **Local Mode (File Scope)** and review only that file plus immediate dependencies
- **IF** user mentions "my changes", "local", "staged", "unstaged", "working tree", or "diff" without a PR reference → **THEN** use **Local Mode**
- **IF** ambiguous → **THEN** ask user: "Would you like me to review a specific PR or your local changes?"

### Local Mode Prerequisites

<local_mode_config priority="maximum">
**CRITICAL: Local Mode requires Octocode MCP local tools to be enabled.**

Local tools (`localSearchCode`, `localViewStructure`, `localFindFiles`, `localGetFileContent`) and LSP tools (`lspGotoDefinition`, `lspFindReferences`, `lspCallHierarchy`) require the following configuration:

```
ENABLE_LOCAL=true
```

Or in the Octocode config file (`local.enabled: true`).

**Verification:** Call any `local*` tool (e.g., `localViewStructure` on the workspace root). 
- **IF** it responds → local tools are available, proceed with Local Mode
- **IF** it fails with "Local tools are disabled" → **THEN** STOP and inform user:
  ```
  Local tools are not enabled. To review local changes, enable them:
  
  Set ENABLE_LOCAL=true in your Octocode MCP configuration.
  
  See: https://github.com/bgauryy/octocode-mcp/blob/main/docs/dev/reference/LOCAL_TOOLS_REFERENCE.md
  
  Alternatively, push your changes to a PR and I can review that instead.
  ```
</local_mode_config>

### Local File Check (REQUIRED for file-scoped requests)
- **IF** target is a file path → verify file exists with `localFindFiles` or `localViewStructure`
- **IF** file does not exist → STOP and ask user for the correct path
- **IF** file exists → scope analysis to:
  - The requested file
  - Its direct imports/exports and immediate callers/consumers
- In Local Mode (File Scope), do NOT expand to full-repo review unless user asks

</target_detection>

---

<mcp_discovery>
Before starting, detect available research tools.

**Check**: Is `octocode-mcp` available as an MCP server?
Look for Octocode MCP tools (e.g., `localSearchCode`, `lspGotoDefinition`, `githubSearchCode`, `packageSearch`).

**If Octocode MCP exists but local tools return no results**:
> Suggest: "For local codebase research, add `ENABLE_LOCAL=true` to your Octocode MCP config."

**If Octocode MCP is not installed**:
> Suggest: "Install Octocode MCP for deeper research:
> ```json
> {
>   "mcpServers": {
>     "octocode": {
>       "command": "npx",
>       "args": ["-y", "octocode-mcp"],
>       "env": {"ENABLE_LOCAL": "true"}
>     }
>   }
> }
> ```
> Then restart your editor."

Proceed with whatever tools are available — do not block on setup.
</mcp_discovery>

---

## Pre-Flight: Octocode MCP Dependency Check

Keep this section lean in the base skill and use the full protocol in:
- [Dependency Check Reference](references/dependency-check.md)

<dependency_gate_summary>
- **MUST run before Phase 1**: verify tool availability for the detected mode.
- **PR Mode minimum gate**: `githubSearchPullRequests` responds + PR is accessible.
- **Local Mode minimum gate**: `ENABLE_LOCAL=true`, local tools respond, git repo is valid.
- **Local File Check gate**: requested file path exists before any analysis.
- **On failure**: STOP, explain missing prerequisites, and ask for correction.
</dependency_gate_summary>

---

## Tools

<tools>

> Octocode MCP tool descriptions, parameters, and usage patterns are available in the MCP server context. This section covers **review-specific** tool rules only.

**Local + LSP review flow** (Local Mode / PR Mode when workspace IS the PR repo):
```
git diff → localSearchCode(pattern) → get lineHint → LSP tools → localGetFileContent (LAST)
```
- `localSearchCode` is ALWAYS the first step — it finds symbols and provides `lineHint` (1-indexed line number) required by ALL LSP tools.
- `lspCallHierarchy(incoming)` traces who calls a changed function. `lspFindReferences` finds all usages of a changed type/variable.
- `localGetFileContent` reads implementation — use ONLY as the final step after discovery.
- NEVER guess `lineHint` — ALWAYS get it from `localSearchCode` first.

**Shell Commands** (Local Mode only — git operations):

| Command | Purpose |
|---------|---------|
| `git status` | Identify staged, unstaged, and untracked files |
| `git diff` | Get unstaged working tree diff |
| `git diff --staged` (or `--cached`) | Get staged diff |
| `git diff HEAD` | Get combined staged + unstaged diff |
| `git log --oneline -10` | Recent commit context |
| `git branch --show-current` | Current branch name |

> Shell `git` commands are ONLY allowed for obtaining diffs and status. All code reading and search MUST use Octocode MCP `local*`/`lsp*` tools.

**Task Tracking**: Use the task/todo tracking tool available in your runtime to track review progress. Use `Task` to spawn parallel agents for independent research domains.

**Tool Selection Rules:**

| Review Mode | Primary Tools | Secondary Tools | FORBIDDEN |
|-------------|---------------|-----------------|-----------|
| **PR Mode** (workspace IS PR repo) | `local*` + `lsp*` | `github*` for PR metadata/diff | Shell for code reading |
| **PR Mode** (workspace is NOT PR repo) | `github*` only | `packageSearch` for external | `local*` or `lsp*` (wrong repo) |
| **Local Mode** | `local*` + `lsp*` + shell `git` | `packageSearch` for external deps | `github*` for code reading (not needed) |

**Tool Transition Matrix**:

| From | Need | Go To |
|------|------|-------|
| `githubSearchCode` | File content | `githubGetFileContent` |
| `githubSearchCode` | Package source | `packageSearch` |
| `githubSearchPullRequests` | File content | `githubGetFileContent` |
| `import` statement | External definition | `packageSearch` → `githubViewRepoStructure` |
| `localSearchCode` | Definition | `lspGotoDefinition` (with lineHint) |
| `localSearchCode` | All usages | `lspFindReferences` (with lineHint) |
| `localSearchCode` | Call chain | `lspCallHierarchy` (with lineHint) |
| `git diff` output | Deep analysis of changed code | `localSearchCode` → `lsp*` tools |
| `git status` output | Read changed file | `localGetFileContent` (with matchString) |
</tools>

---

## Flow Analysis Protocol

<flow_analysis_protocol>

> **Full recipes and detailed examples**: [references/flow-analysis-protocol.md](references/flow-analysis-protocol.md)

**Recipe Selection** (see references for full steps):

| Changed Code | Recipe | Key Tool |
|-------------|--------|----------|
| Function signature changed | Recipe 1 — incoming callers | `lspCallHierarchy(incoming)` |
| New function added | Recipe 2 — outgoing deps | `lspCallHierarchy(outgoing)` |
| Type/Interface changed | Recipe 3 — all usages | `lspFindReferences` |
| Data transformation changed | Recipe 4 — trace chain | Chain `lspCallHierarchy` hops |
| Function signature changed (remote) | Recipe 5 — remote callers | `githubSearchCode` + `githubGetFileContent` |
| Export changed | Recipe 6 — import chain | `githubSearchCode` for consumers |

</flow_analysis_protocol>

---

## Review Guidelines

Keep the base rule here and use detailed guidance from:
- [Review Guidelines Reference](references/review-guidelines.md)

<review_guidelines_base>
- Focus on CHANGED code first.
- Prioritize HIGH/MED confidence, actionable findings.
- Use structural tracing (imports/callers/consumers) before concluding impact.
</review_guidelines_base>

---

## Domain Reviewers

<domain_reviewers>

> **Full domain matrix with detection rules, priority levels, and skip criteria**: [references/domain-reviewers.md](references/domain-reviewers.md)

**Review Domains**: Bug, Architecture, Performance, Code Quality, Duplicate Code, Error Handling, Flow Impact

**Priority Rule**: HIGH confidence + NEW code ('+' prefix) + real problem + actionable fix = MUST include

**Global Exclusions (NEVER Suggest)**: Compiler/linter errors, unchanged code, test details, generated/vendor files, speculative scenarios, already-commented issues
</domain_reviewers>

---

## Execution Flow

<flow_overview>
```
                    ┌──────────────────────┐
                    │  REVIEW TARGET       │
                    │  DETECTION           │
                    └──────────┬───────────┘
                         ┌─────┴─────┐
                         ▼           ▼
                    PR Mode     Local Mode
                         └─────┬─────┘
                               ▼
Phase 1       Phase 2      Phase 3           Phase 4       Phase 5       Phase 6
GUIDELINES → CONTEXT → USER CHECKPOINT → ANALYSIS → FINALIZE → REPORT
    │            │            │                │           │          │
    ▼            ▼            ▼                ▼           ▼          ▼
 Ask user    PR: Fetch     Present &       Deep-dive    Dedupe &   Summary +
 for docs    PR + Comments Ask Focus       Research     Verify vs  Document
 & context   Local: git    (same for       (local* +    guidelines
             diff + status both modes)     lsp* tools)
```

| From → To | Trigger |
|-----------|---------|
| Target Detection → Pre-Flight | Review mode determined (PR or Local) |
| Pre-Flight → Phase 1 | MCP tools verified available |
| Phase 1 → Phase 2 | Guidelines context built (or skipped) |
| Phase 2 → Phase 3 | PR metadata + diff + comments fetched (PR Mode) OR git diff + status collected (Local Mode) |
| Phase 3 → Phase 4 | User provides focus direction |
| Phase 3 → Phase 6 | User says "just give me the summary" (Quick mode) |
| Phase 4 → Phase 5 | All domain analyses complete |
| Phase 5 → Phase 6 | Findings deduplicated + verified |
</flow_overview>

<key_principles>
- **Align**: Every tool call MUST support a hypothesis
- **Validate**: Real code only (not dead/test/deprecated). Check `updated` dates.
- **Links (PR Mode)**: MUST use full GitHub links for code references (https://github.com/{{OWNER}}/{{REPO}}/blob/{{BRANCH}}/{{PATH}}).
- **Links (Local Mode)**: Use `file:line` format for local code references.
- **Refine**: Weak reasoning? Change tool/query.
- **Efficiency**: Batch Octocode MCP queries (1-3 per call). Metadata before content.
- **Tasks**: MUST use the runtime's task/todo tracking tool to track progress for Full mode reviews.
- **FORBIDDEN**: Providing timing/duration estimates.
- **FORBIDDEN**: Referencing findings as `#1`, `#2`, `#N` — GitHub auto-links `#<number>` to issues/PRs.
</key_principles>

---

## Execution Lifecycle

Use detailed lifecycle instructions from:
- [Execution Lifecycle Reference](references/execution-lifecycle.md)

<execution_lifecycle_base>
### Base vs Optional (REQUIRED)
- **Base (in this SKILL):**
  - Target detection
  - Tooling model and selection rules
  - Flow analysis protocol
  - Phase 4 Analysis gate (core reasoning/execution)
- **Optional/Extended (in references):**
  - Full dependency gate details
  - Detailed phase playbooks (1, 2, 3, 5, 6)
  - Expanded verification checklist
</execution_lifecycle_base>

### Phase 4: Analysis

<analysis_gate>
**REQUIRED: Respect user direction from Phase 3 AND guidelines from Phase 1.**

### Pre-Conditions
- [ ] Phase 3 (User Checkpoint) completed
- [ ] User direction received (focus areas or "full review")
- [ ] Guidelines context available (or confirmed empty)

### Actions (REQUIRED — both PR Mode and Local Mode)

> **Tool selection by mode** (see Tool Selection Rules in Tools section):
> - **PR Mode** (workspace IS PR repo): `local*` + `lsp*` primary, `github*` for PR metadata/diff
> - **PR Mode** (workspace is NOT PR repo): `github*` only
> - **Local Mode**: `local*` + `lsp*` + shell `git` (requires `ENABLE_LOCAL=true` — see Target Detection)
> - **File Scope**: Same as Local Mode, but limit all analysis to the target file + its immediate dependency graph (1 hop)

1. **List 3-5 search queries** aligned with user focus, then execute each:
   ```
   Query 1: [tool] — [search pattern] — [goal]
   Query 2: [tool] — [search pattern] — [goal]
   ...
   ```
2. **Guidelines Compliance Check** (REQUIRED if guidelines were loaded in Phase 1):
   - For each changed file, check against loaded guidelines/conventions
   - MUST flag any violations of project-specific rules with reference to the specific guideline
3. **Flow Impact Analysis** (REQUIRED for function/method changes):
   - Apply the matching recipe from the Flow Analysis Protocol based on change type (see Flow Analysis Protocol section and [references/flow-analysis-protocol.md](references/flow-analysis-protocol.md))
   - MUST identify if return values, types, or side effects changed
   - MUST check if existing integrations will break
   - MUST document the blast radius: how many callers/consumers are affected
4. **Validate schemas/APIs/dependencies** using `matchString` targeting (PR Mode: `githubGetFileContent`; Local Mode: `localGetFileContent` + `localSearchCode`)
5. **Assess impact per domain** (prioritize user-specified areas from Phase 3):
   - **Architectural**: System structure, pattern alignment
   - **Integration**: Affected systems, integration patterns
   - **Risk**: Race conditions, performance, security
   - **Business**: User experience, metrics, operational costs
   - **Cascade Effect**: Could this lead to other problems?
6. **Identify edge cases** in changed logic
7. **Security scan**: injection, XSS, data exposure, regulatory compliance
8. **Scan for TODO/FIXME comments** in new code ('+' lines only)
9. **For high-risk changes**: Assess rollback strategy/feature flag needs
10. **Preflight suggestion** (Local Mode only): If changes are substantial, suggest running the project's test/lint suite before finalizing the review

### Gate Check
- [ ] All search queries executed
- [ ] Guidelines compliance checked (if guidelines loaded)
- [ ] Flow impact analyzed for all modified functions (using LSP in Local Mode)
- [ ] All user-specified focus areas covered
- [ ] Findings list compiled with confidence levels

### FORBIDDEN
- Analyzing areas user explicitly excluded in Phase 3
- Skipping flow impact analysis for function/method changes
- Ignoring guidelines loaded in Phase 1
- **Local Mode**: Using `github*` tools for code reading (MUST use `local*` + `lsp*`)
- **Local Mode**: Guessing `lineHint` without calling `localSearchCode` first
- **File Scope**: Expanding analysis beyond the target file + immediate dependencies without user request
- **File Scope**: Spawning parallel agents (single-pass review only)

### ALLOWED
- **PR Mode**: All Octocode MCP tools (github*, local*, lsp*)
- **Local Mode**: Octocode MCP `local*` + `lsp*` tools + shell `git` commands
- **Both**: Spawning parallel agents via `Task` for large change sets (see Multi-Agent section)

### On Failure
- **IF** search returns no results → **THEN** broaden query, try synonym, or change tool
- **IF** flow tracing hits dead end → **THEN** document limitation, proceed with available evidence
- **IF** LSP tool fails (Local Mode) → **THEN** fall back to `localSearchCode` pattern matching
</analysis_gate>

---

### Phase 5 + Phase 6 (Optional Detail)

Keep Finalize/Report details in the lifecycle reference to keep the base skill focused:
- [Execution Lifecycle Reference](references/execution-lifecycle.md)

Base expectation in this SKILL:
- After Phase 4, finalize only high-impact evidence-backed findings
- Present concise recommendation and ask before writing any review document

---

## Multi-Agent Parallelization & Swarm Strategy

<parallel_execution>

> **Full agent definitions, prompt templates, scaling rules, and merge protocol**: [references/parallel-agent-protocol.md](references/parallel-agent-protocol.md)

**Quick Rule**: ≤5 files = single-pass (no agents). >5 files in Full mode = MUST use parallel agents.

**Applies to BOTH PR Mode and Local Mode.** In Local Mode, agents use `local*` + `lsp*` tools exclusively (no `github*` for code reading).

**Agents** (spawn in Phase 4, ALL in a SINGLE message):
- **Agent A**: Flow Impact — traces callers/consumers of modified symbols (uses `lspCallHierarchy` + `lspFindReferences` in Local Mode)
- **Agent B**: Security & Error Handling — scans for vulnerabilities and swallowed exceptions
- **Agent C**: Architecture & Code Quality — patterns, coupling, performance
- **Agent D**: Guidelines & Duplicates — compliance + DRY (only if guidelines loaded)

**Scaling**: 2 agents (6-15 files) → 3 agents (16-30 files) → 4 agents (30+ files). See reference for full matrix.

**Merge**: Collect → Dedupe → Cross-check vs PR comments (PR Mode) or dedupe only (Local Mode) → Prioritize (Security > Bug > Flow > Arch > Perf > Quality) → Apply findings cap (see Execution Lifecycle Reference, Phase 5).

**FORBIDDEN**: Agents in Quick mode, >4 agents, sequential spawning, proceeding before ALL agents return.
</parallel_execution>

---

## Output Protocol

> **Full report template and format specification**: [references/output-template.md](references/output-template.md)

<output_structure>
**Template sections**: Executive Summary (goal, risk, recommendation) → Ratings (correctness, security, performance, maintainability) → PR/Changes Health → Guidelines Compliance → Issues (High/Medium/Low with `file:line` + diff fix) → Flow Impact Analysis

**Each finding MUST have**: Location (`file:line`), Confidence (HIGH/MED), Problem description, Code fix (diff format)

### Finding Labels
- **FORBIDDEN:** Using `#1`, `#2`, or any `#<number>` notation to label or reference findings anywhere in the output. GitHub auto-links `#N` to issues and pull requests, creating broken or misleading cross-links in PR comments.
- Use plain numbering (`1.`, `2.`), lettered labels (`A`, `B`), or descriptive category IDs (e.g., `[SEC-1]`, `[BUG-1]`, `[ARCH-1]`) instead.
- This applies to headings, inline references, summary lists, and any other mention of finding identifiers.
</output_structure>

---

## References

- **Flow Analysis**: [references/flow-analysis-protocol.md](references/flow-analysis-protocol.md) — Tracing recipes (6 recipes for local + remote)
- **Domain Reviewers**: [references/domain-reviewers.md](references/domain-reviewers.md) — Domain detection, priority matrix, exclusions
- **Dependency Check**: [references/dependency-check.md](references/dependency-check.md) — Full pre-flight gates and failure handling
- **Review Guidelines**: [references/review-guidelines.md](references/review-guidelines.md) — Confidence model and changed-code mindset
- **Execution Lifecycle**: [references/execution-lifecycle.md](references/execution-lifecycle.md) — Detailed Phase 1,2,3,5,6 playbooks
- **Verification Checklist**: [references/verification-checklist.md](references/verification-checklist.md) — Full delivery checklist
- **Parallel Agents**: [references/parallel-agent-protocol.md](references/parallel-agent-protocol.md) — Agent definitions, prompts, scaling, merge protocol
- **Output Template**: [references/output-template.md](references/output-template.md) — Report format and markdown template

---

## Verification Checklist

Use the full checklist from:
- [Verification Checklist Reference](references/verification-checklist.md)

<verification_base>
- [ ] Target/mode resolved (including file-scoped local checks when requested)
- [ ] Phase 4 analysis complete with evidence and confidence labels
- [ ] Findings are actionable, deduplicated, and scoped correctly
- [ ] No `#<number>` notation used in any finding label or reference
</verification_base>