Review Phase Current State Research

Context and lane coverage

Three readonly lanes covered the active workflow and child skills, CLI packaging and scaffold evidence, and the private wiki explanation. This synthesis rechecked the retained claims against repository files. It records current facts, conflicts, inferences, and decisions still requiring product authority; it does not choose the rework.

How review-phase works

• The active wrapper is intentionally thin: SKILL.md requires REFERENCE.md, names the target and scope, delegates fixed-point framing to $review, structured candidates to $autoreview, mandatory lenses to $simplify, $improve-codebase-architecture, and scoped skills, then runs safe readonly validation and reports findings (.agents/skills/review-phase/SKILL.md:9-53).
• Scope defaults to the smallest certain target, every inner review step receives that bound, full-codebase review is exceptional, and PR review uses the actual PR base rather than assumed trunk (.agents/skills/review-phase/REFERENCE.md:16-34).
• $review is a diff-only two-axis child: it pins git diff <fixed-point>...HEAD, runs Standards and Spec in parallel lanes, and preserves separate aggregation without cross-axis reranking (.agents/skills/review/SKILL.md:6-23, .agents/skills/review/SKILL.md:58-80).
• $autoreview is advisory candidate generation. The parent verifies findings against real code and adjacent evidence, reruns after accepted fixes, and stops on a successful result with no accepted or actionable findings (.agents/skills/autoreview/SKILL.md:18-43). Its target modes distinguish dirty local work, branch/PR work with the actual PR base, and committed changes (.agents/skills/autoreview/SKILL.md:45-93).
• Standalone mode stops after readonly findings. Delivery-owned mode returns blockers to delivery, which classifies no blockers, implementation blockers, runtime evidence, and broad debt into distinct next routes (.agents/skills/review-phase/REFERENCE.md:3-14, .agents/skills/delivery-phase/phases/review.md:8-31).

Artifact/runtime structure

The wrapper's output contract names finding severity, references, impact, evidence, action, validation, exact command, fixed point, Spec and Standards sources, scoped guidance, residual risk, and mode (.agents/skills/review-phase/SKILL.md:48-53, .agents/skills/review-phase/REFERENCE.md:126-147). Delivery records the review process, findings or no-op, and route decision (.agents/skills/delivery-phase/phases/review.md:22-31).

Inference: neither contract requires a durable report path, filename, commit, or retained reference. Review output may therefore remain conversational or embedded in delivery state; that differs from workflows whose artifact contract names a durable location.

The CLI catalogs review-phase, review, and autoreview separately (apps/cli/src/data/catalog/skills.ts:671-693), and the default research pack installs all three with architecture inspection (apps/cli/src/data/catalog/packs.ts:303-320). Review-oriented subagents receive readonly, severity-first activation instructions, while spec-reviewer additionally receives review-phase and plan-reviewer receives planning review skills (apps/cli/src/data/subagents/manifest.mjs:114-137, apps/cli/src/data/subagents/manifest.mjs:199-236).

Trusted facts

• Safe validation can include focused tests, typecheck, lint, build, docs links, or smoke commands; missing checks must retain a reason and residual risk (.agents/skills/review-phase/REFERENCE.md:63-71).
• Behavior-changing plans or specs without valid RED/GREEN evidence are blocking; forgotten RED is not a valid reason_not_testable (.agents/skills/review-phase/REFERENCE.md:73-76).
• Review must apply the nearest relevant AGENTS.md, ownership rules, runbooks, and named scoped skills, and it must not mutate stack state (.agents/skills/review-phase/REFERENCE.md:47-61, .agents/skills/review-phase/REFERENCE.md:117-124).
• Delivery cannot close an implementation before the review gate and must route stale or missing review back through that gate (.agents/skills/delivery-phase/phases/router.md:13-26).

Conflicts and gaps

Runtime contract conflicts

• The wrapper accepts files, specs, plans, docs, and runtime evidence as direct targets, but mandatory $review requires a resolvable, non-empty fixed-point diff. No adapter or skip rule explains artifact-only review (.agents/skills/review-phase/SKILL.md:19-28, .agents/skills/review/SKILL.md:17-23). $autoreview likewise documents local, branch, and commit targets rather than arbitrary documents (.agents/skills/autoreview/SKILL.md:45-93).
• The wrapper orders final findings globally by severity, while $review forbids merging or reranking Standards and Spec findings. The current files do not define whether severity ordering is nested inside each axis or supersedes axis separation (.agents/skills/review-phase/REFERENCE.md:126-144, .agents/skills/review/SKILL.md:76-80).
• The wrapper sends bounded scope to both $autoreview and a separately named “ClawPatch-backed review,” but neither SKILL.md nor REFERENCE.md defines when that second runtime runs or how it combines with $autoreview (.agents/skills/review-phase/SKILL.md:21-28, .agents/skills/review-phase/REFERENCE.md:30-45).

Invocation and byte-authority conflict

• Repository guidance makes the external shared-skills repository the source of truth and forbids treating .agents/skills/* or apps/cli/skills/* as editable sources (AGENTS.md:11-15). The blind pre-command snapshot has a model-facing trigger description and no disable-model-invocation flag (.devpunks/AGENT-HANDOFF.md:30-35, .devpunks/pre-existing-skills/.agents/skills/review-phase/SKILL.md:1-8), while both active and vendored copies set disable-model-invocation: true (.agents/skills/review-phase/SKILL.md:1-7, apps/cli/skills/phases/review-phase/SKILL.md:1-7).
• The live managed manifest records SKILL.md hash d7c83a..., while the public managed-assets fixture expects 435335...; their REFERENCE.md hashes agree (.devpunks/scaffold-manifest.json:2938-2947, apps/cli/test-fixtures/public-output/managed-assets.json:1000-1009). Durable docs also say review-phase remains model-invocable as a delivery delegate (docs/README.md:94-98, docs/runbooks/hi-cli-scaffolding.md:45-47). This is canonical-intent/live-byte/managed-fixture divergence, not a resolved desired state.

Wiki drift

• The entrypoint correctly teaches candidate verification, mandatory lenses, readonly optional research, and delivery/debug fix routing (apps/wiki/content/docs/harness/entrypoints/review.mdx:18-27, apps/wiki/content/docs/harness/entrypoints/review.mdx:50-89). It omits the smallest-target/full-codebase rule, actual PR base, RED/GREEN blocker, stack boundary, and requirements-grill artifact checks that the active reference requires (.agents/skills/review-phase/REFERENCE.md:16-34, .agents/skills/review-phase/REFERENCE.md:63-124).
• “OpenClaw-backed” is stale or ambiguous engine language: the wiki names OpenClaw as the structured reviewer, while $autoreview says Codex is the default engine and uses “OpenClaw” for one repo-local helper path (apps/wiki/content/docs/harness/entrypoints/review.mdx:20-23, apps/wiki/content/docs/harness/entrypoints/review.mdx:59-66, .agents/skills/autoreview/SKILL.md:6-10, .agents/skills/autoreview/SKILL.md:139-177).
• The review page is a flow carrying spec-only status, and it lacks base links and created plus routed-flow ingested fields required by local schema (apps/wiki/content/docs/harness/entrypoints/review.mdx:1-14, apps/wiki/AGENTS.md:42-75).

Test and packaging coverage

• Catalog and pack wiring include the wrapper and both child runtimes (apps/cli/src/data/catalog/skills.ts:671-693, apps/cli/src/data/catalog/packs.ts:303-320). Scaffold tests assert that the review-phase directory and SKILL.md are emitted (apps/cli/src/scaffold/run.test.ts:480-511, apps/cli/src/scaffold/run.test.ts:768-775, apps/cli/src/scaffold/stage.native.test.ts:856-884).
• Managed-asset validation inventories and hashes the emitted files (apps/cli/src/features/context-planning/managed-assets-fixture.native.test.ts:44-62, apps/cli/src/features/context-planning/managed-assets-fixture.native.test.ts:96-123, apps/cli/src/features/context-planning/managed-assets-fixture.native.test.ts:213-219). Content tests exercise one requirements-grill sentence in review-phase/REFERENCE.md, not the wrapper's invocation frontmatter or end-to-end orchestration (apps/cli/src/content/content.test.ts:75-123).

Inference: current tests are strong on presence, packaging bytes, and one reference invariant, but blind to source/live invocation-policy agreement, artifact-only target routing, axis/severity aggregation, durable review-artifact retention, and an integrated wrapper run across both child runtimes.

Open decisions for the rework

1. Invocation policy: decide whether delivery may invoke review-phase implicitly, then align upstream source, live/vendored bytes, managed fixtures, and docs. Current evidence conflicts; research does not choose the policy (.agents/skills/review-phase/SKILL.md:1-7, docs/runbooks/hi-cli-scaffolding.md:45-47).
2. Target protocol: decide whether artifact-only targets bypass diff-only $review, synthesize a fixed point, or gain an artifact review mode (.agents/skills/review-phase/SKILL.md:19-28, .agents/skills/review/SKILL.md:17-23).
3. Aggregation: decide the report shape that preserves Standards/Spec separation while still presenting severity-first findings (.agents/skills/review/SKILL.md:76-80, .agents/skills/review-phase/REFERENCE.md:126-144).
4. Runtime ownership: decide whether ClawPatch is a real second mandatory runtime, conditional evidence source, or stale wording beside $autoreview (.agents/skills/review-phase/REFERENCE.md:30-45).
5. Durability: decide whether standalone and delivery reviews require a retained artifact, and if so define its authority, path, freshness, and resume semantics (.agents/skills/delivery-phase/phases/router.md:6-10, .agents/skills/delivery-phase/phases/review.md:22-31).

Recommended next local action

Run a bounded requirements/design pass over the five decisions above, beginning with invocation policy because it determines source sync and scaffold fixture changes. Then write one accepted spec that assigns ownership across the upstream shared skill, Harness mirrors, package tests, managed fixtures, delivery routing, and wiki entrypoint; do not patch only the live .agents copy because repository guidance requires source-first synchronization (AGENTS.md:11-15).