diff --git a/.agents/skills/codex-review/SKILL.md b/.agents/skills/codex-review/SKILL.md new file mode 100644 index 00000000000..b861a67d23c --- /dev/null +++ b/.agents/skills/codex-review/SKILL.md @@ -0,0 +1,99 @@ +--- +name: codex-review +description: "Codex code review closeout: local dirty changes, PR branch vs main, parallel tests." +--- + +# Codex Review + +Run Codex's built-in code review as a closeout check. This is code review (`codex review`), not Guardian `auto_review` approval routing. + +Use when: +- user asks for Codex review / autoreview / second-model review +- after non-trivial code edits, before final/commit/ship +- reviewing a local branch or PR branch after fixes + +## Contract + +- Treat review output as advisory. Never blindly apply it. +- Verify every finding by reading the real code path and adjacent files. +- Read dependency docs/source/types when the finding depends on external behavior. +- Reject unrealistic edge cases, speculative risks, broad rewrites, and fixes that over-complicate the codebase. +- Prefer small fixes at the right ownership boundary; no refactor unless it clearly improves the bug class. +- If a review-triggered fix changes code, rerun focused tests and rerun Codex review once. +- Do not push just to review. Push only when the user requested push/ship/PR update. + +## Pick Target + +Dirty local work: + +```bash +codex review --uncommitted +``` + +Branch/PR work: + +```bash +git fetch origin +codex review --base origin/main +``` + +If an open PR exists, use its actual base: + +```bash +base=$(gh pr view --json baseRefName --jq .baseRefName) +codex review --base "origin/$base" +``` + +Committed single change: + +```bash +codex review --commit HEAD +``` + +## Parallel Closeout + +Format first if formatting can change line locations. Then it is OK to run tests and review in parallel: + +```bash +scripts/codex-review --parallel-tests "" +``` + +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 once. + +## Context Efficiency + +Codex review is usually noisy. Default to a subagent filter when subagents are available. Ask it to run the review and return only: +- actionable findings it accepts +- findings it rejects, with one-line reason +- exact files/tests to rerun + +Run inline only for tiny changes or when subagents are unavailable. + +## Helper + +Bundled helper: + +```bash +~/.codex/skills/codex-review/scripts/codex-review --help +``` + +If installed from `agent-scripts`, path is: + +```bash +/Users/steipete/Projects/agent-scripts/skills/codex-review/scripts/codex-review --help +``` + +The helper: +- chooses dirty `--uncommitted` first +- otherwise uses current PR base if `gh pr view` works +- otherwise uses `origin/main` for non-main branches +- writes only to stdout unless `--output` or `CODEX_REVIEW_OUTPUT` is set +- supports `--dry-run` and `--parallel-tests` + +## Final Report + +Include: +- review command used +- tests/proof run +- findings accepted/rejected, briefly why +- whether review was rerun after review-triggered edits diff --git a/.agents/skills/codex-review/scripts/codex-review b/.agents/skills/codex-review/scripts/codex-review new file mode 100755 index 00000000000..0e6147528a7 --- /dev/null +++ b/.agents/skills/codex-review/scripts/codex-review @@ -0,0 +1,188 @@ +#!/usr/bin/env bash +set -euo pipefail + +usage() { + cat <<'EOF' +Usage: codex-review [options] + +Options: + --mode auto|local|branch Target selection. Default: auto. + --base REF Base ref for branch review. Default: PR base or origin/main. + --codex-bin PATH Codex binary. Default: codex. + --output FILE Also save output to file. + --parallel-tests CMD Run review and test command concurrently. + --dry-run Print selected commands, do not run. + -h, --help Show help. + +Modes: + local codex review --uncommitted + branch codex review --base + auto dirty tree -> local, else PR/current branch -> branch +EOF +} + +mode=auto +base_ref= +codex_bin=${CODEX_BIN:-codex} +output=${CODEX_REVIEW_OUTPUT:-} +parallel_tests= +dry_run=false + +while [[ $# -gt 0 ]]; do + case "$1" in + --mode) + mode=${2:-} + shift 2 + ;; + --base) + base_ref=${2:-} + shift 2 + ;; + --codex-bin) + codex_bin=${2:-} + shift 2 + ;; + --output) + output=${2:-} + shift 2 + ;; + --parallel-tests) + parallel_tests=${2:-} + shift 2 + ;; + --dry-run) + dry_run=true + shift + ;; + -h|--help) + usage + exit 0 + ;; + *) + usage >&2 + exit 2 + ;; + esac +done + +case "$mode" in + auto|local|branch) ;; + *) + echo "invalid --mode: $mode" >&2 + exit 2 + ;; +esac + +git rev-parse --show-toplevel >/dev/null + +current_branch=$(git branch --show-current 2>/dev/null || true) +dirty=false +if [[ -n "$(git status --porcelain)" ]]; then + dirty=true +fi + +pr_url= +if [[ -z "$base_ref" && "$mode" != local ]] && command -v gh >/dev/null 2>&1; then + if pr_lines=$(gh pr view --json baseRefName,url --jq '[.baseRefName, .url] | @tsv' 2>/dev/null); then + base_name=${pr_lines%%$'\t'*} + pr_url=${pr_lines#*$'\t'} + if [[ -n "$base_name" ]]; then + base_ref="origin/$base_name" + fi + fi +fi + +if [[ -z "$base_ref" ]]; then + base_ref=origin/main +fi + +review_kind= +if [[ "$mode" == local || ( "$mode" == auto && "$dirty" == true ) ]]; then + review_kind=local +elif [[ "$mode" == branch || ( "$mode" == auto && -n "$current_branch" && "$current_branch" != "main" ) ]]; then + review_kind=branch +else + echo "no review target: clean main checkout and no forced mode" >&2 + exit 1 +fi + +if [[ "$review_kind" == local ]]; then + review_cmd=("$codex_bin" review --uncommitted) +else + review_cmd=("$codex_bin" review --base "$base_ref") +fi + +printf 'codex-review target: %s\n' "$review_kind" +printf 'branch: %s\n' "${current_branch:-detached}" +if [[ -n "$pr_url" ]]; then + printf 'pr: %s\n' "$pr_url" +fi +printf 'review:' +printf ' %q' "${review_cmd[@]}" +printf '\n' +if [[ -n "$parallel_tests" ]]; then + printf 'tests: %s\n' "$parallel_tests" +fi +if [[ "$review_kind" == branch ]]; then + printf 'fetch: git fetch origin --quiet\n' +fi +if [[ -n "$output" ]]; then + printf 'output: %s\n' "$output" +fi + +if [[ "$dry_run" == true ]]; then + exit 0 +fi + +if [[ "$review_kind" == branch ]]; then + git fetch origin --quiet || { + echo "warning: git fetch origin failed; reviewing with existing refs" >&2 + } +fi + +run_review() { + if [[ -n "$output" ]]; then + mkdir -p "$(dirname "$output")" + "${review_cmd[@]}" 2>&1 | tee "$output" + else + "${review_cmd[@]}" + fi +} + +if [[ -z "$parallel_tests" ]]; then + run_review + exit $? +fi + +review_status_file=$(mktemp) +tests_status_file=$(mktemp) + +( + set +e + run_review + status=$? + printf '%s\n' "$status" > "$review_status_file" +) & +review_pid=$! + +( + set +e + bash -lc "$parallel_tests" + status=$? + printf '%s\n' "$status" > "$tests_status_file" +) & +tests_pid=$! + +wait "$review_pid" || true +wait "$tests_pid" || true + +review_status=$(cat "$review_status_file") +tests_status=$(cat "$tests_status_file") +rm -f "$review_status_file" "$tests_status_file" + +printf 'codex-review exit: %s\n' "$review_status" +printf 'tests exit: %s\n' "$tests_status" + +if [[ "$review_status" != 0 || "$tests_status" != 0 ]]; then + exit 1 +fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 7634420a8d8..a21f7060c22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai ### Changes +- Maintainer tooling: add a repo-local `codex-review` skill for Codex closeout reviews, including local dirty-work and PR-branch review helpers. - Maintainer tooling: fail CI when pull requests add package patch files or pnpm patched dependencies, preserving the upstream-and-bump dependency workflow. - Amazon Bedrock: externalize the Bedrock and Bedrock Mantle provider packages so core installs no longer pull AWS SDK dependencies unless those providers are installed. - Plugins: externalize Slack, OpenShell sandbox, and Anthropic Vertex so their runtime dependency cones install only when those plugins are installed.