with one click
codex-review-code
// Review non-trivial implementation changes for quality and regression risk before completion or merge.
// Review non-trivial implementation changes for quality and regression risk before completion or merge.
[HINT] Download the complete skill directory including SKILL.md and all related files
| name | codex-review-code |
| description | Review non-trivial implementation changes for quality and regression risk before completion or merge. |
| context | fork |
This is the default Review-stage owner for non-trivial code changes.
analysisContext.* (structured state)context.md (path: analysisContext.artifacts.contextDocPath)executionRuntime must be resolved before running this skill.
claude-code: use mcp__codex__codex or an equivalent isolated review path when available; keep the caller session as coordinator.codex: prefer a fresh forked review session or equivalent isolated attempt; keep the main session as coordinator and merge back only the structured review summary..claude/scripts/verify-code-policy.sh as the hard gate for machine-checkable code policy violations.code-review-graph as the default source for changed-file review context, impact radius, and caller/importer/test hints before broad file reading..claude/docs/guidelines/code-review-graph-workflow.md: stage-gated, lazy update, summary-only evidence.Codex-native review should explicitly apply:
.claude/rules/quality.md.claude/rules/security.md.claude/rules/coding-style.md.claude/rules/refactoring-guidelines.md.claude/rules/communication.md.claude/rules/output-format.md.claude/docs/guidelines/external-skill-pattern-transfer.mdDetermine runtime and select execution path:
codex -> prefer a forked review path first; use current-session review only if the runtime cannot preserve isolation.claude-code -> verify Codex MCP availability:// Try a simple MCP call to check availability
try {
mcp__codex__codex({
prompt: "ping",
sandbox: "read-only",
cwd: process.cwd()
})
// If successful, MCP is available
} catch (error) {
// MCP not available - proceed with Claude fallback
}
MCP Unavailable Conditions:
Summarize change scope, changed files, and key behaviors
Capture the context.md path (default: {tasksRoot}/{feature-name}/context.md), then use code-review-graph detect changes/review context/impact radius when available to reduce relevant code reads
Build delegation prompt using the 7-section format below with two review stages:
coverage_findings: surface every plausible issue, including low-severity or low-confidence items, each with confidence and severityranking_decision: rank/deduplicate findings and produce the merge decisionIf an isolated review path is available (from Step 1):
If MCP is unavailable (from Step 1):
"codex-fallback: Claude performed review directly (isolated reviewer unavailable)"If runtime is codex:
"codex-fork-review: isolated review executed in Codex runtime"7a. If runtime is codex and isolation is unavailable:
"codex-fallback-in-session: review isolation unavailable".claude/docs/guidelines/document-memory-policy.md: Store full review in archives/review-v{n}.md, keep only short summary in context.mdaccepted, challenged, deferred, or needs_clarification.QA_REPORT.md records the disposition for each meaningful finding.Use the 7-section format:
TASK: Review implementation at [context.md path] for [focus areas: correctness, security, performance, maintainability].
EXPECTED OUTCOME: Issue list with verdict and recommendations.
CONTEXT:
- Code to review: [file paths or snippets]
- Purpose: [what this code does]
- Recent changes:
* [Changed files list]
* [Key behaviors summary]
- Feature summary: [brief description]
CONSTRAINTS:
- Project conventions: [existing patterns to follow]
- Technical stack: [languages, frameworks]
MUST DO:
- Stage A coverage-first finding:
* Report every plausible correctness, security, performance, maintainability, test, or user-visible behavior issue.
* Include low-severity and uncertain findings instead of silently dropping them.
* Attach `confidence` (`low|medium|high`) and `estimatedSeverity` (`critical|high|medium|low`) to each finding.
* Do not filter for importance in Stage A; downstream ranking handles filtering.
- Stage B ranking:
* Prioritize: Correctness -> Security -> Performance -> Maintainability.
* Deduplicate findings and produce the final verdict.
- **Security Checks (CRITICAL)**:
* Hardcoded credentials (API keys, passwords, tokens)
* SQL injection risks (string concatenation in queries)
* XSS vulnerabilities (unescaped user input)
* Missing input validation
- **Code Quality (HIGH)**:
* Long functions (>50 lines)
* Deep nesting (>4 levels)
* Missing error handling (try/catch)
* Repeated or systemic policy violations that indicate weak module boundaries
* Shallow pass-through modules that fail the deletion test
* Public interfaces that expose implementation complexity or are easy to misuse
* Domain/user-facing behavior described with inconsistent terminology
- **React/Next.js Performance (CRITICAL)** [if signals.reactProject]:
* Sequential await instead of Promise.all() (waterfall pattern)
* Barrel file imports (`import { X } from 'lib'` → direct import)
* Missing dynamic imports for heavy components
* RSC serialization: passing entire objects instead of needed fields
* Missing Suspense boundaries for async components
Reference: `.claude/skills/vercel-react-best-practices/SKILL.md`
- In Stage B, focus on issues that matter, not style nitpicks
- Check logic/flow errors and edge cases
- Validate type safety and error handling
- Verify API contract and data model consistency
- For refactors, check that each step keeps the codebase working and that tests target behavior through public interfaces
MUST NOT DO:
- Nitpick style (let formatters handle this)
- Drop plausible findings during Stage A only because they are uncertain or low severity
- Promote theoretical concerns unlikely to matter above concrete defects during Stage B
- Suggest changes outside the scope of modified files
- Require a new abstraction only because code moved; require evidence of better locality, leverage, or testability
OUTPUT FORMAT:
Summary -> Coverage findings -> Ranked issues -> Warnings -> Recommendations -> Verdict
## Approval Criteria (Fix Forward Policy)
- ✅ **APPROVE**: No issues
- ⚠️ **FIX-FORWARD**: HIGH issues → merge allowed + follow-up task 생성
- ⚠️ **MERGE-NOTE**: MEDIUM issues → merge allowed + notes 기록
- ❌ **REJECT**: CRITICAL issues only (보안/데이터 무결성)
mcp__codex__codex({
prompt: "[7-section delegation prompt with full context]",
"developer-instructions": "[contents of code-reviewer.md]",
sandbox: "read-only", // Advisory mode - review only
cwd: "[current working directory]"
})
When MCP is not available, Claude performs the review directly:
When running in Codex runtime, execute review in a fresh isolated review boundary:
"codex-fork-review: isolated review executed in Codex runtime"If isolation is unavailable, degrade to the current session only as a documented fallback.
If you want the expert to fix issues automatically:
mcp__codex__codex({
prompt: "[same 7-section format, but add: 'Fix the issues found and verify the changes']",
"developer-instructions": "[contents of code-reviewer.md]",
sandbox: "workspace-write", // Implementation mode - can modify files
cwd: "[current working directory]"
})
For runtime=codex, prefer a fresh isolated implementation boundary when possible. Use the current session only as an explicit fallback and preserve the same verification requirements.
notes:
- "codex-review: [APPROVE/FIX-FORWARD/MERGE-NOTE/REJECT], critical=[count], high=[count], warnings=[count]"
# If fallback was used:
- "codex-fallback: Claude performed review directly (isolated reviewer unavailable)"
# If Codex runtime isolated path was used:
- "codex-fork-review: isolated review executed in Codex runtime"
# If isolation degraded to current session:
- "codex-fallback-in-session: review isolation unavailable"
# Fix Forward Tasks (HIGH issues that allow merge with follow-up)
fixForward:
tasks:
- issue: "Long function in paymentService.ts (62 lines)"
severity: HIGH
file: "src/services/paymentService.ts"
suggestion: "Extract coupon validation to separate function"
# empty if no HIGH issues
qaReport:
coverageFindings:
- finding: "Potential route shadowing on reorder endpoint"
confidence: low | medium | high
estimatedSeverity: critical | high | medium | low
reviewFindingDecisions:
- finding: "Route shadowing on reorder endpoint"
decision: accepted | challenged | deferred | needs_clarification
rationale: "422 reproduced in current run; route order bug confirmed."
APPROVE → Proceed to next stepFIX-FORWARD (HIGH issues) → Merge allowed, create follow-up tasks in fixForward.tasks[]MERGE-NOTE (MEDIUM issues) → Merge allowed, record in notesREJECT (CRITICAL issues) → Enter Auto-Fix Loopsandbox: "workspace-write"reviewFixLoop:
enabled: true
maxRetries: 2
fixableIssues:
- console.log statements
- missing error handling
- type errors
- simple security issues (hardcoded strings)
nonFixableIssues:
- architectural changes
- breaking API changes
- complex security vulnerabilities
When entering fix mode, add to prompt:
Fix the following issues and verify the changes:
1. [Issue description from review]
2. [Issue description from review]
After fixing, run verification to confirm the issues are resolved.