LIFEAI/rdc:review

⚠️ OUTPUT CONTRACT (READ FIRST): guides/output-contract.md Checklist-only output. No tool-call narration. No raw MCP/JSON/log dumps. One checklist upfront, updated in place, shown again at end with a 1-line verdict.

If dispatching subagents or running as a subagent: read {PROJECT_ROOT}/.rdc/guides/agent-bootstrap.md first (fallback: {PROJECT_ROOT}/.rdc/guides/agent-bootstrap.md), then {PROJECT_ROOT}/.rdc/guides/engineering-behavior.md (fallback: {PROJECT_ROOT}/.rdc/guides/engineering-behavior.md).

Sandbox contract: This skill honors RDC_TEST=1 per guides/agent-bootstrap.md § RDC_TEST Sandbox Contract. Destructive external calls short-circuit under the flag.

rdc:review — Quality Gate

When to Use

• After a build session (especially overnight builds)
• Before merging development → main/production
• Project lead asks "review the work", "is everything clean"
• Before any production deployment
• Called by rdc:overnight after each epic build completes

Arguments

• rdc:review — interactive review, pauses on issues needing judgment
• rdc:review --unattended — silent mode, auto-fixes everything fixable

Procedure

1. Identify modified packages:

   git diff --name-only origin/main...HEAD | grep "^packages/" | cut -d/ -f2 | sort -u
   

2. Run tests for each modified package:

   cd packages/<name> && npx vitest run 2>&1 | tail -10
   

   Report: package → test count → pass/fail → new tests added

   IMPORTANT: pnpm build must NEVER be run (crashes system). Use npx tsc --noEmit --project <path>/tsconfig.json for typecheck instead. For packages without tests, typecheck is the verification method. Do NOT run vitest across the entire monorepo — check only modified packages individually.

3. Check test coverage delta:

   git diff origin/main...HEAD -- packages/*/src/ | grep -c "^+" | head -5
   git diff origin/main...HEAD -- packages/*/test* packages/*/src/**/*.test.* packages/*/src/**/*.spec.* 2>/dev/null | grep -c "^+" || echo 0
   

   Flag any package where implementation lines added > 50 but test lines added = 0.

4. Check for export conflicts:

   • Read packages/*/src/index.ts for any package with new exports
   • Look for duplicate export names across the barrel
   • Verify aliased exports don't shadow each other

5. Check for TODO/FIXME/HACK:

   grep -rn "TODO\|FIXME\|HACK\|XXX" packages/*/src/ --include="*.ts" --include="*.tsx"
   

6. Check package versions:

   • Any package with significant new code should have a version bump
   • Compare package.json versions to what's in docs/SYSTEM-STATE.md

7. Check for stale CLAUDE.md:

   • If new modules were added to a package, does its CLAUDE.md mention them?
   • Flag any package where exports grew by >10 lines but CLAUDE.md wasn't updated

8. Orphan work item audit:

   SELECT id, title, item_type, status, source, created_at::date
   FROM work_items
   WHERE parent_id IS NULL
     AND item_type NOT IN ('epic', 'bug')
     AND status NOT IN ('done', 'archived')
   ORDER BY created_at DESC;
   

   For each orphaned task found:

   • If it clearly belongs to an open epic → do not run raw SQL. No RPC exists for re-parenting. Create a work item: SELECT insert_work_item(p_title := 'Re-parent orphaned task <task-id> to epic <epic-id>', p_priority := 'normal', p_source := 'agent') and flag for supervisor to re-parent manually.
   • If unclear → report (interactive) or flag in REVIEW_STATUS (unattended)
   • Never silently leave orphaned tasks
   • ⛔ Raw UPDATE work_items SET parent_id = ... is forbidden — bypasses RLS and all constraint checks

8b. Mandatory code-review gate — dispatch pr-review-toolkit:code-reviewer:

⛔ No CLEAN verdict without a code-review pass. This is the second half of the quality gate; tsc/vitest catch type and behavior errors, code-reviewer catches logic, security, and convention drift.

Dispatch ONE pr-review-toolkit:code-reviewer agent on the full diff under review:

Agent({
  subagent_type: "pr-review-toolkit:code-reviewer",
  description: "rdc:review code-review pass",
  prompt: "Review `git diff origin/main...HEAD` on the development branch.
           Focus on: bugs, logic errors, security vulnerabilities, project-convention
           adherence (.claude/rules/*, CLAUDE.md, engineering-behavior.md).
           Confidence-based filtering — high-confidence findings only.
           Return CODE_REVIEW_COMPLETE with: { critical_count, high_count, medium_count,
           low_count, findings: [{severity, file:line, issue, suggested_fix}] }."
})

Severity gate:

• critical or high → verdict cannot be CLEAN. Interactive: report and pause. Unattended: auto-fix if mechanical (rename, missing await, obvious null check), otherwise escalate via advisor and mark HAS_ISSUES.
• medium or low → record in the review report's "Issues" section; verdict can still be CLEAN.
• Zero findings → log CODE_REVIEW: CLEAN and continue.

Under RDC_TEST=1: echo [RDC_TEST] skipping code-review dispatch and continue.

9. Verification gate — dispatch the verify agent: After any fixes land, run the verify gate on every touched package. See guides/agents/verify.md. Apply guides/engineering-behavior.md while reviewing: flag unnecessary abstraction, drive-by refactors, missing assumptions, hidden uncertainty, out-of-scope edits, and prose-only verification. Iron Law: no CLEAN verdict without fresh evidence. Quote the checklist decomposition verdict, vitest output, and tsc output in the report. The verify agent must reject any work item that lacks passed decomp-* checklist rows or whose rows are too coarse to prove one observable behavior at a time. If verify fails → do NOT emit CLEAN. Loop back, fix, re-run verify.

10. Fix issues found:

• Failing tests → fix and commit
• Export conflicts → resolve and commit
• Missing version bumps → bump and commit
• All fixes as separate commits with descriptive messages

Judgment calls:

• Interactive: report — don't guess
• Unattended: escalate via advisor tool with: error message, surrounding context, two most likely fix paths. Resume with advisor's recommendation. If advisor unavailable: take the most conservative path, flag in status block.

11. Report:
    • Interactive:

      ## Review Results
      | Package | Tests | Pass/Fail | New Tests | Issues |
      ## Fixed
      ## Remaining Issues
      ## Verdict: CLEAN / HAS ISSUES
      

    • Unattended: emit status block only:

      REVIEW_STATUS: { verdict: "CLEAN|HAS_ISSUES", packages_checked, tests_passed, tests_failed, new_tests_added, fixes_applied, escalations }
      

Rules

• Do NOT run pnpm build (crashes system) — vitest only
• Interactive: fix what you can, flag what needs decision
• Unattended: fix everything fixable; escalate judgment calls to advisor
• Each fix is a separate commit (not batched)
• Always push fixes to origin after committing (skip if $RDC_TEST=1 — echo [RDC_TEST] skipping git push instead)
• Unattended: NEVER pause for input

Capture lessons (exit step)

Before the final verdict line, follow .rdc/guides/lessons-learned-spec.md § Capture procedure. If this run taught something non-obvious — a first root-cause theory that turned out wrong, the documented/standard path not working, a missing gate or check that cost a round, or a surprising tool/infra behavior — write one .rdc/lessons/<YYYY-MM-DD>-review-<short-slug>.md per lesson using the schema in that spec. Set scope (simple | architectural) and status (open, or applied if you shipped the fix in this same run, with the commit linked). Commit the lesson file(s) on develop alongside the run's other commits, and note "N lessons captured" in your verdict/summary. A run that taught nothing writes nothing — absence is the default.