autoreview

Files (4)

• SKILL.md
• openai.yaml · agents/
• autoreview-target-state · bin/
• autoreview-target-state-test.rb · bin/

SKILL.md (first lines)

# Auto Review

Run a structured second-model review as a **closeout check** before commit, push, or ship,
then loop until the review reports no accepted/actionable findings. This is code review, not
PR merge/approval routing.

This skill is the discipline layer. It does not ship its own reviewer; it drives available
review tooling and adds the "verify every finding, fix at the right boundary, re-review until
clean" loop on top.

<!-- host-branch: available-tool start -->
- **Default engine: `codex review`**. It is a concrete CLI command and supports
  local/branch/commit review modes.
- **Alternative engine: Claude review tooling** when the current Claude Code environment provides
  it, such as `/code-review` or `/code-review ultra`. Treat these as environment-specific, not
  repo-local commands.
- **Cursor review tooling** when the current Cursor session provides `/review`,
  `/review-bugbot`, or `/review-security`. Treat these as availability-checked
  host tools, not portable requirements.
- For PR-comment triage (reacting to review comments already on a GitHub PR), use
  `.agents/skills/address-review/SKILL.md`; Claude Code exposes it as `/address-review`.
<!-- host-branch: available-tool end -->

Use when:

- user asks for "autoreview", "codex review", "Claude review", "second-model review",
  or a final review before commit/ship
- after non-trivial code edits, before the final commit/push/PR
- reviewing a local working tree, a branch, or a single landed commit after fixes

## Contract

This is the portable core. Hold it regardless of which engine runs.

Apply [Initial-Pass Optional-Nit Cutoff](../../workflows/pr-processing.md#initial-pass-optional-nit-cutoff)
before accepting findings in every engine/pass, including a final whole-branch
or second-engine review. Carry the existing phase into the review prompt and
triage; a rerun cannot create another optional repair pass.

- Treat review output as **advisory**. Never blindly apply it.
- Verify every finding by reading the real code path and adjacent files before acting.
- Read dependency docs/source/types when a finding depends on external library/framework behavior.
- Reject unrealistic edge cases, speculative risks, broad rewrites, and fixes that over-complicate the code.
- Reviewer severity informs triage; it does not by itself establish scope. An accepted expansion must map to an original acceptance criterion or a direct safety property.
- Stop the automatic fix loop when an accepted finding requires a new grammar, protocol, or schema category beyond the intended mechanism, or accepted findings broaden scope across two review waves. Re-evaluate proportionate alternatives: an authoritative source, a maintained dependency, a bounded guard, or a checklist with replay. Continue only with an option that preserves the required criterion or safety property; seek a decision when none is clearly proportionate.
- Prefer small fixes at the right ownership boundary; no refactor unless it clearly improves the bug class. This matches `AGENTS.md`: never refactor unrelated code.
- Structural drift ("is this making the codebase worse?" — file growth, scattered conditionals, thin abstractions, layer violations, feature-flag branching debt) is out of scope for this gate and does not loosen the rule above. It belongs to `structural-review`, a separate explicitly-invoked axis. Route a structural observation there instead of accepting a broader change here.
- Keep going until the review returns no accepted/actionable findings; once it comes back clean, stop. Do not run an extra review just to get nicer "clean" wording or a redundant second opinion.
- If a review-triggered fix changes code, rerun the focused tests for the changed surface and rerun the review.
- Security perspective is always included, but it must not cripple legitimate functionality. Report a security finding only when the change creates a concrete, actionable risk or removes an important safety check.
- When the diff touches `.github/workflows/**`, a composite `action.yml` / `action.yaml`, or `trusted_actions` in `.agents/agent-workflow.yml`, activate the `secure-github-actions` lens and run its read-only `bin/secure-github-actions-scan <repo-root>` gate from the trusted pack. Its clean result is necessary but not sufficient: still review permissions, triggers, untrusted checkout/execution, credential exposure, and whether each exact `trusted_actions` entry is justified. Never execute the changed workflow or action as part of this lens.
- Record a compact risk and coverage receipt for every completed non-trivial review: `autoreview` source, committed or uncommitted target kind, human base ref, immutable base SHA, head SHA, engine invocation, applied or unavailable risk lenses, included and excluded paths, and material limitations. Report actual coverage, not requested coverage. An uncommitted target is mutable, so its receipt is always partial or unknown with that limitation recorded.
- Treat P0/P1 findings, plus any lower-severity finding with material correctness, security, compatibility, data-loss, or release-process consequences, as consequential. Require an independent validation receipt before clearing or acting on one; primary-review agreement is not independent evidence.
- When independent validation of a consequential finding is unavailable, times out, or returns malformed evidence, keep the finding blocking or `unknown` and record validation as degraded. Never silently drop it.
- Be patient. `codex review` runs an external model when available and can take several minutes on a large diff. Progress that looks quiet is usually still working; do not kill it before about 5 minutes unless it has clearly errored.
- Do not launch multiple reviewers by default. One selected engine, one structured result, then verify it.
- A gated second-engine pass is appropriate only when the user asks or the diff falls into the
  high-risk / hosted-CI-ready / force-full hosted-CI / benchmark categories described by
  `.agents/agent-workflow.yml`. Run it after the primary review is
  clean, keep it to one extra pass, and verify its findings the same way.
- If you reject a finding as intentional/not worth fixing, add a brief inline code comment only when it documents a real invariant or ownership decision a future reviewer should know.
- **Do not push just to review.** Push only when the user asked for push/ship/PR. Follow `AGENTS.md` git boundaries (never force-push `main`/`master`).

## Step 1 - Pick the target

Inspect what changed and choose the diff scope. Resolve the base branch from
`.agents/agent-workflow.yml` key `base_branch`, or from PR metadata when a PR is
open.

```bash
base=$(ruby -ryaml -e 'p=(YAML.safe_load(File.read(".agents/agent-workflow.yml"), aliases: false) || {}); puts(p.fetch("base_branch", "main"))')
git status --short --untracked-files=all
git diff --name-only "origin/$base...HEAD"
git diff --stat "origin/$base...HEAD"
git diff --stat
git diff --cached --stat
git ls-files --others --exclude-standard
```

<!-- host-branch: available-tool start -->
Use these states when deciding the target. If available, resolve
`AUTOREVIEW_SKILL_DIR` to the installed or repo-local directory containing this
`SKILL.md`, then run the read-only helper:

```bash
AUTOREVIEW_SKILL_DIR="${AUTOREVIEW_SKILL_DIR:-.agents/skills/autoreview}"
"${AUTOREVIEW_SKILL_DIR}/bin/autoreview-target-state" --text
```

| State | Trigger | Disposition | Target |
| --- | --- | --- | --- |
| `LOCAL_UNTRACKED_ONLY` | Only untracked files are present. | ready | `codex review --uncommitted` |
| `LOCAL_DIRTY_ONLY` | Staged or unstaged local work is present without committed branch diff. | ready | `codex review --uncommitted` |
| `BRANCH_PLUS_DIRTY_LOCAL` | Committed branch diff and dirty local work both exist. | not_ready | Commit first, or run both branch and uncommitted reviews; staging alone does not put changes in the branch diff. |
| `BRANCH_PR_DIFF` | A branch diff exists and `gh pr view` found a PR base. | ready | `codex review --base "origin/$pr_base"` |
| `BRANCH_NO_PR_DIFF` | A branch diff exists and `gh pr view` reports no PR for the current branch. | ready | `codex review --base "origin/$base"`; this expected non-zero `gh` state is not a failure. |
| `NO_REVIEW_TARGET` | No dirty work and no committed branch diff. | not_ready | Stop or pick an explicit commit; a clean local review only proves there is no local patch. |
| `DETACHED_HEAD` | `HEAD` is detached. | blocked | Attach a branch or use `codex review --commit <sha>` intentionally. |
| `DEFAULT_BRANCH_WITH_LOCAL_COMMITS` | The configured base branch itself has local commits. | blocked | Create a feature branch or review the specific commit explicitly. |
| `PR_BASE_UNKNOWN` | PR base probing failed for reasons other than "no PR". | UNKNOWN | Resolve `gh` auth/network/state before selecting a branch target. |
| `BASE_DIFF_UNKNOWN` | Git cannot compare `origin/$base...HEAD`. | UNKNOWN | Fetch or repair the base ref before selecting a branch target. |

The state table is the source of truth for dirty local work, branch/PR work,
and branch plus dirty local work. Do not duplicate those target decisions
elsewhere in this skill.
<!-- host-branch: available-tool end -->
- **Single landed commit** (already on the configured base branch, or one commit in a stack): review
  that commit's diff (`git show <sha>`). Reviewing a clean base branch against its remote is an
  empty diff after push; point at the commit instead.

Tell the user which target you picked and why.

## Step 2 - Format and lint first

Consume applicable local results using
[Verification evidence reuse](../verify/references/verification-evidence.md).
Do not repeat a passing check merely because workflow stages changed; rerun
affected checks after changes and honor every repository-required repeat.
This reuses local command results only, not independent review verdicts.

From shakacode/agent-workflows, MIT.

More from shakacode/agent-workflows