Om Auto Review Pr

Auto Review PR

Review a GitHub pull request by number without touching the current worktree. Always fetch the exact PR from GitHub, review it in an isolated worktree, submit the verdict, and if the PR still has blockers offer an explicit autofix flow that keeps resolving conflicts, fixing code, testing, typechecking, and re-reviewing until the PR is actually ready or a non-actionable blocker remains.

Arguments

• {prNumber} (required) — the PR number to review or re-review (for example 1234)
• --force (optional) — bypass the in-progress concurrency check; use when intentionally taking over a PR that another auto-skill or human already claimed

Workflow

0. In-progress concurrency check (claim the PR)

Auto-skills MUST NOT clobber each other. Before doing anything else, decide whether you may claim this PR.

CURRENT_USER=$(gh api user --jq '.login')
gh pr view {prNumber} --json assignees,labels,number,title,comments

A PR is considered already in progress when ANY of the following is true:

• It carries the in-progress label
• It has at least one assignee whose login is not $CURRENT_USER
• A claim comment newer than 30 minutes exists from another actor (look for the 🤖 start marker)

Decision tree:

| State | --force set? | Action | |-------|---------------|--------| | Not in progress | — | Claim and proceed | | In progress, current user owns the lock | — | Treat as re-entry; proceed without re-claiming | | In progress, someone else owns the lock | no | STOP. Ask the user via AskUserQuestion: "PR #{prNumber} is in progress (owner: {owner}, signal: {label/assignee/comment}). Override and continue?" Only continue when the user explicitly says yes. | | In progress, someone else owns the lock | yes | Post a force-override comment naming the previous owner, then claim and proceed |

Stale lock recovery:

• If the in-progress label is older than 60 minutes and the assignee did not push or comment in that window, treat it as expired. Still ask the user before overriding unless --force was set.

Claim the PR (only after the check above passes)

gh pr edit {prNumber} --add-assignee "$CURRENT_USER"

# Apply the in-progress label via the same GraphQL flow used for pipeline labels
# (kept atomic with the pipeline label transitions in step 8)

gh pr comment {prNumber} --body "🤖 \`auto-review-pr\` started by @${CURRENT_USER} at $(date -u +%Y-%m-%dT%H:%M:%SZ). Other auto-skills will skip this PR until the lock is released."

The release step happens in step 11 — the lock MUST be released even on failure.

1. Fetch PR metadata and reviewer context

Use GitHub as the source of truth. Collect enough data to decide whether this is a first review or a re-review and whether the PR comes from a fork.

gh pr view {prNumber} --json number,title,url,author,baseRefName,baseRefOid,headRefName,headRefOid,headRepository,headRepositoryOwner,isCrossRepository,maintainerCanModify,mergeable,mergeStateStatus,reviewDecision,labels,latestReviews,reviews,commits,files
gh api user --jq '.login'

Capture at least:

• PR title, URL, base branch, head branch, head SHA
• author login
• whether the PR is cross-repository (isCrossRepository)
• whether maintainers can modify it (maintainerCanModify)
• existing labels
• existing reviews by the current reviewer

2. Decide whether this is a review or a re-review

Treat the run as a re-review when the current reviewer has already submitted a review on the PR. Use reviews first and latestReviews as a fallback.

Rules:

• If there is no prior review from the current reviewer, this is a normal review.
• If there is a prior review from the current reviewer and the PR head SHA changed after that review, this is a re-review of updated code.
• If there is a prior review from the current reviewer and the head SHA did not change, only continue when the user explicitly asked for a re-review. Otherwise, stop and report that there are no new commits to review.

When re-reviewing:

• Title the report Re-review: {PR title} instead of Code Review: {PR title}.
• Re-check all previous blocker areas before approving.
• Replace labels idempotently just like a first review.
• Submit a fresh review rather than assuming the previous review still applies.

3. Early-exit checks

Run these checks before the worktree is created. If either fails, skip the full code review and go straight to the changes-requested flow.

3a. Check for merge conflicts

gh pr view {prNumber} --json mergeable,mergeStateStatus,baseRefName

If mergeable is CONFLICTING or mergeStateStatus is DIRTY, do not continue with checkout or review execution on the first pass.

Submit a changes-requested review with a conflict-focused body, apply the changes-requested label, remove merge-queue, and stop the first pass.

Important:

• On the initial review pass, conflicts are still an early stop.
• On the second pass, if the user approves autofix, conflicts become actionable work and must be resolved inside the isolated worktree or carry-forward branch before re-reviewing.

3b. Check CI status

Discover required checks first:

gh api repos/{owner}/{repo}/branches/{baseRefName}/protection/required_status_checks --jq '.contexts[]' 2>/dev/null

If the branch protection API returns 404, treat all reported PR checks as required.

Fetch the actual PR check results:

gh pr checks {prNumber} --json name,state,link

Treat these states as failing:

• FAILURE
• ERROR
• CANCELLED
• TIMED_OUT

Ignore these as non-failing:

• PENDING
• SUCCESS
• SKIPPED
• NEUTRAL

If any required check is failing, do not continue with checkout or review execution. Submit a changes-requested review listing only the failing required checks, apply changes-requested, remove merge-queue, and stop.

4. Create an isolated worktree for the PR

Never review directly in the repository’s primary worktree.

First detect whether you are already inside a linked worktree:

REPO_ROOT=$(git rev-parse --show-toplevel)
GIT_DIR=$(git rev-parse --git-dir)
GIT_COMMON_DIR=$(git rev-parse --git-common-dir)
WORKTREE_PARENT="$REPO_ROOT/.ai/tmp/auto-review-pr"
CREATED_WORKTREE=0

if [ "$GIT_DIR" != "$GIT_COMMON_DIR" ]; then
  WORKTREE_DIR="$PWD"
else
  WORKTREE_DIR="$WORKTREE_PARENT/pr-{prNumber}-$(date +%Y%m%d-%H%M%S)"
  mkdir -p "$WORKTREE_PARENT"
  git fetch origin "pull/{prNumber}/head"
  PR_HEAD_SHA=$(git rev-parse FETCH_HEAD)
  git worktree add --detach "$WORKTREE_DIR" "$PR_HEAD_SHA"
  CREATED_WORKTREE=1

  cd "$WORKTREE_DIR"
  git switch -c "review/pr-{prNumber}"
fi

If you reused an existing linked worktree, repoint it deliberately to the PR branch or a fresh local branch for that PR before continuing. If you created a new worktree, use the GitHub pull ref so the checkout works for both same-repo PRs and fork PRs.

After selecting the worktree, ensure you are on the correct PR branch context:

cd "$WORKTREE_DIR"
git fetch origin "pull/{prNumber}/head"
git checkout -B "review/pr-{prNumber}" FETCH_HEAD
git fetch origin "{baseRefName}"

Rules:

• If you are already in a linked worktree, reuse it instead of creating a nested worktree.
• The repository’s main worktree must remain untouched.
• Review, testing, and any optional follow-up fixes must happen inside the isolated worktree.
• Always clean up the temporary worktree at the end, even on failure, but only if you created it in this run.

Before running any Yarn-based validation in the new worktree, restore the package-manager install state:

yarn install --mode=skip-build

If --mode=skip-build is unavailable in the current Yarn version, run plain yarn install.

Cleanup sequence:

cd "$REPO_ROOT"
if [ "$CREATED_WORKTREE" = "1" ]; then
  git worktree remove --force "$WORKTREE_DIR"
fi

4a. Check for duplicated or already-merged changes

Before proceeding with the full review, verify that the PR does not duplicate work already present in the base branch. This catches cases where:

• The base branch already contains the same fix (e.g., merged via a different PR)
• A parallel PR landed the same feature while this one was open
• The PR's changes are a subset of recently merged work

Steps:

1. Get the list of changed files from the PR diff:

``bash gh pr diff {prNumber} --name-only ``

1. For each changed file, compare the PR version against the base branch version to identify overlap:

``bash git diff origin/{baseRefName} -- ``

1. Check recent commits on the base branch that touch the same files:

``bash git log origin/{baseRefName} --oneline -20 -- ``

1. Look for semantic duplication — the same logic, function, or fix already present in the base branch even if the code differs slightly.

If the PR's core changes are already present in the base branch:

• Submit a changes-requested review explaining that the changes duplicate already-merged work.
• List the specific commits or PRs in the base branch that already contain the equivalent changes.
• Apply changes-requested, remove merge-queue, and stop.

If partial overlap exists (some changes are new, some are redundant):

• Note the redundant parts as a finding in the review.
• Continue reviewing the genuinely new changes.

5. Diff-level automated checks

Before running the full code-review skill, scan the PR diff for hard-rule violations. Use:

gh pr diff {prNumber}
gh pr diff {prNumber} --name-only

Record findings from the patterns below. These are mandatory findings, not optional heuristics.

Critical auto-detections

| Pattern in diff | Finding | |-----------------|---------| | Removed or renamed event ID in any events.ts | Critical: event ID is a frozen contract surface | | Removed or renamed widget spot ID in injection-table.ts | Critical: spot ID is a frozen contract surface | | Removed field from an API response schema or zod response type | Critical: response fields are additive-only | | Renamed or removed a database column or table in a migration | Critical: DB schema is additive-only | | Removed a public import path without re-export bridge | Critical: import paths require deprecation protocol | | Missing organization_id or tenant_id filter on a tenant-scoped query | Critical: tenant isolation breach |

High auto-detections

| Pattern in diff | Finding | |-----------------|---------| | findWithDecryption or findOneWithDecryption replaced with raw em.find or em.findOne | High: encryption helpers must not be downgraded | | New API route file missing export const openApi or export const metadata | High: required exports for auto-discovery | | New subscriber or worker file missing export const metadata | High: required exports for auto-discovery | | Raw fetch( call in UI or backend page code, outside tests | High: must use apiCall or apiCallOrThrow | | New raw em.findOne( or em.find( in non-test production code (grep the diff: gh pr diff {prNumber} \| grep "^+" \| grep -v "test\." \| grep -v "__tests__" \| grep "em\.find") | High: must use findOneWithDecryption/findWithDecryption from @open-mercato/shared/lib/encryption/find | | Behavior change with no corresponding test file in the diff | High: behavior changes must include tests |

Medium auto-detections

| Pattern in diff | Finding | |-----------------|---------| | Hardcoded user-facing string in API errors or UI labels | Medium: must use i18n | | New any type annotation outside tests | Medium: use zod plus z.infer | | alert( or custom toast instead of flash() | Medium: use flash() | | Hand-written migration SQL file | Medium: never hand-write migrations | | Entity schema changed but no migration file in the diff | Medium: run yarn db:generate | | Missing explicit tenant scoping in sub-entity queries | Medium: defense in depth | | New or modified i18n locale JSON keys not in alphabetical order | Medium: CI i18n-check-sync requires sorted keys — run yarn i18n:check-sync --fix or sort manually |

Low auto-detections

| Pattern in diff | Finding | |-----------------|---------| | One-letter variable name outside loop counters i, j, k | Low: use descriptive names | | Inline comment on self-explanatory code | Low: remove comment | | Added docstring or comment on unchanged function | Low: do not annotate unchanged code |

6. Run the full code-review skill inside the worktree

Execute skills/om-code-review/SKILL.md in the isolated worktree.

Mandatory scope and gates:

• Scope changed files with gh pr diff {prNumber} --name-only
• Gather context from all matching AGENTS.md files, related specs, and .ai/lessons.md
• Run the full CI/CD verification gate
• Run yarn template:sync
• Check BACKWARD_COMPATIBILITY.md
• Apply the full review checklist
• Verify test coverage and cross-module impact

Merge findings from step 5 into the final review report. Do not duplicate the same issue twice.

6a. Run DS Guardian gate (UI-touching PRs only)

If the PR diff touches any .tsx or .ts files under packages/, apps/, or any module's backend/ / frontend/ / components/ directories, run the DS Guardian gate. The gate has two phases — a deterministic grep pass that runs first, then an LLM-augmented review that consumes the grep findings as input.

Phase 1 — Deterministic grep gate (fast path, ~5s).

# Detect UI-touching files
UI_FILES=$(gh pr diff {prNumber} --name-only \
  | grep -E '\.(tsx|ts)$' \
  | grep -E '(packages/|apps/).*((backend|frontend|components|widgets|primitives)/|\.tsx$)' \
  | grep -v '__tests__' \
  | grep -v '\.test\.' \
  | grep -v '\.spec\.')

# Run the deterministic linter. Output is one finding per line:
#   :::
# Empty stdout means no greppable violations — the LLM phase still runs for
# judgment cases (decoration vs status, primitive choice, missing empty/
# loading states), but it gets a clean slate instead of having to redetect.
GREP_FINDINGS=""
if [ -n "$UI_FILES" ]; then
  GREP_FINDINGS=$(echo "$UI_FILES" | bash skills/om-ds-guardian/scripts/ds-diff-check.sh)
fi

If $UI_FILES is empty, skip the rest of step 6a entirely.

Phase 2 — LLM-augmented REVIEW (judgment + fix language).

Execute skills/om-ds-guardian/SKILL.md Capability 4 (REVIEW) against $UI_FILES in the isolated worktree, passing $GREP_FINDINGS as known-violations input. The LLM's contract changes from "detect everything" to "augment what grep already caught and add what grep can't see":

• For every finding in $GREP_FINDINGS: confirm severity per the mapping below, attach a fix recipe from references/token-mapping.md, and downgrade or drop only with an explicit reason (e.g. "decorative use, not status semantics — DS-SKIP").
• Then scan the diff for what grep cannot see:
• Decoration vs status — is a flagged hardcoded color decorative (chart legend, brand illustration) or status-semantic (form error, success badge)?
• Primitive choice — was ` chosen where ` would express the model better, or vice versa?
• Missing empty/loading states — list pages with DataTable but no EmptyState/LoadingMessage.
• Color-as-only-info — status conveyed by color without a label or icon.
• IconButton without aria-label — when the icon is not decorative.
• Form structure — ` without / not wrapped in `.

Severity mapping (applies to both phases):

• Critical: hardcoded status colors, raw `//, missing empty/loading states, wrong selection-color contract (data-[state=checked]:bg-primary`)
• Medium: arbitrary text sizes, deprecated Notice, missing aria-label on non-decorative icon buttons, disabled:opacity-50, hardcoded brand hex, old focus rings (focus:ring-2 ring-offset-2)
• Low: inline SVG, minor inconsistencies

Merge the combined output into the same report under a "Design System" section. D

…

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: SHGrowth
• Source: SHGrowth/om-superpowers
• License: MIT

Install and usage instructions live in the source repository linked above.