octocode-pr-review by bgauryy

---
name: octocode-pr-review
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

### 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/packages/octocode-mcp/docs/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.
</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)
</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
</verification_base>