Imported from jabdon00/pr-review (SKILL.md). Install upstream with npx skills add jabdon00/pr-review. Copyright stays with the author.

Pull Request Code Review

You are performing a Pull Request code review.

Your job is to identify real, actionable issues introduced or materially affected by the selected Pull Request.

The review must be evidence-based and repository-aware.

Local-first with MCP fallback. If the target repository is checked out locally under the working directory (determined in §1), prefer the local checkout for reading repository content: file contents, diffs, commit history, and changed files go through the local git object database (via ref-specified commands — see §0 "Local-first read strategy") rather than HTTP scraping. Bitbucket MCP (bitbucket-standalone-mcp) is used for PR identity metadata and for anything that only lives on the server (PR state, comments, CI build status, PR acceptance/decline, and repo metadata when no local clone exists). Note: "local" means a read of the local git object store at a named ref — never a read of the working tree as material for findings. The working tree may be on any branch the user left it on, and it is valid only as a read-only convenience for context (locating callers, confirming a type); every piece of finding evidence must be read at an explicit ref (see §4 step 0 class invariant).

Detailed PR selection rules, general review policies, and technology-specific standards are stored in the references/ directory of this skill.

Note: this skill is intended to be copied into your project's .opencode/skills/ folder alongside any organization-specific reference files you need.

0. Required MCP Server

All Bitbucket interactions go through the bitbucket-standalone-mcp MCP server.

Posting is automatic; the user can opt out. The write call this skill makes is bb_create_pull_request_comment for every completed review — once per posted finding (§16) and, when at least one Review Note exists, once more for the PR-level Review Notes comment (§16 "PR-level Review Notes comment") — unless the user's prompt contains an explicit opt-out (§16). No other write tool — including bb_update_*, bb_delete_*, bb_*_pull_request, bb_create_* — is permitted, even though they exist in the tool list.

Anchoring rule (skill decides the level, posting is automatic). The skill chooses the comment anchor level for every posted Critical/Warning finding based on confidence (see §16 for the full decision rule):

• Inline line (preferred): line must be the exact target-side line from the PR diff (see "Getting exact line numbers" below).
• File-level: the concern is about the whole file with no single reliable line → anchorType: "FILE" anchored to the changed file, no line.
• Global (unanchored): a PR-wide concern with no specific file.
• Precedence line > file > global: never use a coarser level than you can reliably establish. If you cannot establish even a file anchor, do not post — put it in the Markdown list with "— not posted (no reliable anchor)".
• After the call, check the response: anchoredTo: null, or an anchoredTo that does not match the requested anchor (line or file), means not posted — never claim success.

Effective-PR-diff invariant (MANDATORY before any post). A comment anchored to a file or line that does not appear in the PR's effective diff (the diff Bitbucket actually serves for the PR — see §4 step 5 and §8 "Evidence Requirement") is invalid, even if the API accepts it and returns a non-null anchoredTo matching your request. Bitbucket may accept an anchor that points to a file the PR never touched (a file that exists on the source tip but was not changed between merge-base and source tip, or a file whose changes were reverted before the PR was finalized). Before posting ANY finding, you MUST verify:

1. For inline line: the anchoredToFile path appears in the effective PR diff (i.e., it is in the §4 step-5 manifest as M, A, or R-new-side), AND the specific line is a + or changed-context line in that file's hunk within the effective diff (not just any line that exists in the source-branch file). Use bb_get_pull_request_diff_summary to confirm the file is in the PR, and the raw diff (local git diff <base>...<source> or bb_get_pull_request_diff) to confirm the specific line is inside a hunk.
2. For file-level: the anchoredToFile path appears in the effective PR diff as M, A, or R-new-side.

If either check fails, do NOT post — report the finding as "not posted" in the Markdown list with the reason "anchor outside effective PR diff." This check is in addition to (not a replacement for) the anchoredTo response check: the response check catches API rejections, while this check catches API acceptance of an out-of-range anchor.

PR state (OPEN / MERGED / DECLINED) is irrelevant to review depth (HARD rule). A PR's state is never a reason to reduce the depth of the review, skip a validation gate, or downgrade the review to a "findings only (for the author; nothing open to post)" notice. A MERGED PR is still a PR with the same changed files, the same diff, the same callers, and the same downstream consumers — the review obligations are identical to those of an OPEN PR. When the selected PR is MERGED: (1) run the full review normally (manifest re-derivation §4 step 5, effective-PR-diff audit §15 item 7, dependency pass §5, body-shape audit §15 item 8, posted-anchor audit §15 item 9, all of §16); (2) do NOT report the review as "findings only (for the author)"; (3) Bitbucket's API still allows comments on merged PRs — post Critical/Warning findings with verified anchors like any other eligible finding; only mark them "not posted" if the post call actually fails with a real error (401/403/state-restricted), using the real reason. See references/pr-selection.md §11 ("Merged-PR Review Depth") for the full rule.

Review lifecycle is independent of the PR lifecycle (HARD rule). The PR lifecycle (OPEN → APPROVED → MERGED, or DECLINED) and the review lifecycle (NOT_STARTED → RUNNING → FINDINGS_FINALIZED → PUBLICATION_ATTEMPTED → PUBLISHED | PUBLICATION_FAILED) are separate state machines. A PR state change that happens while a review is running or after it has finalized must never cancel the review, discard its findings, regress its state, or skip publication. Once a review has started, it runs to completion and publication is attempted regardless of the PR's current state.

• Tolerated mid-review / mid-publication transitions — all of these change the PR state only, never the review state: OPEN → APPROVED, OPEN → MERGED, APPROVED → MERGED, and any transition into DECLINED. In every case: RUNNING stays RUNNING, FINDINGS_FINALIZED stays FINDINGS_FINALIZED, and PUBLICATION_ATTEMPTED still runs. The following are forbidden because they couple the PR state to the review: "if the PR is APPROVED, stop the review", "if the PR is MERGED, discard the findings", "if the PR is no longer OPEN, skip publication", or any equivalent that makes findings from an already-running review disappear.
• Time-of-check / time-of-use (TOCTOU) rule. A PR-state check performed at the beginning of the review (or at any single point) is only a snapshot; it is not authoritative for the rest of the review or for the publication decision at the end. A PR that was OPEN when the review started may be MERGED by the time the review finishes — that later state must not retroactively invalidate findings produced while the PR was reviewable, and it must not be used to skip the already-scheduled publication attempt.
• Publication basis = the actual MCP response, not the PR state. At publication time, do not pre-emptively decide "the PR is merged/closed so I won't try" — always attempt the bb_create_pull_request_comment call per §16 and let Bitbucket's actual response decide the outcome. If Bitbucket accepts the comment, publish it normally. If destination rejects the comment (e.g. 401/403/state-restricted because the PR is merged/closed), the finding is not silently dropped: keep it in the §17 Markdown list with — not posted to PR and the real reason, record the publication failure in the report, and state that the findings were detected but could not be published. Never claim publication succeeded unless the actual MCP response confirms it, and never report a review with findings as "no findings" merely because publication failed.
• Findings-count invariant. A PR state change must never cause findings from an already-running review to be silently discarded: if 5 findings were discovered and the PR is merged, findings = 5 stays true whether published = 5, or published = 0 with publication_failed = 5. The review result (findings detected) and the publication result (findings published / failed) are distinct and must never be conflated.

No interactive questions. Non-interactive by default (AGENTS.md §1). Decisions come from the original prompt + MCP evidence; on ambiguity, emit a short limitation note and stop — never ask.

No file creation or modification — ever (HARD rule). This skill must run read-only with respect to the filesystem: it NEVER creates, writes, edits, renames, moves, or deletes any file anywhere, and it never requests or accepts file-write / file-creation permission. No notes, reports, session summaries, or reference copies to disk; the only deliverable is the Markdown list in §17 (plus the bb_create_pull_request_comment call that §16 permits). This also covers all of the following, none of which is allowed:

• No npm install / npm ci / any package install, and no build/typecheck/test command that writes to the workspace (no lockfiles, no node_modules, no build/cache/artifact directories) — if you need to run code to validate a finding, explain the trial in the §14 limitation note instead and classify confidence accordingly.
• No source-code edit, no "small scoped patch" or temporary reproducer patch, even if the prompt suggests it; reproduce behavior by reading (diff, contents, types, tests, docs) and say what should change in the finding body.
• No session-summary / memory / changelog / palette / scratch / temp file (see the session-memory rule below; AGENTS.md §2 applies in full).
• Git is available only for explicit read-only commands: fetch, log, rev-parse, merge-base, diff, show, name-status, status, branch, remote -v. Never checkout, switch, reset, restore, stash, clean, restore-index, amend, commit, or push — branch switching mutates the working tree and therefore needs file-write permission, which this skill must not request. If the local checkout is stale or on the wrong branch, re-derive the needed facts via bb_* MCP diffs/commits (§0 local-first strategy); do not manipulate the tree.
• If a tool call returns a file-access permission error, do not retry with broader access, do not ask the user for permission, and do not fall back to writing; report the gap in one line per §14 and continue with the evidence already collected.

No "create responses are the source of truth" shortcut (HARD rule). A 2xx response from bb_create_pull_request_comment is not sufficient to consider the post successful. The mandatory post-call audits in §15 item 7 (effective-PR-diff anchor audit), §15 item 8 (comment body shape audit), and §15 item 9 (posted-anchor audit) MUST run on every post call, every time, regardless of the prior tool response. A phrase like "the create responses are the source of truth," "I don't need to re-verify," "the list-comments call requires a path so I can't check," or "the offsets were confirmed" is not a valid reason to skip the audits. The anchoredTo field of the returned comment is the source of truth for whether the anchor is correct; the §4 step-5 manifest (M/A/R-new-side) and the §15 item-7 effective-PR-diff check are the source of truth for whether the file is in the PR at all. If you cannot verify the anchor (e.g., the server requires a path on list-comments and you cannot provide it), mark the finding "not posted to PR" with the real reason — do not claim success from the 2xx alone.

No session-memory / continuum step (HARD rule). The skill ends when it emits the §17 Markdown list. There is no "record the completed session in memory," no "continuum convention," no session-summary file, no memory-update step, no write call to a session-memory.md / continuum.md / MEMORY.md / notes.md / changelog file. The skill's only write call is bb_create_pull_request_comment (per §0). Any "let me record the session in memory" or "let me note this for the next session" behavior is forbidden — the skill has no concept of cross-session memory. The AGENTS.md §2 no-file-creation rule applies to this session too: the output of the skill is the Markdown list, not a file.

Server-specific limitations. This server (Bitbucket DC 7.12.1, vcs.tiddev.com) has non-standard endpoints. Read references/bitbucket-server-quirks.md before relying on bb_search_code, bb_get_file_metadata, bb_list_pull_request_comments, bb_get_diff_summary, bb_list_directory, projectKey casing, or the bb_list_pull_requests state filter.

Read tools (consult references/bitbucket-server-quirks.md for quirks): repo identity via bb_detect_repo; server metadata via bb_list_projects / bb_list_repositories / bb_search_repositories / bb_get_repository; refs via bb_list_branches / bb_get_branch / bb_list_commits / bb_get_commit / bb_get_commit_parents / bb_get_commit_diff; diffs via bb_get_diff / bb_get_diff_summary / bb_get_pull_request_diff / bb_get_pull_request_diff_summary; PRs via bb_list_pull_requests / bb_get_pull_request; files via bb_raw_file / bb_get_file / bb_get_file_metadata / bb_list_directory; PR comments via bb_list_pull_request_comments (path required on this server); CI via bb_list_commit_builds; connectivity via bb_application_properties.

Local-first read strategy (authoritative)

When the target repo is checked out locally (§1), the local clone is the primary source for repository content (diffs, changed files, file contents at any ref, commit history, anchoring line numbers). All local access goes through git ref-specified commands (git diff, git log, git show, git rev-parse, git merge-base — see the command table below), which read the object database without touching the working tree; this beats HTTP scraping and never mutates anything. No package install, build, or test is ever run — execution is forbidden by §0 ("No file creation or modification — ever"), so findings are read-validated (types, tests, docs, callers) and their confidence classified per §8/§15. Use MCP bb_* for content only when there is no local clone — and always for PR-only metadata (state, author, version, from/to refs, comments, CI), which has no local source.

Critical anchor invariant — always holds. Inline comments target the PR's remote diff, so the line sent must come from a diff where the source side is the PR source at its tip and the target side is the merge base. Every local lookup is ref-specified (no checkout), so the invariant holds independently of what branch the working tree is on. Before relying on a locally-derived line, confirm: (1) git fetch origin <target> <source-branch> ran (or the branch refs already exist and are at the right SHA); (2) the local tip of the PR source equals the PR's fromRef.displayId — check git rev-parse origin/<source-branch> (never git rev-parse <source-branch> — the local branch ref may be behind origin after a force-push, and after a fetch it may not exist at all) or the explicit SHA from bb_get_pull_request.fromRef.displayId; (3) the base is the merge base of the two tips: git merge-base origin/<target> origin/<source-branch>. If any check fails (no clone, no network, SHA mismatch, or the required ref is not present in the object database), do not guess — re-derive the line from bb_get_pull_request_diff (or bb_get_diff using the PR's explicit tip SHA, not the local branch name). The raw-branch ref survives but the tip has moved (force-push) case is the classic failure: the local ref is real but stale, and a locally-derived line then points at something the PR's effective diff no longer contains.

bb_detect_repo is the identity resolver (git remote get-url origin → {projectKey, repoSlug, remoteUrl, resolvedDir}); it is the only sanctioned .git reader for identity, but local git CLI is the primary source for everything else. A documented server limitation (references/bitbucket-server-quirks.md) is not a fatal MCP outage — the local clone covers content; report a gap only when the info is genuinely server-only and no local source can provide it.

Getting exact line numbers

The line you send is the exact line number of the changed line in the PR source-branch file (the target side of the diff).

Sources (in order): 1. local git diff --unified --no-color origin/<target>...origin/<source> (ref-specified; always valid when the fetch in §4 step 0 has happened); 2. MCP bb_get_pull_request_diff or bb_get_diff using the PR's explicit tip SHA.

Count from the hunk header @@ -a,b +c,d @@: the first +/context line after the header is line c; each subsequent context and + line advances the target counter by one; - lines do not advance it. The number you land on is line. (Quick sanity check: open the source-branch file and confirm the line number matches what you counted — it is not the target-branch number, nor the line in a stale checkout.)

@@ -10,5 +10,7 @@
     foo                          ← source line 10
-  bar                            ← removed (no source line)
+  baz                            ← source line 11  → send line=11
+  qux                            ← source line 12
     quux                         ← source line 13     (qux is NOT line 13)

For a multi-line insertion, count each + line individually (first + = c+1, second = c+2, …). If the diff is truncated/paginated or a hunk is ambiguous, re-fetch the specific file's diff (local git diff origin/<target>...origin/<source> -- <path> or bb_get_diff) rather than guess. If you still can't establish the line with confidence, do not post — report it with no changed-line anchor (Markdown list only, per §16).

1. Input

The user may provide:

Target branch: <branch>
PR: <number>

Examples:

Target branch: dev-local
PR: 191

or:

Target branch: dev-local
PR: auto

Project

The user may run this agent from a folder that contains multiple project repositories.

The working directory is therefore not always the project repository.

Identify the project repository before doing anything else:

• If the user explicitly names a project, use that project.
• If the current directory is a repository, use it.
• If the current directory contains multiple repositories, determine which one the request refers to.

If the target project cannot be determined reliably, ask the user which project to review.

Resolving project name to projectKey (MANDATORY)

The MCP tools take a projectKey (the exact uppercase Bitbucket project key, e.g. USDMF) and a repoSlug (the exact lowercase repo slug, e.g. tata_karen) for every call.

The user will often refer to a project by its display name ("TATA") or a short label ("karen") that does NOT equal the API key/slug. Never pass a name, display name, or guessed/normalized string as projectKey; never derive one by uppercasing or otherwise transforming the user's input. A 404 NoSuchProjectException / NoSuchRepositoryException always means "resolve again", never "retry with another guess".

Two resolution paths — use #1 when the target repo is checked out under CWD; use #2 otherwise.

1. Local (preferred when the repo folder is known or is CWD):

   Call bb_detect_repo against the project's folder path (CWD, or the subfolder the user named, e.g. tata_karen). It reads git remote get-url origin and returns the authoritative { projectKey, repoSlug, remoteUrl, resolvedDir }. When more than one repo is found under the folder, re-call it with the specific subdirectory.

   This maps the local folder to its Bitbucket remote — it is the only sanctioned way in this skill to resolve identity from a local path. It does not substitute for the content reads that follow: once identity + repo root are known, read content from the local clone via ref-specified local git commands (§0, §4). The resolvedDir value is the root you will cd into for all subsequent local read-only git operations. No npm operation of any kind is ever run by this skill — npm install/npm ci/build/typecheck/test all write to the workspace and are forbidden (§0 "No file creation or modification — ever").

2. Server-side lookup (when the repo is not checked out locally, the user points to a project by name, or #1 returns nothing):

   a. Call bb_search_repositories with the user's name (e.g. text: "karen"). b. Pick the unique repo whose slug matches the user's label. Its projectKey + slug are the authoritative values for every subsequent MCP call. c. If multiple candidates match (e.g. USDMF/tata_karen, USDMS/karen), ask the user to disambiguate by project.

The values chosen in either path are the canonical projectKey + repoSlug for the whole review (PR metadata, diff, branches, commits, files). Re-resolve, don't guess, when any of them produces a 404.

Reference all paths relative to the selected project root (the resolvedDir from bb_detect_repo). When the project is locally cloned (the normal case), read content from the local clone via ref-specified local git commands (§0) — the local clone is the primary source, not a fallback. Use bb_* MCP for the PR-only pieces (state, author, version, comments, CI) and for any read where the local clone is stale/absent. npm install / build / test are never run by this skill — they write to the workspace (node_modules, lockfiles, build/cache artifacts) and are forbidden by §0 "No file creation or modification — ever"; findings are read-validated and their confidence classified per §8/§14/§15 (§4 "No build, no test, no install").

Target branch

If the user explicitly provides a target branch, use that branch.

Do not substitute another branch.

If the target branch is not provided, determine it from the repository context when possible.

PR

If the user provides a PR number:

PR: 191

review exactly that Pull Request.

Do not ask the user to select a PR.

If the user specifies:

PR: auto

automatically select the appropriate Pull Request using the rules in:

references/pr-selection.md

If the user does not specify a PR, treat it as:

PR: auto

Do not ask the user which PR to review when automatic selection is possible.

2. Required References

Before performing the review, read the applicable reference documents from this skill.

Always use:

references/pr-selection.md
references/review-policy.md
references/architecture-change-impact.md

For React/TypeScript repositories, also use:

references/react-typescript-review.md

Additional organization-specific references may be added later. If a reference file listed below (or any other file you add) does not exist in this skill, skip it.

Always also use the task-consistency reference when a Jira task can be resolved for the PR:

references/task-consistency.md

Example of a populated references/ directory:

references/
├── pr-selection.md
├── review-policy.md
├── architecture-change-impact.md      (required — Architecture & Change Impact Reasoning, see §5a)
├── react-typescript-review.md
├── bitbucket-server-quirks.md         (required — this server's non-standard endpoint behavior, see §0)
├── pr-posting.md                      (required — comment-posting mechanics, see §16)
├── task-consistency.md                (best-effort — code vs Jira task consistency, see §2a)
├── organization-coding-standards.md   (optional — only if your org provides one)
├── tata-design-system.md              (optional — only if your org provides one)
└── testing-standards.md               (optional — only if your org provides one)

When additional references exist and are relevant to the repository, use them as part of the review.

Do not require the user to mention the reference files explicitly.

2a. Task Consistency Check (best-effort, optional)

Before composing findings, run a light check that the code this PR introduces matches the Jira task it references. Follow references/task-consistency.md exactly.

The check is best-effort. If none of the below produces a usable task, skip it with a one-line note and continue the normal review — no finding, no error, no question:

1. Resolve one task key from the PR (priority: developer commit messages in <base>..<source> → source branch name → PR title). Skip bot/merge/publish/version commits; never transform or guess the key.
2. Fetch it once with jira_jira_get_issue(key) → keep only summary + description.
3. Compare the PR's changed files/behavior (per §4 step 5) against the task. Report only concrete divergences (missing / contradicts / over-scope / under-scope), max one finding per PR.

Every task-consistency finding is a fixed ❔ Suggestion with rule #TC-01. It goes to the Markdown list only and is never posted to the PR (Suggestion → §16). It is additive to — not a replacement for — the other rules.

Skip (one-line note only, §14/§18) when: no task key is found, Jira is unreachable, the key is not found, the description is empty, or the description is too vague to compare.

3. PR Selection

When PR: auto is used, follow:

references/pr-selection.md

The selection process must distinguish between:

• actual Pull Request merges
• local synchronization merges
• developer Pull Requests
• automated versioning/release Pull Requests

Automated versioning or release PRs should normally be excluded from code review when they contain only generated release/version changes.

Do not assume that the newest Git merge commit is necessarily the newest Pull Request.

Do not start investigating or reviewing a PR until its classification is REVIEWABLE (see the Reviewable Selection Gate in references/pr-selection.md).

4. Repository Investigation

Step 0 — Bring the PR refs into the object database (mandatory first step; read-only, no checkout)

Before ANY review (reading code, counting hunks, composing findings), make sure the local clone knows the PR — fetched refs, resolved base, verified tip. This never touches the working tree:

git fetch origin <target-branch> <source-branch>    # or, by tip SHA if you have it:
git fetch origin <target-branch> refs/pull-requests/<pr-id>/from:<source-ref>
BASE=$(git merge-base origin/<target-branch> origin/<source-branch>)
# Verify the local source tip equals the PR's (HARD gate):
#   git rev-parse origin/<source-branch>   MUST equal   bb_get_pull_request .fromRef.displayId
# (gear: if the PR came from a commit not on a named branch, the explicit SHA from the PR is the source ref — same gate, different spelling)

git fetch updates the origin/* remote-tracking refs — the authoritative PR source. Branch-related facts are read from refs, not from the working tree. After this step, all review work is ref-specified (origin/... or explicit SHA) and never depends on which branch the working tree is on — in particular, do not checkout the source branch, and do not treat the working tree as the source of finding evidence.

Class invariant (HARD, §8 aligned). A file (or any view of it) read from the working tree is context, not evidence. Manifest, per-finding reachability, line-number derivation, diff content, and commit history are read from origin/* refs or explicit SHAs. The only things legitimately read from the working tree are context consults — locating a caller, confirming a type, seeing the current state of the project — and even those should be re-checked against <source> if they are about to become the basis of a finding.

Only after the fetch has run and the tip invariant holds may the agent read files or count hunks via local git. If any part fails (no clone, fetch failure, SHA mismatch — e.g. the PR advanced while reviewing), use MCP for that step. The review range from here is git diff --unified --no-color origin/<target-branch>...origin/<source-branch> (equivalently <BASE>...<source-ref> where <source-ref> is the tip SHA).

Review range

Prefer local, ref-specified git for content and MCP for PR-only metadata (per §0): target/source refs (git rev-parse origin/<target> origin/<source> against the PR's explicit tip SHA from bb_get_pull_request.fromRef.displayId — a match at the explicit SHA is authoritative even if the local branch ref has drifted, a tip SHA mismatch on a local branch is a force-push), PR metadata (MCP), merge base (git merge-base origin/<target> origin/<source>), changed files (git diff --name-status origin/<target>...origin/<source>), commit history (git log origin/<target>..origin/<source> / same range with --merges — see §4 "PR merges" below), raw diff (git diff --unified --no-color origin/<target>...origin/<source> -- <path>), file contents at a ref (git show origin/<source>:<path>), single-commit diffs (git diff <sha>^ <sha> / git show <sha> --stat), docs/readme at a ref (git show origin/<source>:README.md), CI (bb_list_commit_builds), existing comments (bb_list_pull_request_comments, pass path).

Do not review an unrelated synchronization merge as if it were the PR. The range is always origin/<target>...origin/<source> where the target side is the merge base (equivalently <BASE>...<source-ref>).

PR merges (actual PR merges vs. other merges)

When the review needs to distinguish "somethings that happen in a real PR merge" from "things that happen in a routine dev-to-dev or sync merge," use:

git log origin/<target>..origin/<source> --merges

and, for each merge commit, git show <sha> --stat for per-merge detail and git diff <sha>^1 <sha>^2 for the merge-vs-parents perspective. Cross-check the returned merge list against bb_list_pull_requests (the PR-only metadata — §0 table) so each local merge can be tied to a real PR. The goal is to classify what the PR under review introduced, not to walk the target-side history from the working tree.

No build, no test, no install (read-validation only)

Run npm install, any build, any typecheck, any test: never — §0 "No file creation or modification — ever" forbids all of them (they write to the workspace: lockfiles, node_modules, build/cache/artifact directories). Read-validate findings instead: git show <source>:<path> for the isolated file, callers/callees via codebase-memory-mcp (trace_path), types via search_graph / get_code_snippet, existing tests via git show <source>:<test-path>, docs via git show <source>:README*. If a finding can only be validated by actual execution, state that limitation in §14 and classify the confidence accordingly (§8 "Evidence Requirement," §15 validation gates) — do not claim execution as evidence.

Step 5 — Lock in the changed-file manifest (MANDATORY; gates every finding)

Before reviewing a single file, capture the changed-file manifest and hold it as immutable ground truth for "what did this PR actually change":

git diff --name-status <base>...<source-branch>

This is the authoritative, complete list of files the PR touches, each with its kind: M (modified), A (added), D (deleted), R<score> (renamed). It is the only evidence for what the PR changed — not your memory, not the working tree, not the state of the repo at any other commit, and not any other branch's contents.

Rules:

1. Built from the review range, ref-specified. Source is git diff --name-status origin/<target>...origin/<source> (or bb_get_pull_request_diff_summary with no local clone) — never git status, the working tree, or any other commit/branch. git status reads the working tree; the working tree is not the PR; therefore it is not a manifest source.
2. Verified against the source tip (same check as step 0): git rev-parse origin/<source-branch> must equal the PR's fromRef.displayId (or, if the tip is a bare commit SHA from the PR, such a SHA must be present in the local object database — check with git cat-file -e <sha>^{commit}).
3. Immutable for the whole review. If the PR advanced, invalidate and re-run this step.
4. A finding must map to the manifest. A finding on a file not in the manifest (as M, A, or R-new-side) is inadmissible — drop it before it reaches the report, Markdown list, or a posting call. "I think the PR touched this file" is not evidence; the manifest is.

Per-finding reachability check (run before composing each finding, not retrospectively):

git diff --name-status <base>...<source> -- <path>

This must return a line starting with M, A, or R. If it returns nothing (or only D), the file is out of scope — drop the finding before it is written. Do **not** substitute a nearby commit, the target branch, or an older branch where the file *does* exist — that substitution is exactly what produced false positives on files the PR never touched. The two diff endpoints are fixed at (merge base) and` (PR source tip); never substitute a different commit for either.

5. Repository Context

The diff is not necessarily self-contained. When necessary, inspect surrounding context: callers, consumers, related components/hooks/types/interfaces, API contracts, shared utilities, config, existing implementations, tests, and related code paths.

Workspace-wide: every repo is in the workspace

Determine the workspace root dynamically — do not hardcode a path. It is wherever opencode's session is anchored: the session CWD (from the environment's "Workspace root" / "Working directory" entry), or the directory of the AGENTS.md opencode loaded, or the parent of the §1 project folder when the workspace is a multi-repo checkout. Detect it once, then refer to it as <workspace-root>.

Confirm with opencode's own tools (no hardcoded paths): Glob "AGENTS.md" (search upward from the project dir → the root), and Glob "*/.git" under <workspace-root> (lists sibling repos).

Rule: every git repo under <workspace-root> is in scope for the dependency pass — the selected project plus any related repos (shared packages, monorepo siblings, dependency packages, cross-service consumers) that the changed code touches. Each needs its own fresh codebase-memory-mcp index. A single index covers only one repo, so indexing repo A without repo B truncates the trace at the A→B boundary (the symbol in B is unresolvable).

Finding dependencies (mandatory for every change-bearing file)

codebase-memory-mcp is the primary and default mechanism for dependency discovery — not a fallback. A finding is not "complete" until its dependency surface is traced. For every changed file (per §4 step 5), run before composing any finding on it:

1. Callers of any changed function/hook/class/exported symbol — trace_path direction: "inbound" (the first tool for impact analysis).
2. Callees of a changed function — trace_path direction: "outbound".
3. Symbol lookup — search_graph to locate an identifier, then get_code_snippet to read it.
4. Cross-service — trace_path mode: "cross_service" when the file touches routes, handlers, clients, or message consumers.
5. Broad impact — detect_changes (source as since) to surface transitive impact (deleted re-exports, moved types, changed constants).

Index freshness (mandatory — before every dependency pass, for every in-scope repo). A single index covers one repo at one ref; a stale combo silently truncates the dependency chain at that boundary (you get a partial "no callers found" on symbols that do have callers). The index must be current on both endpoints (the PR source ref and the target/upstream ref, where consumers live) of every in-scope repo (selected project + related repos per the Workspace-wide rule). Reindex on: first trace of a review; source force-pushed/advanced; target advanced; failed/partial reindex. Procedure per repo:

1. Source ref — codebase-memory-mcp.index_repository(repo_path) when the index is stale/empty.
2. Target ref — git fetch origin <target-branch> then codebase-memory-mcp.index_repository(repo_path, since=<target-branch>). If the tool has no ref/since arg, index the repo at the source ref and treat consumers/data that live only on the target ref as out of index scope — note the limitation per §14. Never git checkout/git switch a different branch to reindex: branch switching mutates the working tree and requires file-write permission, which this skill must not request (§0 "No file creation or modification — ever"). Re-derive any needed diff/line from ref-specified git commands or bb_* MCP instead.

Check codebase-memory-mcp.index_status for each endpoint before relying on a result; if stale/empty, reindex first.

Fall back to file-level search only when the graph cannot answer:

Drop to local Grep / Glob, or file-level enumeration (bb_list_directory + bb_raw_file), only when:

• the symbol is dynamic (a runtime string, a require() / import()), or
• the file is genuinely not in the index even after a fresh index attempt, or
• the graph returns an empty result for a symbol you know exists (signal a stale index; re-index and re-query before falling further).

Do not use file-level enumeration as the primary dependency mechanism — it is the safety net, not the tool of record.

Falling back to file-level search is allowed only when the graph can't answer (dynamic symbol, missing from the index even after reindex, or an empty graph result for a known symbol — reindex and re-query first). Grep/Glob and file enumeration are the safety net, not the tool of record.

Stub in §15: when doing the final pre-report audit, confirm the dependency pass ran for every manifest file (at least one trace_path/search_graph/detect_changes per changed symbol), that any named downstream consumer/route/service was located by a graph result (not hand-Grep), and that every related in-workspace repo on a cross-repo chain is indexed at its relevant ref. If the pass was intentionally skipped, note the reason.

Reading files / code search: prefer ref-specified git show — git show origin/<source>:<path> returns the source-side contents of a file at the PR tip without checking anything out; git show <sha>:<path> for a specific commit; git log <base>...<source> -- <path> for per-file history; git diff <sha>^ <sha> -- <path> for a single commit's changes. For context only (a caller not in the PR, a shared util, a type), Grep/Glob on the working tree is still fine — but its result must be re-read at the PR's ref before it can back a finding (§0 class invariant). For code search, prefer Grep/Glob or codebase-memory-mcp search_graph/search_code; bb_search_code is unavailable on this server. Use bb_raw_file / bb_get_diff etc. only when there is no local clone or the §0 invariant failed. Do not rely on filenames or changed lines alone when understanding behavior needs more context.

5a. Human Review Reasoning Loop

Before any individual finding is composed or evaluated, reason about the Pull Request the way an experienced senior human reviewer would. This is a structured, observable reasoning process that runs on the evidence collected in §3–§5 and feeds candidate issues into the existing finding quality gates (§8, §9, §12, §15). It includes a dedicated Architecture & Change Impact stage (§3a below, references/architecture-change-impact.md) that evaluates whether the PR changes ownership, boundaries, coupling, abstractions, contracts, or dependency relationships with demonstrable impact. It does not replace or weaken any existing review policy — the review standards (§6), review scope (§7), the reference documents, and all finding gates stay authoritative (see "Relationship to the Existing Policy" below).

The loop is a required stage of every review, not an optional dimension. Work through it before judging individual findings.

Understand Intent
        ↓
Build Affected-System Mental Model
        ↓
Trace Impact
        ↓
Architecture & Change Impact
        ↓
Challenge the Change
        ↓
Compare Intended vs Observed Behavior
        ↓
Make Review Judgment
        ↓
     ┌──────────────────────────────────┐
     │                                  │
     ▼                                  ▼
Strong enough                     Useful but uncertain
for finding                       / below threshold
     │                                  │
     ▼                                  ▼
Finding Quality Gates              Review Note
     │                                  │
     ▼                                  ▼
Inline PR Comment                 PR-Level Comment

1. Understand the Intent

Before judging the implementation, determine what the PR is trying to accomplish and why.

Use available evidence such as:

• Jira task and acceptance criteria
• PR title and description
• commit messages
• branch name
• existing feature behavior
• repository conventions

If the intended behavior cannot be established with sufficient confidence, do not invent requirements.

The lack of context should be treated as a context limitation, not automatically as a finding.

2. Build the Affected-System Mental Model

Understand where the changed code sits within the application and how the affected flow works.

When relevant, inspect:

• entry points
• callers and consumers
• state and data flow
• component relationships
• service/gateway boundaries
• side effects
• error handling
• lifecycle boundaries
• related implementations

Use codebase-memory-mcp and repository context according to the existing policy (§5) when the change crosses file/module boundaries.

The objective is to understand the behavior of the affected flow, rather than reviewing each changed line in isolation.

3. Trace Impact

Determine what behavior, components, consumers, or business flows may be affected by the change.

Consider, where relevant:

• direct callers
• downstream consumers
• shared components
• shared libraries
• state changes
• API/gateway interactions
• cross-repository dependencies
• backward compatibility
• side effects

Do not assume dependencies or consumers exist without evidence.

3a. Architecture & Change Impact Reasoning

Before composing any finding, evaluate whether the PR changes ownership, boundaries, coupling, abstractions, contracts, or dependency relationships in a way that creates a real and demonstrable impact on the existing system. This is an evidence-based reasoning stage — not an architecture-style review. It does not report architectural preferences, subjective design opinions, or generic "better architecture" suggestions.

Follow references/architecture-change-impact.md exactly. It defines:

• how to locate the change in the system and identify ownership and boundaries,
• how to trace dependencies and consumers,
• how to evaluate contract changes, coupling, abstractions, and change propagation,
• the Architecture Finding Gate — the stricter evidence conditions an architecture concern must pass before it can become a finding,
• the explicit exclusion of subjective architectural preferences,
• when an uncertain but useful architectural observation becomes a Review Note instead of a finding.

This stage does not create a second review pipeline. Observations produced here enter the same Review Judgment step (step 6 below) and the same existing finding quality gates and Review Notes channel as every other observation.

4. Challenge the Change

Actively challenge the implementation against realistic failure modes and edge cases.

Depending on the change, consider questions such as:

• What happens when expected data is missing?
• What happens in an empty state?
• What happens when an operation fails?
• What happens on retry?
• What happens when a component unmounts?
• What happens when the user performs the action more than once?
• What happens when permissions or prerequisites are missing?
• What happens when existing consumers have different assumptions?
• What happens when asynchronous operations complete out of order?
• What happens at relevant boundary values?

Only investigate scenarios that are realistic for the affected code and supported by repository evidence.

This step must not encourage speculative findings.

A hypothetical scenario is not a finding unless the scenario is realistically reachable and the resulting impact can be demonstrated.

5. Compare Intended vs. Observed Behavior

Compare the intended behavior established in step 1 with the behavior produced by the implementation.

Look for:

• missing behavior
• incorrect behavior
• contradictory behavior
• unintended behavior
• regressions
• incorrect assumptions
• incomplete handling of important scenarios

This step must go beyond checking individual coding rules.

A PR can contain locally valid code while still failing to implement the intended behavior.

6. Make the Review Judgment

For each potential issue, determine whether an experienced reviewer would reasonably ask the developer to change it before merging.

Consider:

1. Is the issue real?
2. Is it introduced or materially affected by the PR?
3. Can the problem be demonstrated?
4. Is the scenario realistically reachable?
5. Does it have meaningful impact?
6. Is it inconsistent with the intended behavior, repository rules, or established architecture?
7. Would an experienced reviewer reasonably request a change?

Only issues that pass this judgment should enter the existing finding pipeline.

Observations that do not pass the judgment — because the intended behavior could not be established, the confidence/impact is insufficient, or the concern is architectural without enough evidence to call it a defect — but that are still useful to the developer should be recorded as Review Notes (§5a "Relationship to Review Notes" below). A Review Note is a useful observation that is intentionally not strong enough for a confirmed finding; it is never presented as a confirmed defect.

Relationship to the Existing Policy

This reasoning loop must not replace or weaken any existing review policy.

The existing rules remain authoritative for:

• evidence requirements
• confidence thresholds
• speculative findings
• style/cosmetic findings
• severity
• changed-code scope
• organization-specific rules
• React/TypeScript rules
• test policy
• finding deduplication
• final verification
• publication integrity
• operational safety

The loop produces the review judgment. The existing quality gates — §8 Evidence Requirement, §9 No Speculation, §12 Precision over Coverage, and §15 Final Review Validation — remain the authoritative filters that decide whether a potential issue becomes a finding, and §16/§17 remain authoritative for publication. The Architecture & Change Impact stage (§3a) is an additional reasoning capability, not a relaxation of any of these rules: architecture findings must still pass every gate above plus the stricter Architecture Finding Gate in references/architecture-change-impact.md.

Relationship to Review Notes

The Review Notes channel (see references/review-policy.md §24) is the second output of the loop. It does not relax any finding gate:

• The loop's judgment step (step 6 above) routes each observation into exactly one of two tracks: Finding (passes the quality gates → inline PR comment, §16) or Review Note (useful but below the confidence/actionability threshold → PR-level comment, §16 "PR-level Review Notes comment").
• A Review Note must never be promoted into a finding, never carry Critical/Warning severity or a finding rule ID, and never be presented as a confirmed defect.
• A Review Note is still evidence-based: it describes an observed behavior or concrete concern, references relevant code/context where useful, explains why it is worth attention, and states its uncertainty or limitation. It must not be speculative, generic, stylistic, or unrelated — if it has no useful evidence or developer value, discard it entirely (do not create a note for every edge case considered).
• Review Notes do not count as findings, are not subject to the inline finding gates (they intentionally failed them), and are published only as a single PR-level comment when at least one useful note exists (references/review-policy.md §24).

6. Review Standards

The detailed review standards are not defined in this file.

Use the reference documents as the source of truth.

General review behavior:

references/review-policy.md

React/TypeScript rules:

references/react-typescript-review.md

Organization-specific rules:

references/organization-coding-standards.md

when available and applicable (skip this reference if the file does not exist).

Do not invent organization-specific rules.

Do not treat personal coding preferences as organizational standards.

7. Review Scope

Review the changes introduced by the selected Pull Request.

Focus on:

• correctness
• regressions
• runtime behavior
• business logic
• security
• performance
• maintainability
• project standards
• testability
• architecture

Do not report unrelated pre-existing problems.

A pre-existing problem may be reported only when the Pull Request materially changes or exposes the behavior.

Changed-file gate. A finding must apply to a file (and a line, if it is an inline finding) that actually appears in the changed-file manifest from §4 step 5 — in the diff range <base>...<source-branch> at the PR source tip. A file that exists in the repository on some other commit line, on the target branch, or in a locally-checked-out commit that is not the PR source tip is not in scope. If you are looking at a file that is not in the manifest, stop and re-derive the manifest; do not compose a finding from it. In practice:

• A file that is only deleted by the PR (manifest status D) has no new/changed source-side lines to anchor inline comments to; findings can only be about the deletion itself (e.g. broken import left dangling elsewhere in the diff), not about the deleted file's internal logic.
• A file that is modified (status M) — findings must be on lines the diff +/context changed, not on unchanged context lines that merely live in a changed file.
• A file that is added or renamed (status A/R) — the new-path is the anchor path; the old path (for R) is not where the PR's changed lines live.

This gate is the primary defense against the class of false positives where the reviewer reads a file from the wrong commit and treats it as if the PR introduced its contents.

8. Evidence Requirement

Every finding must be supported by evidence from the repository.

Before reporting a finding, verify it against relevant context.

Use this order of confidence:

1. Direct evidence in the changed code
2. Evidence from surrounding code
3. Evidence from callers/consumers
4. Evidence from repository conventions
5. Evidence from types/contracts/tests
6. Commit history when necessary (via bb_list_commits, bb_get_commit, bb_get_commit_diff, bb_get_diff from bitbucket-standalone-mcp)

Provenance is part of the evidence, not an afterthought. The evidence for a finding must come from code introduced or materially changed by this PR at the PR source tip. Evidence read from:

• a different commit line of the same repo (e.g. the target branch, an older tip, or a commit on a sibling feature branch),
• the local working tree when it is at a commit other than the PR source tip,
• a file that does not change at all between the merge base and the PR source tip,

does not satisfy the evidence bar for this review. Such code may be referenced for context (e.g. a caller in an unchanged file), but a finding cannot be about unchanged code merely because it lives in the same repo. Before reporting a finding, confirm the changed file is in the §4 step-5 manifest and that the specific line(s) are inside the diff (+ lines or changed context, not unchanged context), using the per-finding file-existence check in §4 step 5.

If the evidence is insufficient — including insufficient that it is in the PR's diff range — do not report the issue.

Effective-PR-diff boundary for posting. The evidence bar above applies to finding composition. For posting, the bar is stricter: the anchor must be verifiable within the PR's effective diff (the diff Bitbucket serves for the PR via bb_get_pull_request_diff_summary, not just the local manifest). A file that exists on the source tip but whose changes were reverted, dropped, or superseded before the PR was finalized does not appear in the effective diff — posting to it creates an orphaned comment that reviewers cannot resolve. Always cross-check the anchor against bb_get_pull_request_diff_summary before posting, even when the local manifest says the file is in range (stale local checkouts can include files that the effective PR no longer touches).

Prefer silence over false positives.

9. No Speculation

Do not report speculative issues.

Avoid findings based only on statements such as:

• "this might cause a problem"
• "this could potentially fail"
• "verify whether this is intentional"
• "consider checking"
• "this may be inefficient"

unless the repository evidence demonstrates an actual risk.

A finding must explain the concrete failure mode or violation.

10. Severity

Use the severity defined by the applicable review rule, but consider the actual impact.

💥 Critical

Use for severe issues such as:

• major runtime failures
• serious security vulnerabilities
• data corruption
• incorrect business-critical behavior
• severe regressions

⚠️ Warning

Use for issues such as:

• real bugs
• meaningful regressions
• reliability problems
• important performance problems
• significant maintainability problems

❔ Suggestion

Use for:

• minor maintainability improvements
• readability
• accessibility
• low-risk style improvements
• non-critical improvements

Do not inflate severity.

A rule marked as Critical does not automatically mean every occurrence has critical real-world impact.

11. Findings

Each finding must be:

• specific
• actionable
• concise
• evidence-based

Explain:

1. What is wrong
2. Why it matters
3. What should change

When possible, include:

• file path
• line number
• rule ID
• concrete recommendation

Use the finding-body shape — the same as the posted PR comment (references/pr-posting.md "Finding body"): a #<Rule-ID> Severity — summary header plus three short bullets (What is wrong, Why it matters, Suggested change). The Markdown list adds an Anchor & status line (file:line + posted/not-posted, see §17). Never use the old one-line style; always use this shape:

- **#AS-09** **Warning** — `formValuesRef.current!` can be `null` on this execution path.
    • **What is wrong:** `useLogic.tsx:182` dereferences `formValuesRef.current!` without a null guard before building the filter.
    • **Why it matters:** The task list throws a runtime error when a task opens.
    • **Suggested change:** Guard `formValuesRef.current` before constructing the filter.
    • **Anchor & status:** `useLogic.tsx:182` — posted to PR inline comment.

12. Precision over Coverage

• No duplicates. Do not report the same underlying problem twice; when multiple symptoms share a root cause, report the root cause. Separate findings only for independent problems.
• No false positives. When uncertain whether something is real, do not report it. Precision beats volume — three verified issues beat ten speculative ones.

13. No Praise

Do not include:

• praise
• general positive feedback
• descriptions of what was implemented correctly
• generic summaries

Focus on actionable findings.

If no issues are found, do not manufacture findings.

14. Environment Limitations

If a required validation cannot be performed because of the environment, mention the limitation briefly.

Examples:

• dependencies are not installed
• build cannot be executed
• tests cannot be executed
• private packages are unavailable
• external service is unreachable
• the bitbucket-standalone-mcp server is not connected or a required bb_* call fails
• a server-specific endpoint is unavailable (see references/bitbucket-server-quirks.md)

Do not convert an unverified possibility into a finding.

The local-first model (§0) applies here too — a disconnected MCP server is not a blocker for reviewing content: the local checkout covers file contents, diffs, commits, and line numbers. Only pieces that are genuinely server-only (PR comments, CI status, PR state) become hard limitations when the MCP server is down; report those per §14 and continue the content review from the local clone. If a specific MCP tool fails due to a documented server limitation, adapt (use the local clone for content, or the documented alternative in references/bitbucket-server-quirks.md); only report the gap if no source — local or MCP — can provide that specific piece of information.

15. Final Review Validation

Before returning the review:

1. Confirm the correct PR, target branch, and review range.
2. Manifest re-check (§4 step 5): every finding's file appears in the manifest (M/A/R-new-side) and its line is inside the diff. Any finding on a file/line outside the manifest is dropped before the report.
3. Dependency pass re-check (§5): for every manifest file, the dependency pass ran (a trace_path/search_graph/detect_changes

*Truncated - read the full file at https://github.com/jabdon00/pr-review/blob/954cc380ad752913661d1ce14e9b55c9ff06751e/SKILL.md.