Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
{
"name": "review-utils",
"description": "Self-contained PR-review pipeline: composite GitHub Actions (agent-setup, run-agent), reusable workflow integration, and the review-code skill that drives the review methodology. Replaces the previous scattered setup across nsheaps/agents + nsheaps/.ai-agent-henry.",
"version": "0.2.9",
"version": "0.3.0",
"author": {
"name": "Nathan Heaps"
},
Expand Down
23 changes: 0 additions & 23 deletions .github/scripts/review-receiver/dismiss-prior-approvals.sh

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ fi

verdict=$(grep -E '^verdict:' "$path" | sed 's/^verdict:[[:space:]]*//' | tr -d '"' || echo unknown)
follow_ups=$(grep -E '^follow_ups:' "$path" | sed 's/^follow_ups:[[:space:]]*//' || echo 0)
skipped=$(grep -E '^skipped:' "$path" | sed 's/^skipped:[[:space:]]*//' | tr -d '"' || echo false)

case "$verdict" in
APPROVE|approve)
Expand Down Expand Up @@ -117,6 +118,15 @@ case "$verdict" in
;;
esac

# skipped: true means the skill decided (per incremental-review.md) that no
# new review was warranted — same diff content, or a routine-update refresh
# with nothing new to say. The verdict/follow_ups above are carried over
# from prior state unchanged; swap in a title that makes clear nothing new
# ran, so the check doesn't read as if a fresh review just happened.
if [ "$skipped" = "true" ]; then
title="No changes requiring re-review — previous review still stands (${verdict}, ${follow_ups} follow-ups)."
fi

{
echo "conclusion=$conclusion"
echo "title=$title"
Expand Down
39 changes: 15 additions & 24 deletions .github/workflows/review-receiver.yaml

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where's the part that uses actions/cache to save information for PR reviews between runs?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not in review-receiver.yaml — the cache restore/save lives in the composite action it calls, plugins/claude-code/review-utils/actions/run-agent/action.yaml (restore at lines 222-229, save at 332-337), since that's where ${{ runner.temp }}/review-state.json is populated and where the agent run actually happens. review-receiver.yaml just invokes run-agent — it doesn't touch the cache directly.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it was already present? Can you trigger the review workflow through this PR and test that?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No — verified via git show main:.../run-agent/action.yaml | grep actions/cache, which returns nothing. The cache restore/save is new in this PR.

Can't easily test it live through this PR though: this repo's own .github/workflows/dispatch-review.yaml (the consumer template copy) calls the reusable decider via uses: nsheaps/agents/.github/workflows/review-dispatch.yaml@9384e01d... — a pinned commit SHA, not @main and not this branch. Reusable workflow_call workflows always execute the code at the ref in uses:, so triggering a review on this PR right now would run that pinned SHA's code (which predates even current main, let alone this branch) — not the cache logic added here. Also this PR is a draft, so the gate wouldn't even fire without the request-review label per its if: condition.

Real options to validate before merge:

  1. Merge this, then validate on the next PR that touches review-utils (dogfooding via this repo's own future PRs).
  2. Temporarily repoint the SHA pin in dispatch-review.yaml to this branch's HEAD, label this PR request-review to force the gate despite draft state, watch it run, then revert the pin before merge. Invasive but does exercise the real code path.

I'd lean toward (1) unless you want the smoke test now — let me know which you'd prefer.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can ignore this, this session will solve that: https://claude.ai/code/session_01HfFLavgfxQetUjPgy1VYFR

Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,25 @@ name: Review Receiver (reusable)
# for non-draft transitions. Shell scripts >3 lines are extracted to
# .github/scripts/review-receiver/.
#
# Approval dismissal is no longer handled by this workflow — see the
# "2026-08-12 redesign" section of the spec above.
#
# Flow (review job):
# 1. Checkout the plugin contents (nsheaps/agents@plugin-ref) — must happen
# early so .github/scripts/review-receiver/ is available.
# 2. Generate review App token (used to call back into consumer repo).
# 2. Generate App token for updating checks (used to call back into
# consumer repo).
# 3. Update the consumer-side check-run to in_progress "Review agent
# running...".
# 4. Remove the request-review label from the consumer PR (idempotent —
# no-op if absent). Frees it to be re-applied for the next round.
# 5. Dismiss prior APPROVED reviews from this bot on the PR.
# 6. Invoke the plugin's `run-agent` composite action.
# 7. Read metrics yaml/json file emitted by the agent; fail if absent.
# 8. Update the consumer-side check to terminal state (success/neutral/failure)
# 5. Invoke the plugin's `run-agent` composite action, which restores prior
# review state, runs the skill's skip/brief/full decision, and (only
# when warranted) dismisses a stale prior approval itself.
# 6. Read metrics yaml/json file emitted by the agent; fail if absent.
# 7. Update the consumer-side check to terminal state (success/neutral/failure)
# based on agent verdict + follow-up count.
# 9. `if: failure()` final guard: post "review agent failed to run" check.
# 8. `if: failure()` final guard: post "review agent failed to run" check.

on:
workflow_call:
Expand Down Expand Up @@ -191,14 +196,6 @@ jobs:
private-key: ${{ secrets.AUTOMATION_GITHUB_APP_PRIVATE_KEY }}
owner: ${{ steps.consumer.outputs.owner }}

- name: Generate App token for dismissing review
id: review-auth
uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3
with:
app-id: ${{ secrets.REVIEW_GITHUB_APP_ID }}
private-key: ${{ secrets.REVIEW_GITHUB_APP_PRIVATE_KEY }}
owner: ${{ steps.consumer.outputs.owner }}

# Update check-run on consumer to in_progress.
- name: Update check (in_progress)
uses: LouisBrunner/checks-action@937cbbcde3259005b50746dc91cde29098aac2ff # v3.1.0
Expand All @@ -225,16 +222,10 @@ jobs:
PR_NUMBER: ${{ inputs.source-pr-number }}
run: bash agent-repo/.github/scripts/review-receiver/remove-request-review-label.sh

# Dismiss prior APPROVED reviews from this bot so the PR can't merge
# on a stale approval. Keep REQUEST_CHANGES + COMMENT reviews intact.
# Logic in .github/scripts/review-receiver/dismiss-prior-approvals.sh.
- name: Dismiss prior approval reviews
env:
GH_TOKEN: ${{ steps.review-auth.outputs.token }}
SOURCE_REPO: ${{ inputs.source-repo }}
PR_NUMBER: ${{ inputs.source-pr-number }}
APP_SLUG: ${{ steps.review-auth.outputs.app-slug }}
run: bash agent-repo/.github/scripts/review-receiver/dismiss-prior-approvals.sh
# Prior-approval dismissal is NOT done here — it's the skill's job now,
# gated on the full-review decision (see the "2026-08-12 redesign"
# section of the spec). An unconditional pre-dismissal here would
# re-open an already-approved, unchanged PR on every rebase.

# Invoke the plugin's run-agent composite. It owns the actual review:
# auth, checkout consumer repo, claude-code-action with plugins, finalize.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "review-utils",
"version": "0.2.9",
"version": "0.3.0",

Check notice on line 3 in plugins/claude-code/review-utils/.claude-plugin/plugin.json

View workflow job for this annotation

GitHub Actions / version-preview

Pending version bump

review-utils: already bumped to 0.3.0 in this PR (kept on merge to main)
"description": "Self-contained PR-review pipeline: composite GitHub Actions (agent-setup, run-agent), reusable workflow integration, and the review-code skill that drives the review methodology. Replaces the previous scattered setup across nsheaps/agents + nsheaps/.ai-agent-henry.",
"author": {
"name": "Nathan Heaps",
Expand Down
38 changes: 38 additions & 0 deletions plugins/claude-code/review-utils/actions/run-agent/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,12 @@ runs:
else
echo "REVIEW_METRICS_PATH=" >> $GITHUB_ENV
fi
# REVIEW_STATE_PATH: cross-run cache of the skill's prior conclusions
# (diff fingerprint, classification, verdict, etc — see
# partials/incremental-review.md). Always set, unlike
# REVIEW_METRICS_PATH, since incremental review applies regardless of
# receiver vs direct-invocation mode.
echo "REVIEW_STATE_PATH=$RUNNER_TEMP/review-state.json" >> $GITHUB_ENV
# JOB_CONTEXT is JSON, use heredoc to handle multiline safely
DELIMITER="DELIM_$(date +%s%N)"
{
Expand All @@ -208,6 +214,20 @@ runs:
echo "$DELIMITER"
} >> $GITHUB_ENV

# --- Prior review state cache ---
# Restores the skill's last-run conclusions for this PR so it can decide
# to skip/brief-refresh/full-review instead of always doing a full
# review. Cache keys are immutable, so restore-keys prefix-matches the
# most recent entry (each save below uses a unique run_id-suffixed key).
- name: Restore prior review state
id: restore-review-state
uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
with:
path: ${{ runner.temp }}/review-state.json
key: review-state-${{ inputs.trigger-repo }}-${{ env.PR_NUMBER }}-${{ github.run_id }}
restore-keys: |
review-state-${{ inputs.trigger-repo }}-${{ env.PR_NUMBER }}-

# Strip YAML frontmatter from the skill body so envsubst only operates on
# the prompt text. The skill defines `name` + `description` for skill
# tooling — the action only needs the methodology that follows the second
Expand Down Expand Up @@ -341,6 +361,24 @@ runs:
}
}

# Ensure a state file exists before saving to cache — actions/cache/save
# fails if the path doesn't exist. If the skill errored before step 12
# (write review state), or this is a legacy direct-invocation without a
# receiver, fall back to whatever restore-review-state populated (or an
# empty placeholder on the very first run for this PR).
- name: Ensure review state file exists
if: always()
shell: bash
run: |
touch "${{ runner.temp }}/review-state.json"

- name: Save review state
if: always()
uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
with:
path: ${{ runner.temp }}/review-state.json
key: review-state-${{ inputs.trigger-repo }}-${{ env.PR_NUMBER }}-${{ github.run_id }}

# --- Finalize check run on source repo ---
# Only finalize when this action created the check. If the caller passed
# in a check-run-id, the caller (review-receiver) owns the terminal
Expand Down
61 changes: 42 additions & 19 deletions plugins/claude-code/review-utils/skills/review-code/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ PR_NUMBER: ${PR_NUMBER}
CHECK_RUN_ID: ${CHECK_RUN_ID}
WORKFLOW_RUN_URL: ${WORKFLOW_RUN_URL}
REVIEW_METRICS_PATH: ${REVIEW_METRICS_PATH}
REVIEW_STATE_PATH: ${REVIEW_STATE_PATH}

Review this PR providing inline feedback via the GitHub review system.

Expand All @@ -21,40 +22,56 @@ Do not duplicate questions from past reviews. Respond to engagement on your prev

1. **Get diff info**: Use `mcp__github__*` tools and `gh` CLI to understand changes, previous reviews, and line numbers.

2. **Review previous reviews** including your own. Your new review must be self-contained with all relevant details.
2. **Load prior review state and classify the PR, then decide whether to skip**: See `partials/incremental-review.md` for the full procedure — loading `${REVIEW_STATE_PATH}`, classifying the PR (`routine-update` vs `standard`), and the skip / brief-refresh / full-re-review decision tree. **Do this before any further steps.** If the decision is "skip", stop after emitting state + metrics (step 11/12) — do not touch comments, threads, or reviews.

3. **Track findings in a local doc** updated after every piece reviewed. Include summary, inline comments, questions, reference links. Do not trust memory.
3. **Review previous reviews** including your own. Your new review must be self-contained with all relevant details.

4. **Manage previous comments and threads**: See `partials/review-thread-management.md` for the full procedure on minimizing comments, resolving threads, and updating existing comments.
4. **Track findings in a local doc** updated after every piece reviewed. Include summary, inline comments, questions, reference links. Do not trust memory.

5. **Start a review**: Use `mcp__github__create_pending_pull_request_review`.
5. **Manage previous comments, threads, and prior approvals**: See `partials/review-thread-management.md` for the full procedure on minimizing comments, resolving threads, updating existing comments, and — for `standard`-depth re-reviews only — dismissing a stale prior approval. **Do not dismiss a prior approval for a `routine-update` brief refresh or a skip** (see `partials/incremental-review.md`).

6. **Add inline comments**: Use `mcp__github__add_comment_to_pending_review` for each piece of feedback. Use `suggestion` blocks for code changes. Only suggest changes with clear benefit (bug fix, perf, security, correctness, maintainability, simplicity). Never suggest changes to code outside the PR.
6. **Start a review**: Use `mcp__github__create_pending_pull_request_review`.

7. **Fetch review comments** to get URLs for cross-linking. Update your local doc.
7. **Add inline comments**: Use `mcp__github__add_comment_to_pending_review` for each piece of feedback. Use `suggestion` blocks for code changes. Only suggest changes with clear benefit (bug fix, perf, security, correctness, maintainability, simplicity). Never suggest changes to code outside the PR.

8. **Draft review summary** in your local doc with high-level assessment, strengths, improvements, critical issues, recommendation, and follow-ups.
8. **Fetch review comments** to get URLs for cross-linking. Update your local doc.

9. **Hide your previous reviews** just before submitting. Only hide YOUR OWN reviews:
9. **Draft review summary** in your local doc with high-level assessment, strengths, improvements, critical issues, recommendation, and follow-ups. Use full-depth formatting for `standard` classification; use brief formatting for `routine-update` classification — see `partials/review-formatting.md`.

```bash
gh pr view <PR_NUMBER> --json reviews --jq '.reviews[] | select(.author.login == "<BOT_USERNAME>") | {id, state}'
# Minimize each with GraphQL minimizeComment mutation, classifier: OUTDATED
```
10. **Hide your previous reviews** just before submitting. Only hide YOUR OWN reviews:

10. **Submit the review**: Use `mcp__github__submit_pending_pull_request_review`.
```bash
gh pr view <PR_NUMBER> --json reviews --jq '.reviews[] | select(.author.login == "<BOT_USERNAME>") | {id, state}'
# Minimize each with GraphQL minimizeComment mutation, classifier: OUTDATED
```

11. **Submit the review**: Use `mcp__github__submit_pending_pull_request_review`. Skip this step entirely if step 2 decided "skip" or "brief refresh with nothing new to say" — see `partials/incremental-review.md`.
- **REQUEST_CHANGES**: P0 or P1 follow-ups remain (security, perf, correctness, significant quality issues)
- **APPROVE**: no outstanding issues, PR is ready to merge (barring CI)
- **COMMENT**: only P2 follow-ups remain; prefer over APPROVE when improvements are wanted but won't break anything

11. **Emit review metrics** (REQUIRED; receiver gates final check on this file's presence).
12. **Write review state** (REQUIRED whenever `${REVIEW_STATE_PATH}` is non-empty; skip only for the legacy direct-invocation path with no receiver). This is the cross-run cache the next CI run reads in step 2. Schema v1 — see `partials/incremental-review.md` for the full field list and skip/refresh/full-review examples:

```yaml
version: 1
diff_fingerprint: <sha256 of gh pr diff> # see partials/incremental-review.md
pr_classification: routine-update # routine-update | standard
verdict: APPROVE # carried over unchanged on skip/refresh, updated on full review
follow_ups: 3
review_url: https://github.com/${REPO}/pull/${PR_NUMBER}#pullrequestreview-XXXX
skipped: false # true when this run posted nothing new
unresolved_thread_ids: [] # thread node IDs still open, for the next run's context
```

13. **Emit review metrics** (REQUIRED; receiver gates final check on this file's presence).
Write a yaml file at `${REVIEW_METRICS_PATH}` (the receiver workflow exports this env var). Schema v1:

```yaml
version: 1
verdict: APPROVE # one of: APPROVE | REQUEST_CHANGES | COMMENT
follow_ups: 3 # integer count of P0/P1/P2 follow-ups raised in the review
review_url: https://github.com/${REPO}/pull/${PR_NUMBER}#pullrequestreview-XXXX
skipped: false # true when step 2 decided to skip/refresh without posting a new review
```

Use the `Write` tool to create `${REVIEW_METRICS_PATH}` directly (it is permitted under the
Expand All @@ -68,12 +85,13 @@ Do not duplicate questions from past reviews. Respond to engagement on your prev
verdict: COMMENT
follow_ups: 5
review_url: ${REVIEW_URL}
skipped: false
```

If `${REVIEW_METRICS_PATH}` is empty (legacy direct-invocation path with no receiver), skip this step.

12. **Update the check run** (only when NOT invoked via the receiver — i.e. `${REVIEW_METRICS_PATH}` is empty).
When the receiver workflow owns the check_run lifecycle, it reads the metrics file emitted in step 11 and updates the check itself; don't write to it from here.
14. **Update the check run** (only when NOT invoked via the receiver — i.e. `${REVIEW_METRICS_PATH}` is empty).
When the receiver workflow owns the check_run lifecycle, it reads the metrics file emitted in step 13 and updates the check itself; don't write to it from here.
- APPROVE -> `success`, COMMENT -> `action_required`, REQUEST_CHANGES -> `action_required`
(COMMENT is deliberately NOT `neutral` — GitHub treats `neutral` as a passing state for
required status checks, so it wouldn't block merge on an unapproved PR. Only APPROVE passes.)
Expand All @@ -87,25 +105,30 @@ Do not duplicate questions from past reviews. Respond to engagement on your prev
fi
```

13. **Post-review verification**: Verify your latest review is visible, previous reviews minimized, thread states are correct, other users' threads untouched, the metrics file written (step 11), and the check run updated (either by you in step 12 OR by the receiver workflow that invoked you).
15. **Post-review verification**: Verify your latest review is visible (or, on skip/refresh, that nothing new was posted and the prior review/approval is untouched), previous reviews minimized (full-review path only), thread states are correct, other users' threads untouched, the review state written (step 12), the metrics file written (step 13), and the check run updated (either by you in step 14 OR by the receiver workflow that invoked you).

## Design Principles

Design principles (KISS, YAGNI, DRY, incremental development, etc.) are provided by the `common-sense` plugin via the project's enabled plugins in `.claude/settings.json`.

## Incremental review (skip / brief / full)

See `partials/incremental-review.md` for PR classification, the prior-state cache schema, and the skip / brief-refresh / full-re-review decision tree. This is REQUIRED reading before step 2.

## Formatting

See `partials/review-formatting.md` for emoji legend, badge requirements, review structure template, and footnote formatting.
See `partials/review-formatting.md` for emoji legend, badge requirements, review structure template (`standard` classification), brief-format template (`routine-update` classification), and footnote formatting.

## Critical Rules

- Never post test/progress comments. Only post your final review.
- Never post "detailed review at <url>". Post the FULL review in the PR.
- Review MUST use `<details>` / `<summary>` HTML tags for collapsible detail.
- A `standard`-classification review MUST use `<details>` / `<summary>` HTML tags for collapsible detail. A `routine-update` brief review does not need them — see `partials/review-formatting.md`.
- Review MUST detail how you arrived at your conclusions and scores.
- Do not base review on CI output. Review the code itself.
- Use repo documentation (AGENTS.md, .claude/rules/, CLAUDE.md, README.md) for style guidance.
- If you need a tool that isn't available, call it out outside the details block.
- Never dismiss a prior approval, and never post anything, on a "skip" or "brief refresh with nothing new" decision — see `partials/incremental-review.md`.

<job-context>
${JOB_CONTEXT}
Expand Down
Loading
Loading