Review Issue

---
name: review-issue
description: Use when reviewing an open apijack PR that targets the dev branch — verifies acceptance criteria, leaves PR comments on any findings, or merges and hands off to next-deployer
---

# Review Issue PR

Phase 3 of the apijack issue workflow. Review a PR targeting `dev` against the linked issue's acceptance criteria.

## When to Use

- User asks to review an open PR on the apijack repo
- Only for PRs whose **base is `dev`** — refuse to proceed if base is `main` or any other branch

## Steps

### 1. Load the PR and linked issue

```bash
gh pr view <pr-number> --comments --json number,title,body,baseRefName,headRefName,state,author,labels,comments,reviews
gh pr diff <pr-number>
gh pr checks <pr-number>
```

> Use the `--json` form on `gh pr view`. The plain form currently fails with `GraphQL: Projects (classic) is being deprecated ... (repository.pullRequest.projectCards)` because `gh` still requests the deprecated `projectCards` field by default.

Find the linked issue (`Closes #NN` in the body) and load it:

```bash
gh issue view <issue-number> --comments
```

**Abort if the base branch is not `dev`** — tell the user and stop.

### 1.5. Apply "review in progress" label

Use the helper scripts in `.claude/skills/review-issue/scripts/` for label management. They go through the REST API and bypass the `gh pr edit --add-label` / `--remove-label` GraphQL "Projects (classic) is being deprecated" failure.

Ensure the four workflow labels exist (idempotent — safe to run every time, no-op if they already exist):

```bash
.claude/skills/review-issue/scripts/ensure-review-labels.sh
```

Move the PR into the "review in progress" state. This single call removes any of the other review-state labels (`needs review`, `first pass reviewed`, `changes requested`) and adds `review in progress`. Other PR labels are untouched:

```bash
.claude/skills/review-issue/scripts/set-review-label.sh <pr-number> "review in progress"
```

### 2. Establish acceptance criteria

- **Explicit** — issue lists them → use those
- **Narrow scope** — issue is a simple, well-defined fix → improvise minimal criteria (the bug no longer reproduces, relevant tests added)
- **Involved scope** — issue is multi-faceted and criteria are unclear → STOP and ask the user how to proceed

### 3. Review

Check each of:

- **Correctness**: does the diff actually solve the issue?
- **Acceptance criteria**: each criterion met?
- **Tests**: new behavior is tested; existing tests still pass (CI green)
- **Scope**: no unrelated changes smuggled in
- **Style/consistency**: matches the surrounding codebase
- **Backwards compatibility**: public API and generated output unchanged unless the issue explicitly asks

For a structured review, invoke the code-review skill on the diff:

```
Skill(skill="superpowers:requesting-code-review")
```

### 4. Decide

**If ANY blocking issue is found OR any acceptance criterion is unmet:**

Build the review body using GitHub-flavored markdown. Blocking items appear at the top; non-blocking observations and nits are hidden in collapsible `<details>` blocks. Omit any section that has no items.

```
> 🤖 **Automated review by claude** — generated by the `review-issue` skill. Treat as advisory; a human still owns the merge decision.

<1-2 sentence summary of what is blocking the merge>

**Blocking:**
- <blocking item 1>
- <blocking item 2>

<details>
<summary>Non-blocking observations</summary>

- <observation 1>
- <observation 2>

</details>

<details>
<summary>Nitpicks</summary>

- <nitpick 1>

</details>
```

Post the review and request changes.

> **Important:** do NOT pass the review body inline via `--body "..."`. Putting markdown into a shell-arg context tempts the model to escape backticks (the escapes get posted literally and the rendered review is full of `\`code\`` artifacts — see issue #48 for the symptom). Use the helper script, which takes a body file and uses `gh pr review --body-file`. The `Write` tool stores content literally — no shell context, no escaping.
>
> Steps:
> 1. Use the `Write` tool to write the review body verbatim to `.claude-jobs/review-bodies/<pr-number>.md`. Write backticks as plain backticks. Do not insert `\\` anywhere.
> 2. Run the helper:

```bash
.claude/skills/review-issue/scripts/post-review.sh <pr-number> request-changes .claude-jobs/review-bodies/<pr-number>.md
```

Update labels:

```bash
.claude/skills/review-issue/scripts/set-review-label.sh <pr-number> "changes requested"
```

Then **STOP**. Report findings to the user and end.

**If AND ONLY IF all criteria are met and no blocking issues are found (only non-blocking observations / nits, or nothing at all):**

Build the review body — skip the **Blocking:** section entirely. Only include non-empty `<details>` blocks for observations/nits:

```
> 🤖 **Automated review by claude** — generated by the `review-issue` skill. Treat as advisory; a human still owns the merge decision.

<1-2 sentence summary confirming the PR looks good>

<details>
<summary>Non-blocking observations</summary>

- <observation 1>

</details>

<details>
<summary>Nitpicks</summary>

- <nitpick 1>

</details>
```

Post a non-blocking comment (does not block merging). Same procedure as the request-changes path — write the body to a file with the `Write` tool, then call the helper:

```bash
.claude/skills/review-issue/scripts/post-review.sh <pr-number> comment .claude-jobs/review-bodies/<pr-number>.md
```

Update labels:

```bash
.claude/skills/review-issue/scripts/set-review-label.sh <pr-number> "first pass reviewed"
```

Merge the PR:

```bash
gh pr merge <pr-number> --merge --delete-branch
```

Then invoke the next skill:

```
Skill(skill="next-deployer", args="<issue-number>")
```

## Red Flags — Do Not Merge

- "CI is red but I know it's flaky" → STOP
- "The PR targets main" → STOP, do not review here
- "Acceptance criteria are missing but I'll guess" → STOP, ask the user
- "There are nits but I'll merge anyway" → leave comments, do not merge
- "I'll squash-merge" → use plain `--merge` to preserve history on `dev`

## Output

Either: PR comments + "changes requested" review and STOP, or: merged to `dev` and handed off to `next-deployer`.