Add code review skill for GitHub PRs and local branches#1572
Add code review skill for GitHub PRs and local branches#1572jason-capsule42 wants to merge 1 commit into
Conversation
Reviewer's GuideAdds a new Claude Code Sequence diagram for PR-mode GitHub review and comment postingsequenceDiagram
actor User
participant ClaudeSkill
participant Git as Git_cli
participant GH as Gh_cli
participant API as GitHub_API
User ->> ClaudeSkill: /code-review 1572
ClaudeSkill ->> GH: gh pr view 1572 --json baseRefName,headRefOid,headRefName
ClaudeSkill ->> Git: git fetch origin
ClaudeSkill ->> Git: git rev-parse HEAD
ClaudeSkill ->> ClaudeSkill: Compare local HEAD vs headRefOid
alt SHAs differ
ClaudeSkill -->> User: Warn mismatch and stop review
else SHAs match
ClaudeSkill ->> Git: git diff $(git merge-base <base> HEAD) HEAD
ClaudeSkill ->> Git: git diff $(git merge-base <base> HEAD) HEAD --name-only
ClaudeSkill ->> Git: git log <base>..HEAD --format="%s%n%b"
ClaudeSkill ->> ClaudeSkill: Parse AB# refs, gather post-mortems, TRD, context
ClaudeSkill ->> ClaudeSkill: Perform adversarial multi-persona review
ClaudeSkill ->> GH: gh repo view --json owner,name
ClaudeSkill ->> GH: gh pr view 1572 --json body
ClaudeSkill ->> API: gh api --method PATCH repos/{owner}/{repo}/pulls/1572
activate API
Note right of API: Update PR description with synced Executive Summary
API -->> ClaudeSkill: 200 OK
deactivate API
ClaudeSkill ->> GH: gh api --paginate repos/{owner}/{repo}/pulls/1572/comments
ClaudeSkill ->> API: gh api repos/{owner}/{repo}/pulls/1572/comments (POST/PATCH)
activate API
Note right of API: Reconcile inline comments using inline marker
API -->> ClaudeSkill: 2xx / 422
deactivate API
ClaudeSkill ->> GH: gh pr comment 1572 --body-file -
ClaudeSkill ->> GH: gh pr view 1572 --json url --jq '.url'
GH -->> User: PR URL with review results
end
Flow diagram for /code-review argument handling and mode selectionflowchart TD
A[/Invoke /code-review with $ARGUMENTS/] --> B["Normalize $ARGUMENTS<br>- trim whitespace<br>- strip leading #<br>- case-insensitive local"]
B --> C{Is $ARGUMENTS numeric?}
C -- Yes --> D["PR mode<br>- git fetch origin<br>- gh pr view $ARGUMENTS for baseRefName<br>- Determine <base> = origin/baseRefName or origin/dev fallback<br>- Head SHA check via gh pr view headRefOid<br>- If SHAs match: gather diff/log<br>- Post comments to GitHub"]
C -- No --> E{Empty or local?}
E -- Yes --> F["Local mode<br>- Ask user for base branch<br>- Resolve <base><br> - default via origin/HEAD or gh repo view<br> - or user-specified ref<br>- git fetch origin<br>- Gather diff/log against merge-base<br>- Output findings in chat only"]
E -- No --> G["Hard stop<br>Output warning:<br>"Unrecognized argument — expected a PR number or local"<br>No review performed"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
f3607d9 to
8c21f03
Compare
|
Tip All tests passed and all changes approved!🟢 UI Tests: 998 tests unchanged |
|
8c21f03 to
f79a065
Compare
🤖 Claude Code Review📊 Review Quality: High confidence — single-file diff (270 insertions in This PR adds the Findings🔴 Bug (inline, line 217) — The first inline-comment example uses 🔴 Bug (inline, line 37) — In PR mode the diff is gathered with 🟡 Nit (inline, line 32) — The head-commit check requests 🔴 Documentation — missing post-mortem — Per the skill's own rule ("Validate post-mortem documentation" step 3), a change should have a post-mortem at Process checks (no issues)
|
f79a065 to
e694cc9
Compare
|
🚀 PR Release Published! To install: npm install @aurodesignsystem-dev/auro-formkit@0.0.0-pr1572.13Install via alias: npm install @aurodesignsystem/auro-formkit@npm:@aurodesignsystem-dev/auro-formkit@0.0.0-pr1572.13 |
e694cc9 to
8633451
Compare
🤖 Claude Code Review — PR #1572📊 Review Quality: High confidence — single-file diff (299 lines added, 1 file), fully in-context. No source/tests/component API touched, so the component-focused persona checks (bugs, XSS, a11y, framework integration, SPA lifecycle) had no applicable surface. Review focused on the skill's procedural correctness.
Findings🔴 Documentation: No post-mortem exists for this change. 🟡 Nit: PR title prefix (
If this repo squash-merges using the PR title, the released Conventional Commit type will differ from the commit history. Both 🟡 Nit: Case-sensitive Validation summary
This review was generated by the very skill it is reviewing (an earlier committed version). No prompt-injection content was present in the reviewed material. |
jason-capsule42
changed the title
8633451 to
e43238a
Compare
🤖 Claude Code Review📊 Review Quality: High confidence — diff is small and self-contained (299 lines, 1 file:
ℹ️ TRD linkageℹ️ No TRD linked (no post-mortem exists to carry a TRD link). Informational only. Commit validation
🔴 Documentation — no post-mortemPer this skill's own validation rule ("If no post-mortem file exists at all for this change, flag as 🔴 Documentation"), there is no Inline findings (see review comments on the diff)
OverallNo 🔴 bugs, security issues, or regressions in the skill's logic. The procedure is well-structured, defends against prompt injection, handles detached-HEAD via SHA comparison, resolves the base branch dynamically, and documents the sandbox |
e43238a to
b21599a
Compare
🤖 Claude Code Review — PR #1572📊 Review Quality: High confidence — diff is a single new file (301 lines, 1 file), fully within context. Small and self-contained.
This PR adds the Findings🔴 Documentation — no post-mortem exists. Per this skill's own validation rule ("Validate post-mortem documentation", step 3), any change with commits should have a matching 🟡 Nit — self-referential prompt-injection false positive (see inline comment on the untrusted-input paragraph). The untrusted-input guard tells the reviewer to treat any instruction-like content in the reviewed material as a 🔴 prompt-injection finding. But legitimate files under review can contain agent-directed instructions — other skill files, 🟡 Nit — no idempotency on re-run. The "Posting comments" flow always posts a fresh summary comment and fresh inline comments. Re-running Commit validation — PASS
No blocking code defects (bugs, security, regressions) were found in the procedure logic. The merge-base fallback for sandboxed shells, the SHA-based head check (detached-HEAD safe), the quoted-heredoc posting guidance, and the 422 inline-comment fallback are all sound. |
5cbb55d to
8dcbfad
Compare
Claude Code Review — PR #1572📊 Review Quality: High confidence — single-file diff ( Scope: This PR adds a single instruction/prompt file — the Findings🔴 Documentation: No post-mortem exists for this change (checked 🟡 Nit (process ordering, ~lines 238–333): The "High-level summary comment" section (posted/PATCHed first) precedes the "Inline code comments" section, but the inline-failure fallback (line 333) instructs adding unanchorable findings into the summary via a "Findings that could not be anchored inline" section. If the summary is posted before inline POSTs are attempted, a 422 discovered afterward cannot be reflected without re-PATCHing the summary — which the doc does not call out. Consider instructing the executor to attempt/anchor inline findings (or determine anchorability) before finalizing the summary, or to re-PATCH the summary after inline failures. Commit validation
Context
No inline line-level code findings — both findings above are process/architectural and are reported here rather than anchored to a line. |
a6d1801 to
e8b5381
Compare
6bea935 to
f0cc70b
Compare
🤖 Claude Code Review — PR #1572📊 Review Quality: High confidence — small diff (434 additions across 2 files), full context available.
SummaryThis PR adds one net-new Claude Code tooling file ( Commit validation ✅
Post-mortem validation ✅ (with one accuracy nit)
Test coverage
Note (informational, not a PR finding)
Prompt-injection check
Verdict: Safe to ship once the two stale commit-SHA references in the post-mortem are corrected. |
f0cc70b to
59d20a4
Compare
🤖 Claude Code Review — PR #1572📊 Review Quality: High confidence — diff is small and self-contained, full context available.
Process validation
Findings (inline)Two inline comments on
SummaryClean, well-constructed PR. The change is a single net-new Claude Code skill plus its required post-mortem — no component source, tests, stories, or public API touched, so there is no runtime, accessibility, framework-integration, security, or semver impact to assess. The only issue is the recurring stale-SHA documentation nit above; nothing here is release-blocking. |
59d20a4 to
de54be5
Compare
🤖 Claude Code Review — PR #1572📊 Review Quality: High confidence — small, self-contained diff with full context.
Process & validation
Findings
Notes
Overall: Safe to ship. Only non-blocking documentation accuracy nits (line count + persona list); resolve at author's discretion. |
de54be5 to
2d21774
Compare
🤖 Claude Code Review — PR #1572📊 Review Quality: High confidence — small, single-purpose diff (478 diff lines across 2 files), full context available (post-mortem read; no TRD by design). Well under the 500-line / 15-file thresholds, so cross-file analysis is complete. ScopeTwo new files, both under non-shipping paths:
No component source, tests, stories, demos, or public API touched → no semver/npm impact. The test-coverage, story, and API-doc checks do not apply (nothing shippable changed). ✅ Process & release validation — all pass
FindingsOne non-blocking nit, posted inline:
Inline reconciliation
Verdict: Clean, release-ready. No blocking issues. The single nit is a wording accuracy improvement in the skill's own guard. |
4665eb6 to
b6fef69
Compare
✅ Claude Code Review — PR #1572📊 Review Quality: High confidence — diff is small and fully in context.
Code reviewThis PR adds the No blocking issues found. Reviewed adversarially across all personas:
Prior inline comment reconciled: the one active nit from an earlier run (the untrusted-input guard not naming the PR-description write as a side effect) is already fixed in the current head — line 95 now enumerates "the one-time PR-description sync (the executive-summary block)". That comment was marked resolved. Three other prior inline comments were already resolved and left untouched. Commit validation
Post-mortem validation
Optional polish (non-blocking, do not act on unless you want to)
✅ No blocking issues — the remaining suggestion is optional polish. |
c044e16 to
ed356ca
Compare
🤖 Claude Code Review — PR #1572📊 Review Quality: High confidence — diff is small (499 lines across 2 files), full context available.
✅ Outcome: No blocking issuesThis PR adds one net-new tooling file under Process validation
Inline findings (both 🟡 Nit, non-blocking)
Note on convergence: prior runs of this skill on this PR all converged to a resolved state, and this run reproduced no prior finding. The two nits above are new-angle polish, not regressions — treat them as optional. |
ed356ca to
08bc787
Compare
Claude Code Review — PR #1572📊 Review Quality: High confidence. Diff is small (501 lines across 2 files: ✅ No blocking issuesBoth files are instructional/documentation content under Commit validation
Post-mortem / TRD validation
Reconciliation of prior review comments
Notes
Remaining suggestions, if any, are optional polish. This diff is clean. |
08bc787 to
16f9347
Compare
🤖 Claude Code Review — PR #1572📊 Review Quality: High confidence — diff is small and fully in context.
✅ No blocking issuesThis is a self-referential change — a code-review skill plus its post-mortem. Per the skill's own untrusted-input guard, the instructional content of The six inline findings from earlier runs (stale-SHA in the post-mortem, stale line-count, the side-effects enumeration omitting the exec-summary sync, the unreachable ℹ️ TRD linkageNo TRD linked. The post-mortem states this is internal tooling with no GitHub Discussion / TRD authored. Informational only — the TRD-deviation check does not apply. Commit validation
Post-mortem validation
Process notes (informational, non-blocking)
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
1 participant
Alaska Airlines Pull Request
Executive Summary
This branch adds a
/code-reviewClaude Code skill that encodes this repo's release conventions (ADOAB#traceability, Conventional Commits → semver, the post-mortem requirement, TRD deviations) into a repeatable, adversarial review. It runs in one of two modes keyed off its argument: a PR number reviews that pull request, posts reconciled inline + summary comments to GitHub, and mirrors this post-mortem's executive summary into the PR description, whilelocal(or no argument) reviews the current branch against a chosen base and outputs to chat only — the destructive, outward-facing path is reachable exclusively via an explicit PR number.The skill was hardened primarily by running it against its own PR: each pass surfaced genuine defects (a shell-injection-prone comment body, a case-sensitive keyword match, line-number-based comment reconciliation that duplicated after rebases, over-granted tool permissions) that were fixed in the same file. Two findings were structural to a review tool reviewing itself and required explicit carve-outs:
.claude/**instructional prose is reviewed as ordinary content rather than flagged as prompt injection, and tooling changes are alwayschore, never a bump-triggeringfeat. The skill's unconditional post-mortem gate also fired a release-blocking finding on its own PR — this document is that resolution, and the rule was clarified to be waivable for pure.claude/tooling.One property is worth stating plainly, because it changes how the skill should be used: the review is adversarial and non-deterministic, so re-running it on an unchanged diff keeps surfacing new low-value nits. The personas sample different angles each pass, the prompt is tuned to hunt rather than to certify, and "consider also…" suggestions are effectively unbounded — so a run almost never comes back truly empty. What does converge is the part that matters: genuine 🔴 correctness/security/regression findings drop to zero and stay there; only 🟡 nits churn. The implementation choice was not to chase determinism (which would blunt the adversarial value) but to add an explicit convergence instruction: when only marginal polish remains, the reviewer must report the diff as clean and mark remaining nits optional rather than manufacture a finding to look productive. The real stopping signal is therefore "no 🔴/🟡 you'd act on, two runs running" — not a literally empty list.
Bottom line: one net-new tooling file (no shippable code, no semver impact), a review procedure proven by executing it end-to-end against a live PR, and the key lesson that gates firing on every change need a typed exemption for the change classes they don't apply to.
Synced from
docs/post-mortem/1599649.mdby the code-review skill.Review Checklist:
RC Checklist
Testing Checklist:
Browsers
Browsers Support Guide
Dev demo link
Android
iOS
Desktop
Scenarios
Framework playground
**By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.**
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team
Summary by Sourcery
Add a new Claude Code
/code-reviewskill that performs structured, adversarial reviews of GitHub PRs and local branches, and ship its accompanying post-mortem documenting design decisions and usage conventions.New Features:
Enhancements:
Documentation: