Skill Staged Review

Two-Stage Review Pipeline

Separates spec compliance (did you build the right thing?) from code quality (did you build it right?). Stage 1 must pass before Stage 2 runs.

Stage 1: Spec Compliance

Validates the implementation against the intent contract.

Step 1: Load Intent Contract

PLAN_STORAGE="${CLAUDE_PLUGIN_ROOT:-${HOME}/.claude-octopus/plugin}/scripts/plan-storage.sh"
OCTO_PLAN_DIR="$("$PLAN_STORAGE" current "$PWD" 2>/dev/null || true)"
INTENT_FILE="${OCTO_PLAN_DIR:+${OCTO_PLAN_DIR}/session-intent.md}"
if [[ -f "$INTENT_FILE" ]]; then
  echo "Intent contract found: $INTENT_FILE"
  cat "$INTENT_FILE"
else
  echo "WARNING: No current resolved intent contract found for this workspace"
  echo "Skipping Stage 1 — proceeding to Stage 2 (code quality) only."
fi

If no intent contract exists: Warn the user and skip to Stage 2. Do NOT fabricate success criteria — the contract must exist from a prior workflow.

Step 2: Validate Success Criteria

For each success criterion in the intent contract:

1. Read the criterion from the ## Success Criteria section
2. Find evidence in the codebase that the criterion is met
3. Mark status:
   • [PASS] — Evidence confirms criterion is met
   • [FAIL] — Evidence shows criterion is NOT met
   • [PARTIAL] — Partially met, gaps identified

Present results:

## Stage 1: Spec Compliance

### Success Criteria Check

#### Good Enough Criteria
- [PASS] Criterion 1: <how it was met>
- [FAIL] Criterion 2: <why not met, what's missing>

#### Exceptional Criteria
- [PARTIAL] Criterion 1: <what's done, what's remaining>

Step 3: Validate Boundaries

For each boundary in the intent contract:

1. Read the boundary from the ## Boundaries section
2. Check for violations in the implementation
3. Mark status:
   • [RESPECTED] — No violations found
   • [VIOLATED] — Implementation crosses the boundary

### Boundary Check
- [RESPECTED] Boundary 1: <confirmation>
- [VIOLATED] Boundary 2: <what violated it>

Step 4: Stage 1 Gate

If user chooses to fix: Stop review, list specific fixes needed. If user chooses to proceed: Note the overrides and continue to Stage 2.

Stage 2: Code Quality

Runs stub detection and full code quality review.

Step 1: Stub Detection

Run 5 checks on all changed files:

# Get changed files
if git diff --cached --name-only 2>/dev/null | head -1 > /dev/null; then
  changed_files=$(git diff --cached --name-only)
elif git diff --name-only HEAD~1..HEAD 2>/dev/null | head -1 > /dev/null; then
  changed_files=$(git diff --name-only HEAD~1..HEAD)
else
  changed_files=$(git diff --name-only)
fi

# Filter source files
source_files=$(echo "$changed_files" | grep -E "\.(ts|tsx|js|jsx|py|go|rs|sh)$" || true)

STUB_ISSUES=0

for file in $source_files; do
  [[ -f "$file" ]] || continue

  # Check 1: TODO/FIXME/PLACEHOLDER markers
  todo_count=$(grep -cE "(TODO|FIXME|PLACEHOLDER|XXX)" "$file" 2>/dev/null || echo "0")
  if [[ "$todo_count" -gt 0 ]]; then
    echo "WARNING: $file has $todo_count TODO/FIXME markers"
    STUB_ISSUES=$((STUB_ISSUES + 1))
  fi

  # Check 2: Empty function bodies
  empty_fn=$(grep -cE "function.*\{\s*\}|=>\s*\{\s*\}" "$file" 2>/dev/null || echo "0")
  if [[ "$empty_fn" -gt 0 ]]; then
    echo "ERROR: $file has $empty_fn empty functions"
    STUB_ISSUES=$((STUB_ISSUES + 1))
  fi

  # Check 3: Suspicious null/undefined returns
  null_ret=$(grep -cE "return (null|undefined);" "$file" 2>/dev/null || echo "0")
  if [[ "$null_ret" -gt 0 ]]; then
    echo "WARNING: $file has $null_ret null/undefined returns — verify intentional"
    STUB_ISSUES=$((STUB_ISSUES + 1))
  fi

  # Check 4: Substantive line count
  subst_lines=$(grep -cvE "^\s*(//|/\*|\*|#|import|export|$)" "$file" 2>/dev/null || echo "0")
  if [[ "$subst_lines" -lt 5 ]]; then
    echo "WARNING: $file has only $subst_lines substantive lines"
    STUB_ISSUES=$((STUB_ISSUES + 1))
  fi

  # Check 5: Mock/test data in production code
  mock_count=$(grep -cE "const.*(mock|test|dummy|fake).*=" "$file" 2>/dev/null || echo "0")
  if [[ "$mock_count" -gt 0 ]]; then
    echo "WARNING: $file has $mock_count mock/test data references"
    STUB_ISSUES=$((STUB_ISSUES + 1))
  fi
done

echo "Stub detection complete: $STUB_ISSUES issues found"

Step 2: Multi-LLM Quality Review (RECOMMENDED)

After stub detection, dispatch code to multiple providers for parallel quality review. A Claude-only review pipeline misses what external models catch — Codex excels at logic errors and correctness, while Antigravity provides an independent security and edge-case analysis. Using both produces higher-confidence findings.

Check provider availability and dispatch in parallel:

# Get the diff for review
DIFF_CONTENT=$(git diff --cached 2>/dev/null || git diff HEAD~1..HEAD 2>/dev/null || git diff)

If external providers are available — dispatch focused reviews through Octopus routing:

providers=()
command -v codex >/dev/null 2>&1 && providers+=(codex)
command -v agy >/dev/null 2>&1 && providers+=(agy)

for provider in "${providers[@]}"; do
  safe_provider=$(printf '%s' "$provider" | tr -c '[:alnum:]_-' '_')
  "${HOME}/.claude-octopus/plugin/scripts/orchestrate.sh" spawn "$provider" \
    "Review this code diff for LOGIC, CORRECTNESS, and SECURITY issues. Focus on:
1. Logic bugs and off-by-one errors
2. Unhandled error paths
3. Race conditions or concurrency issues
4. Incorrect type handling or implicit coercions
5. Security issues at trust boundaries

Report ONLY high-confidence issues. Do NOT flag style preferences.

DIFF:
${DIFF_CONTENT}" > "/tmp/octopus-review-${safe_provider}.md" 2>/dev/null &
done

Wait for external reviews to complete, then synthesize all findings from Claude plus available external providers into a unified quality assessment. If external providers are unavailable, fall back to the Claude-only review below.

Claude (you) performs the full quality review regardless:

1. Architecture alignment — Does the code follow project patterns?
2. Error handling — Are errors caught and handled appropriately?
3. Security — Any OWASP Top 10 issues?
4. Performance — Any obvious bottlenecks or N+1 queries?
5. Readability — Clear naming, reasonable complexity?
6. Test coverage — Are new behaviors tested?

Synthesize external findings: If external providers returned results, merge their findings with yours. Where providers disagree on severity, note the divergence. Where multiple providers flag the same issue, mark it as high-confidence.

Step 3: Present Stage 2 Results

## Stage 2: Code Quality

### Stub Detection
- Files scanned: N
- Issues found: N
- [Details of each issue]

### Quality Review
- Architecture: [PASS/WARN/FAIL]
- Error Handling: [PASS/WARN/FAIL]
- Security: [PASS/WARN/FAIL]
- Performance: [PASS/WARN/FAIL]
- Readability: [PASS/WARN/FAIL]
- Test Coverage: [PASS/WARN/FAIL]

### Blocking Issues
[List any issues that must be fixed before merge]

### Recommendations
[Non-blocking suggestions for improvement]

Combined Report

After both stages complete, present the unified report:

## Staged Review — Complete

### Stage 1: Spec Compliance
- Success Criteria: N/N passed
- Boundaries: N/N respected
- Verdict: [PASS/FAIL]

### Stage 2: Code Quality
- Stub Detection: N issues
- Quality Score: [HIGH/MEDIUM/LOW]
- Blocking Issues: N
- Verdict: [PASS/FAIL]

### Overall Verdict: [PASS/FAIL]

[If FAIL: list specific items that must be addressed]
[If PASS: ready for merge/ship]

When to Use Each Review Type

Error Handling

The Bottom Line

Staged Review = Spec Compliance (Stage 1) + Code Quality (Stage 2)
Stage 1 gates Stage 2. Both must pass for overall PASS.

Build the right thing, then build it right.

Post Results to PR (v8.44.0)

After the combined report is generated, check for an open PR and post findings.

# Detect open PR on current branch
CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "")
PR_NUM=""

if [[ -n "$CURRENT_BRANCH" && "$CURRENT_BRANCH" != "main" && "$CURRENT_BRANCH" != "master" ]]; then
    if command -v gh &>/dev/null; then
        PR_NUM=$(gh pr list --head "$CURRENT_BRANCH" --json number --jq '.[0].number' 2>/dev/null || echo "")
    fi
fi

if [[ -n "$PR_NUM" ]]; then
    # Post combined report through the outbound credential gate.
    REPO_SLUG=$(gh repo view --json nameWithOwner --jq .nameWithOwner)
    COMMENT_BODY="## Staged Review — Claude Octopus

${COMBINED_REPORT}

---
*Staged review by Claude Octopus (/octo:staged-review)*"
    if ! "${CLAUDE_PLUGIN_ROOT:-${HOME}/.claude-octopus/plugin}/scripts/safe-gh-comment.sh" \
            --repo "$REPO_SLUG" pr-comment "$PR_NUM" - <<< "$COMMENT_BODY"; then
        echo "GitHub write state is unknown; check for the staged review comment before retrying:" >&2
        gh pr view "$PR_NUM" --repo "$REPO_SLUG" --comments || true
        return 1 2>/dev/null || exit 1
    fi

    echo "Staged review posted to PR #${PR_NUM}"
fi

Behavior:

• Auto-posts when running inside /octo:deliver or /octo:embrace workflows
• Asks user first when invoked standalone via /octo:staged-review