|
| 1 | +--- |
| 2 | +name: autoreview |
| 3 | +description: "Autoreview closeout: local dirty changes, PR branch vs main, parallel tests." |
| 4 | +--- |
| 5 | + |
| 6 | +# Autoreview |
| 7 | + |
| 8 | +Run Codex's built-in code review as a closeout check. This is code review (`codex review`), not Guardian `auto_review` approval routing. |
| 9 | + |
| 10 | +Codex native review mode performs best and is recommended. Non-Codex reviewers are fallback/second-opinion paths that receive a generated diff prompt, not the full Codex review-mode runtime. |
| 11 | + |
| 12 | +Use when: |
| 13 | +- user asks for Codex review / autoreview / second-model review |
| 14 | +- after non-trivial code edits, before final/commit/ship |
| 15 | +- reviewing a local branch or PR branch after fixes |
| 16 | + |
| 17 | +## Contract |
| 18 | + |
| 19 | +- Treat review output as advisory. Never blindly apply it. |
| 20 | +- Verify every finding by reading the real code path and adjacent files. |
| 21 | +- Read dependency docs/source/types when the finding depends on external behavior. |
| 22 | +- Reject unrealistic edge cases, speculative risks, broad rewrites, and fixes that over-complicate the codebase. |
| 23 | +- Prefer small fixes at the right ownership boundary; no refactor unless it clearly improves the bug class. |
| 24 | +- Keep going until the selected review path returns no accepted/actionable findings. |
| 25 | +- If a review-triggered fix changes code, rerun focused tests and rerun the review helper. |
| 26 | +- Default to Codex review with no fallback. Prefer Codex for final closeout because it uses native review mode; non-Codex reviewers use a Codex-inspired generated diff prompt. Use `--fallback-reviewer auto|claude|pi|opencode|droid|copilot` only when a second-model fallback is explicitly wanted and authenticated. The helper runs nested Codex review in yolo/full-access mode by default; use `--no-yolo` only when intentionally testing sandbox behavior. |
| 27 | +- Stop as soon as the review command/helper exits 0 with no accepted/actionable findings. Do not run an extra direct `codex review` just to get a nicer "clean" line, a second opinion, or clearer closeout wording. |
| 28 | +- Treat the helper's successful exit plus absence of actionable findings as the clean review result, even if the underlying Codex CLI output is terse. |
| 29 | +- If rejecting a finding as intentional/not worth fixing, add a brief inline code comment only when it explains a real invariant or ownership decision that future reviewers should know. |
| 30 | +- If creating or updating a PR while rejecting any autoreview finding, record the rejected finding and reason in the PR description so later reviewers can distinguish intentional design decisions from missed review output. |
| 31 | +- Do not push just to review. Push only when the user requested push/ship/PR update. |
| 32 | +- For OpenClaw maintainers, keep autoreview validation Crabbox/Testbox-aware when maintainer validation mode is enabled (`OPENCLAW_TESTBOX=1` or `AUTOREVIEW_OPENCLAW_MAINTAINER_VALIDATION=1`). A review pass may inspect files and run cheap non-Node probes, but it must not start local `pnpm`, Vitest, `tsgo`, `npm test`, or `node scripts/run-vitest.mjs` from a Codex/worktree review unless the operator explicitly requested local proof. For runtime proof, use existing evidence or route through Crabbox/Testbox and report the id. Do not apply this rule to ordinary contributors who do not have maintainer Testbox access. |
| 33 | + |
| 34 | +## Pick Target |
| 35 | + |
| 36 | +Dirty local work: |
| 37 | + |
| 38 | +```bash |
| 39 | +codex review --uncommitted |
| 40 | +``` |
| 41 | + |
| 42 | +Use this only when the patch is actually unstaged/staged/untracked in the |
| 43 | +current checkout. For committed, pushed, or PR work, point Codex at the commit |
| 44 | +or branch diff instead; do not force `--mode local` / `--uncommitted` just |
| 45 | +because the helper docs mention dirty work first. A clean `--uncommitted` review |
| 46 | +only proves there is no local patch. |
| 47 | + |
| 48 | +Branch/PR work: |
| 49 | + |
| 50 | +```bash |
| 51 | +git fetch origin |
| 52 | +codex review --base origin/main |
| 53 | +``` |
| 54 | + |
| 55 | +Do not pass any prompt with `--base`, `--commit`, or `--uncommitted`. Codex CLI |
| 56 | +review targets and custom review prompts are mutually exclusive: target modes |
| 57 | +generate their own review prompt internally. Use plain target review for native |
| 58 | +Codex closeout, or use custom prompt review (`codex review -`) only when you |
| 59 | +intentionally want a generated diff prompt instead of native target review. |
| 60 | + |
| 61 | +If an open PR exists, use its actual base: |
| 62 | + |
| 63 | +```bash |
| 64 | +base=$(gh pr view --json baseRefName --jq .baseRefName) |
| 65 | +codex review --base "origin/$base" |
| 66 | +``` |
| 67 | + |
| 68 | +Committed single change: |
| 69 | + |
| 70 | +```bash |
| 71 | +codex review --commit HEAD |
| 72 | +``` |
| 73 | + |
| 74 | +or with the helper: |
| 75 | + |
| 76 | +```bash |
| 77 | +.agents/skills/autoreview/scripts/autoreview --mode commit --commit HEAD |
| 78 | +``` |
| 79 | + |
| 80 | +Use commit review for already-landed or already-pushed work on `main`. Reviewing |
| 81 | +clean `main` against `origin/main` is usually an empty diff after push. For a |
| 82 | +small stack, review each commit explicitly or review the branch before merging |
| 83 | +with `--base`. |
| 84 | + |
| 85 | +## Parallel Closeout |
| 86 | + |
| 87 | +Format first if formatting can change line locations. Then it is OK to run tests and review in parallel: |
| 88 | + |
| 89 | +```bash |
| 90 | +.agents/skills/autoreview/scripts/autoreview --parallel-tests "<focused test command>" |
| 91 | +``` |
| 92 | + |
| 93 | +Tradeoff: tests may force code changes that stale the review. If tests or review lead to code edits, rerun the affected tests and rerun review until no accepted/actionable findings remain. Once that rerun exits cleanly, stop; do not spend another long review cycle on redundant confirmation. |
| 94 | + |
| 95 | +## Context Efficiency |
| 96 | + |
| 97 | +Codex review is usually noisy. Default to a subagent filter when subagents are available. Ask it to run the review and return only: |
| 98 | +- actionable findings it accepts |
| 99 | +- findings it rejects, with one-line reason |
| 100 | +- exact files/tests to rerun |
| 101 | + |
| 102 | +Run inline only for tiny changes or when subagents are unavailable. |
| 103 | + |
| 104 | +## Helper |
| 105 | + |
| 106 | +Bundled helper: |
| 107 | + |
| 108 | +```bash |
| 109 | +.agents/skills/autoreview/scripts/autoreview --help |
| 110 | +``` |
| 111 | + |
| 112 | +The helper: |
| 113 | +- chooses dirty `--uncommitted` first |
| 114 | +- otherwise uses current PR base if `gh pr view` works |
| 115 | +- otherwise uses `origin/main` for non-main branches |
| 116 | +- auto-runs `PNPM_CONFIG_PM_ON_FAIL=ignore PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN=false PNPM_CONFIG_OFFLINE=true pnpm run check` in parallel when a repo has `package.json`, `pnpm-lock.yaml`, `node_modules`, and a `check` script; disable with `AUTOREVIEW_AUTO_TESTS=0` |
| 117 | +- use `--mode commit --commit <ref>` for already-committed work, especially clean `main` after landing |
| 118 | +- should be left in `--mode auto` or forced to `--mode branch` for PR/branch work; do not force `--mode local` after committing |
| 119 | +- supports `--reviewer codex|claude|pi|opencode|droid|copilot|auto`; `auto` means Codex first |
| 120 | +- supports `--fallback-reviewer auto|claude|pi|opencode|droid|copilot|none`; default is `none` |
| 121 | +- falls back only when Codex is unavailable or exits nonzero, not when Codex reports findings |
| 122 | +- writes only to stdout unless `--output` or `AUTOREVIEW_OUTPUT` is set |
| 123 | +- supports `--dry-run`, `--parallel-tests`, and commit refs |
| 124 | +- runs nested review with `--dangerously-bypass-approvals-and-sandbox --sandbox danger-full-access` by default |
| 125 | +- with `OPENCLAW_TESTBOX=1` or `AUTOREVIEW_OPENCLAW_MAINTAINER_VALIDATION=1`, disables auto local `pnpm run check` and routes Codex through generated prompt review (`codex review -`) so the no-local-heavy-tests policy is included; native Codex target review cannot accept extra prompt text |
| 126 | +- non-Codex reviewers receive the generated diff prompt and maintainer validation policy text when maintainer validation is active |
| 127 | +- keeps accepting `--full-access`; use `--no-yolo` or `AUTOREVIEW_YOLO=0` to opt out |
| 128 | +- still accepts legacy `CODEX_REVIEW_*` env vars when the matching `AUTOREVIEW_*` var is unset |
| 129 | +- prints `autoreview clean: no accepted/actionable findings reported` when the selected review command exits 0 |
| 130 | + |
| 131 | +## Final Report |
| 132 | + |
| 133 | +Include: |
| 134 | +- review command used |
| 135 | +- tests/proof run |
| 136 | +- findings accepted/rejected, briefly why |
| 137 | +- the clean review result from the final helper/review run, or why a remaining finding was consciously rejected |
| 138 | + |
| 139 | +Do not run another Codex review solely to improve the final report wording. If the final helper run exited 0 and produced no accepted/actionable findings, report that exact run as clean. |
| 140 | + |
| 141 | +## PR / CI Closeout |
| 142 | + |
| 143 | +- Prefer direct run/job APIs after CI starts: `gh run view <run-id> --json jobs`; use PR rollup only for final mergeability. |
| 144 | +- After rebase, compare `origin/main..HEAD`; drop CI-fix commits already upstream before pushing. |
| 145 | +- For prompt snapshot CI failures, prove/generate with Linux Node 24 before rerunning the failed job. |
| 146 | +- Update PR body once near the final head unless proof labels are missing or stale enough to block CI. |
0 commit comments