Review Pr

Review Pull Request

Purpose

Review a specific GitHub Pull Request by fetching its diff, analyzing it against the relevant .github/instructions files, and posting structured review comments directly on the PR using the GitHub CLI (gh).

Execution mode (read this first)

This skill runs in two possible modes. Detect the mode before doing anything else:

• CI mode: the environment variable CI is set to true (this is the case in the GitHub Actions pipeline that invokes this skill in non-interactive/print mode).
• Interactive mode: CI is not set (a human is chatting with the agent directly).

In CI mode there is no human available to answer questions. --print is a single-shot, non-interactive invocation — if this skill ever stops to ask a question and wait for a reply, the run silently wastes its turn budget and ends without doing anything useful. So in CI mode:

• Never ask for the PR number/repo. Read them from environment variables (see step 1).
• Never ask for confirmation before posting. The pipeline itself is the approval gate (a human triggered the workflow, or reviews the run logs). Post directly once the review is generated.
• Never wait for user input of any kind.

In Interactive mode, keep the original behavior: ask for the PR if not given, and ask for confirmation (sí/no) before posting.

Prerequisites

• gh must be authenticated (gh auth status)
• The .github/instructions/ directory must exist with instruction files
• The PR must be open and accessible

Required workflow

1. Resolve PR context

CI mode: read directly from environment, do not ask the user anything:

echo "REPO=$GITHUB_REPOSITORY"
echo "PR=$PR_NUMBER"
echo "BASE=$GITHUB_BASE_REF"

If GITHUB_REPOSITORY or PR_NUMBER is empty, stop and print a clear error ("[FATAL] Missing GITHUB_REPOSITORY or PR_NUMBER env vars") instead of asking a question — there is no one to answer it.

Interactive mode: the user must provide a PR URL or owner/repo#number format (e.g., andresjz/API#1). If not provided, ask for it. Default repo can be inferred from gh pr view.

From here on, $REPO and $PR refer to whichever of the two sources above was used. Persist them once in a shell variable/temp file at the start and reuse — do not re-derive them repeatedly.

Define an isolated working directory for this run, scoped by repo and PR number. The runner may execute multiple reviews concurrently (different PRs, or even the same PR re-triggered), so a shared /tmp/pr_review path can cause one run to read/overwrite another run's cached files. Always do this before step 3:

REPO_SAFE=$(echo "$REPO" | tr '/' '_')
WORKDIR="/tmp/pr_review/${REPO_SAFE}/${PR}"
mkdir -p "$WORKDIR"
echo "WORKDIR=$WORKDIR"

Every temp file referenced from here on (meta.json, diff.txt, files.txt, head_sha.txt, comments.json, inline_comments.json, summary.md, inline_comment.json) lives under $WORKDIR, not under a fixed shared path.

2. Check working tree status (safety guard)

git status --short

If this returns any output, stop and tell the user to commit/stash first (Interactive mode) or fail the step with a clear message (CI mode). Ignore changes to SKILL.md, AGENTS.md, Taskfile.yml, OPENCODE_SETUP.md.

3. Fetch the PR metadata, diff, and comments — once, cache locally

Fetch everything needed in this step and write it to temp files. Do not re-fetch the same data later in the run; reuse the files.

gh pr view "$PR" --repo "$REPO" --json number,title,body,headRefName,baseRefName,author,additions,deletions > "$WORKDIR/meta.json"
gh pr diff "$PR" --repo "$REPO" > "$WORKDIR/diff.txt"
gh pr view "$PR" --repo "$REPO" --json files --jq '.files[].path' > "$WORKDIR/files.txt"
gh pr view "$PR" --repo "$REPO" --json commits --jq '.commits[-1].oid' > "$WORKDIR/head_sha.txt"
gh pr view "$PR" --repo "$REPO" --json comments --jq '.comments[] | {author: .author.login, body: .body, createdAt: .createdAt}' > "$WORKDIR/comments.json"
gh api "/repos/$REPO/pulls/$PR/comments" --jq '.[] | {path: .path, line: .line, body: .body, author: .user.login}' > "$WORKDIR/inline_comments.json"

Read SHA=$(cat "$WORKDIR/head_sha.txt") once and reuse it for every inline comment in step 6 — do not call gh pr view --json commits again.

4. Discover and load only relevant instructions (repo-agnostic)

This skill must work on any repo/language/framework. Never hardcode filenames like java.instructions.md or controllers.instructions.md — discover what actually exists and decide relevance dynamically.

ls .github/instructions/*.instructions.md 2>/dev/null

For each *.instructions.md file found, determine whether it applies to this PR:

Tier 1 — frontmatter applyTo (preferred, matches GitHub's Copilot custom-instructions convention): Read the YAML frontmatter at the top of the file (between --- lines). If it has an applyTo key with one or more glob patterns (e.g. applyTo: "**/*.java" or applyTo: "**/*Controller.java,**/*Controller.kt"), match every path in $WORKDIR/files.txt against those globs.

• Match → include this instruction file's content when reviewing those files.
• No match on any changed file → skip this instruction file entirely (don't load its content, don't mention it).
• A file with applyTo absent, empty, or set to ** / * → treat as always applicable (general/repo-wide guidance, e.g. general.instructions.md).

Tier 2 — filename-keyword fallback (only if the repo's instruction files have no frontmatter): Strip .instructions.md from the filename to get a keyword (e.g. services, migrations, frontend). Include the file if that keyword (singular or plural, case-insensitive) appears in the path of at least one modified file. Always include any file literally named general.instructions.md (or equivalent, e.g. default, common).

Load only the instruction files that matched. State in the review draft which instruction files were considered relevant and why (one line is enough), so the mapping is auditable per repo instead of assumed.

5. Generate structured review draft

Analyze the diff + instructions + existing comments and produce structured feedback in Spanish (see "Review goals" below).

Use existing comments to resolve uncertainty, not just to avoid duplication. Before flagging something as a finding, check $WORKDIR/comments.json and $WORKDIR/inline_comments.json:

• If a past comment already explains why something that looks odd is intentional, don't re-flag it — reference the earlier discussion instead if relevant.
• If a past comment raised the same concern and it's still unresolved in the current diff, you can reference it ("como se comentó antes...") rather than presenting it as a new finding.
• When you're genuinely unsure whether something is a real issue (e.g. an unusual pattern that might be a deliberate project convention), check whether prior comments already settled that question before guessing.

You may read files outside the diff, in a bounded way, to check for patterns and duplicate code. The original guard against browsing the repo was too strict — some findings (e.g. "this duplicates logic already in X", "this doesn't follow the pattern used in Y") genuinely need to see another file to be correct instead of speculative. Rules to keep this bounded (see also Anti-loop guards):

• Only read a file outside the diff when it materially changes a specific finding — not for general exploration.
• Prefer a targeted grep/find for a function/class name over browsing directories.
• Cap it at roughly 5 extra file reads per review. If you need more than that, the finding probably isn't worth chasing further — state the suspicion in the review as a question for the author instead of confirming it exhaustively.

Interactive mode only: show the draft to the user and ask:

¿Confirmas que quieres publicar esta review en el PR? (sí/no)

Si dices "sí", se publicará. Si dices "no" o quieres modificarlo, indícame los cambios.

CI mode: skip this. Go straight to step 6.

6. Post the review

CI mode: post automatically, no confirmation needed. Interactive mode: only after user confirmation.

Option A: Post as a PR comment (simple, always works)

Always write the body to a temp file first — never inline multiline content into --body, this is a common source of shell-escaping failures that cause retries/loops:

cat > "$WORKDIR/summary.md" << 'ENDOFFILE'
<review_content>
ENDOFFILE
gh pr comment "$PR" --repo "$REPO" --body-file "$WORKDIR/summary.md"

Option B: Post inline comments on specific lines (for critical findings)

Use temp JSON files, not process substitution (<(...)) — process substitution is not supported by sh/dash and silently fails on some CI runners, which is a likely source of retry loops:

SHA=$(cat "$WORKDIR/head_sha.txt")

cat > "$WORKDIR/inline_comment.json" << EOF
{
  "body": "[TAG] Comment text here",
  "path": "src/main/java/.../File.java",
  "line": 42,
  "commit_id": "$SHA",
  "side": "RIGHT"
}
EOF

gh api \
  --method POST \
  -H "Accept: application/vnd.github.v3+json" \
  "/repos/$REPO/pulls/$PR/comments" \
  --input "$WORKDIR/inline_comment.json"

Hard rule to prevent loops: for a given finding, attempt an inline comment at most twice (e.g. the diff line, then one nearby line inside the same hunk if the first is rejected with "could not be resolved"). If both attempts fail, do not keep guessing lines — immediately fall back to including that finding in the Option A general PR comment instead, tagged with the file/line in text, and move on to the next finding.

Using suggestion blocks (one-click auto-apply)

GitHub renders a fenced ```suggestion code block inside an inline PR comment as an "Apply suggestion" button, letting the PR author accept the exact replacement with one click. Use this whenever the fix is mechanical and safe to apply verbatim — e.g. a null check, a typo, a wrong variable, a missing import, a formatting issue, a simple off-by-one. This is the single highest-leverage change reviewers can make, since it turns a comment into a one-click fix instead of manual retyping.

The body of the comment is: a short explanation, then a suggestion fence containing only the corrected code that should replace the commented line(s) — nothing else inside the fence (no comments-about-the-code, no partial line).

[BUG] `user` puede ser null aquí si el lookup falla; agrega el check antes de usarlo.

```suggestion
if (user == null) {
    return Optional.empty();
}

For a **single line** fix, comment on that `line` as usual (see JSON payload above) and the suggestion fence must contain exactly one line (the full replacement for that line).

For a **multi-line** fix, add `start_line` to the JSON payload (in addition to `line`, which becomes the end of the range) so the range being replaced is unambiguous:
```json
{
  "body": "[PATTERN] ...\n\n```suggestion\n<replacement for the whole range>\n```",
  "path": "src/.../File.java",
  "start_line": 38,
  "line": 42,
  "commit_id": "$SHA",
  "side": "RIGHT",
  "start_side": "RIGHT"
}

The suggestion fence must contain the complete replacement for the entire start_line..line range, not just the changed portion — GitHub replaces the whole commented range with the fence content.

When NOT to use suggestion: architectural/pattern changes that touch multiple files, migrations that need manual judgment (e.g. backfill strategy, data volume considerations), renames that require consistency checks elsewhere in the codebase, or anything where auto-applying without further thought could introduce a new inconsistency. In those cases keep the code example as a plain fenced block (no suggestion tag) — informative, not one-click-appliable — as before.

Option C: Post a review with approval or request changes

gh pr review "$PR" --repo "$REPO" --approve --body "<message>"
gh pr review "$PR" --repo "$REPO" --request-changes --body "<message>"

7. Cleanup

This is a mandatory last step, always run — whether the review was posted, skipped, or the run failed partway through. The concern is narrow: empty only the temp files this skill itself created during the run — nothing else in /tmp.

for f in "$WORKDIR/meta.json" \
         "$WORKDIR/diff.txt" \
         "$WORKDIR/files.txt" \
         "$WORKDIR/head_sha.txt" \
         "$WORKDIR/comments.json" \
         "$WORKDIR/inline_comments.json" \
         "$WORKDIR/summary.md" \
         "$WORKDIR/inline_comment.json"; do
  [ -f "$f" ] && echo "" > "$f"
done

Do this as the very last action, after any gh posting calls have completed (success or failure) — never empty these files mid-run, since later steps still read from them. Don't glob or delete anything else under $WORKDIR — only the files this skill wrote.

Review goals

You are a senior code reviewer reviewing a Pull Request.

Use:

• The PR diff ($WORKDIR/diff.txt)
• The PR metadata (title, description, author)
• The relevant .github/instructions for modified files
• Existing comments on the PR (to avoid duplication)

Produce a structured review in Spanish with:

0. Puntuación de Calidad

Always open the review with an overall quality score for the PR, using this exact format:

## Puntuación de Calidad
⭐⭐⭐⭐ — [Justificación de 1-2 líneas, basada en los hallazgos concretos de las secciones siguientes, no genérica.]

Scale (pick one, use the criterion closest to the PR's actual state — this is stack-agnostic, applies equally to backend, frontend, infra, etc.):

The justification must reference what was actually found (e.g. "hay 2 bugs potenciales y falta cobertura de tests" or "sigue el patrón del proyecto, solo detalles de naming"), never a generic phrase like "buen trabajo" without grounding it in the sections below. Determine the score after doing the file-by-file and guideline-compliance analysis, not before — the score must follow from the findings, not the other way around.

1. Summary of changes

• 3 to 7 bullets describing the key changes in the PR.

2. File-by-file feedback

For each modified file, identify:

• Probable or confirmed bugs.
• Missing edge cases.
• Performance, concurrency, or security concerns.

3. Guideline compliance

Verify compliance against whatever instruction files were loaded in step 4 — one subsection per loaded instruction file, using that file's own name/topic as the heading (e.g. if the repo has frontend.instructions.md and terraform.instructions.md, review against those; do not assume a Java/API-style breakdown by controllers/services/entities/DTOs unless the repo's instructions actually define those categories).

4. Testing gaps

Identify missing or weak tests in the PR. Propose concrete test cases where needed.

5. Suggested improvements

Propose practical improvements such as:

• Small refactors.
• Naming improvements.
• Helper extraction.
• Better module organization.

Writing actionable comments — include resolution examples

Every [BUG], [MIGRATION], [PATTERN], and [BREAKING] comment should include a code example showing how to fix or mitigate the issue.

[MIGRATION] is not backend/SQL-specific — use it for any structural, hard-to-reverse change: a DB migration, an infra state change (Terraform/Pulumi), a config schema change, a queue/topic rename, a feature-flag rollout step, a frontend build/routing migration, etc. The concrete example should be written in whatever language/format the change actually is (SQL, HCL, YAML, JSON, shell), not defaulted to SQL.

Bad — just the problem (backend/DB example):

[MIGRATION] El DROP COLUMN no tiene backfill. Sugiero agregar un UPDATE de backfill como precaución.

Good — problem + concrete fix (backend/DB example):

[MIGRATION] El DROP COLUMN no tiene backfill. Agrega un UPDATE de backfill antes del DROP:

UPDATE notifications
SET message_data = (
    COALESCE(message_data::jsonb, '{}'::jsonb) ||
    jsonb_build_object('unitType', unit_type)
)::text
WHERE unit_type IS NOT NULL;

ALTER TABLE notifications DROP COLUMN IF EXISTS unit_type;

Good — problem + concrete fix (infra example, same principle in a non-backend repo):

[MIGRATION] Este cambio de instance_type en Terraform va a forzar recreación del recurso (no in-place update), lo que causa downtime. Usa lifecycle.create_before_destroy o migra por etapas:

resource "aws_instance" "app" {
  instance_type = "t3.large"
  lifecycle {
    create_before_destroy = true
  }
}

The two examples above illustrate the same tag applied to different stacks — pick the format that matches the actual repo, don't force SQL onto a non-backend change.

Guía por tipo de hallazgo:

Output rules

• Respond in Spanish.
• Use code blocks for examples.
• Be specific and actionable.
• Prefer concrete findings over generic praise.
• If there are no issues, say so explicitly and explain why.
• The review should be posted as a PR comment or inline comments via gh.

Anti-loop guards (hard limits)

• Reading outside the PR diff is allowed only in the bounded way described in step 5 (verifying a specific pattern/duplication finding, capped at ~5 extra file reads, targeted grep over browsing). Do not use it as general exploration.
• Do not broaden the task beyond the PR scope.
• Do not repeat the same command or reasoning path.
• Fetch PR metadata/diff/comments once (step 3) and reuse the cached files in $WORKDIR — never re-fetch.
• If the diff is large (>1000 lines), prioritize the files most likely to carry risk for this repo — infer from what actually changed and from which loaded instruction files flagged them as sensitive (e.g. auth/security-related code, data access/migration files, public API contracts, infra state) rather than assuming a fixed backend layer like "services/controllers". Explicitly state which files were skipped and why.
• If information is missing, state the limitation instead of guessing.
• Limit every tool call to a maximum of 2 attempts. On the first failure, print the full output/error and explain why. If it fails a second time, skip it and continue — never retry a third time with a variation.
• Inline comments: max 2 attempts per finding (see Option B). After that, fall back to a general comment and move on.
• CI mode never blocks on a question. If at any point the instructions below seem to require waiting for a human, prefer the CI-mode default from the "Execution mode" section over stalling.
• Global budget: if you notice you are repeating the same class of action (e.g. retrying inline comments) more than ~5 times across the whole run, stop attempting inline comments entirely, note it in the summary comment, and finish.
• Always run step 7 (cleanup) before ending, regardless of how the run went — emptying the working files is not optional, even on early exit/error paths.

Usage examples

CI (GitHub Actions, env vars already set: GITHUB_REPOSITORY, PR_NUMBER, GITHUB_BASE_REF, CI=true):

Run the agent in non-interactive/print mode with the skill's SKILL.md as input. The exact command depends on the agent being used. The agent needs access to git, gh, jq, ls, cat, find, grep, head, mkdir, tr, echo, bash, Read, and Write tools.

Example (generic):

<agent-cli> --print --max-turns 40 < SKILL.md

Interactive:

• User: "Review PR myorg/myrepo#42"
• User: "Review https://github.com/myorg/myrepo/pull/42"
• User: "Review the current PR (assumes gh pr view works on the checked-out branch)"