Review Execute

Review-Execute: Adversarial Post-Execution Verification

Philosophy: Execution is where intent becomes code. The general /review skill checks code quality — bugs, patterns, security. This skill checks something different: did the code actually implement what the beads specified? Every bead carries acceptance criteria, failure criteria, FR references, and design decisions. This skill verifies each claim in the execution manifest against the actual codebase. A bead marked "Completed" means nothing if the acceptance criteria aren't satisfied in the code.

Duration targets: BRIEF ~15-20 minutes (≤6 beads, single module), STANDARD ~30-60 minutes (typical feature, 7-20 beads), COMPREHENSIVE ~60-120 minutes (multi-module, 20+ beads, full upstream traceability). Most time is Phase 3 (bead-by-bead verification) — this is where real issues are found.

Why This Matters

The execution agent self-reviews each bead before committing (lightweight check). But self-review has blind spots: the implementing agent believes its own code is correct. An independent review with fresh context catches:

• Acceptance criteria claimed as met but not actually satisfied in code
• Failure criteria violations the implementing agent didn't notice
• Design decisions silently reversed during implementation
• FR coverage gaps where the bead was "completed" but the FR's Given/When/Then aren't testable
• Pattern deviations where the agent chose a different approach than the referenced pattern
• Scope creep where implementation went beyond the bead's objective

Real-world execution reviews have caught: endpoints with wrong HTTP verbs vs design, missing entity properties that the data model specified, tests that verify framework behavior instead of application logic, and beads marked complete where the success criteria were only partially met.

---

Trigger Conditions

Run this skill when:

• Execution is complete (/execute finished, manifest written)
• User says "review execution", "verify beads", "check the implementation"
• Before creating a PR for executed work
• After significant post-execution fixes

Do NOT use for:

• General code quality review (use /review — bugs, security, patterns)
• Reviewing beads before execution (use /review-beads)
• Reviewing designs or plans (use /review-design or /review-plan)

Stage Gates — AskUserQuestion

At every PAUSE point in this skill, call the AskUserQuestion tool to present structured options to the user. Do not present options as plain markdown text — use the tool. The YAML blocks at each PAUSE point show the exact parameters to pass.

For pattern details and examples: ../_shared/references/stage-gates.md

> Fallback: Only if AskUserQuestion is not available as a tool (check your tool list), fall back to presenting options as markdown text and waiting for freeform response.

Shared References

Before starting any review, load these shared reference files:

• CONVERGE loop & classification: ../_shared/references/converge-mode.md
• Severity model & finding quality: ../_shared/references/review-finding-taxonomy.md

These define the CONVERGE mode behavior, MECHANICAL vs DECISION classification, authority hierarchy, PRE_EXISTING severity rules, and finding quality standards shared across all review skills. Skill-specific CONVERGE behavior (wave definitions, authority hierarchy overrides) is documented inline below.

---

Mode Selection

| Mode | When | Scope | |------|------|-------| | BRIEF | ≤6 beads, single module, low-risk | Manifest check + spot-check 3 beads | | STANDARD | Typical feature, 7-20 beads | Full bead-by-bead verification + design traceability | | COMPREHENSIVE | Multi-module, 20+ beads, or critical path | Full verification + FR acceptance criteria depth + upstream cross-reference | | CONVERGE | Default. Fix all issues until 0 FAILs | Selected depth + auto-fix loop |

CONVERGE is the default mode. Unless the user explicitly says "no converge" or "review only", always run with CONVERGE enabled. The whole point of review-execute is to leave the code in a passing state — finding issues without fixing them is half the job.

Verification-mode recommendation: For verification-mode executions (>70% pre-existing, gap-closure/modification beads), recommend STANDARD unless the user explicitly requests COMPREHENSIVE. The extra UC/FR depth in COMPREHENSIVE rarely finds issues in modification-only bead sets — it loads 8+ design docs but typically produces the same verdict as STANDARD in twice the context.

Verification-mode Phase 3 scoping: For verification-mode reviews, Phase 3 (Design Traceability) is abbreviated but NOT skippable. Use this concrete checklist:

• API surface (mandatory): For every endpoint touched by a bead, open the api-surface.md and compare response codes, verbs, and request/response shapes line-by-line. Do this systematically, not by following threads. This is how the Sessions review caught the missing token rotation — line-by-line comparison, not ad-hoc tracing.
• Data model: Verify properties for new/modified entities only
• UC tracing: Skip unless a bead's scope explicitly mentions UC scenario steps
• FR AC depth: Skip unless greenfield beads exist

Report format by outcome:

• Round 1 = 0 FAILs: Abbreviated 1-page report — verdict, WARN table, one-line per-bead status
• CONVERGE fixed all FAILs: Hybrid report — detailed findings table showing what was found AND fixed, plus final PASS verdict. Include the fix commits so the user can see what changed.
• FAILs remain after CONVERGE: Full report with unresolved findings for user action

WARN actionability: Every PRE_EXISTING WARN must include exact file:line references for BOTH the code AND the upstream doc that need alignment. Example: "W1: UserDTO.cs:45 uses LinkMethod but api-surface.md:112 specifies LinkedMethod." This makes the br issue actionable — the person fixing it doesn't need to re-discover the mismatch.

Frontend test execution: When running frontend tests for verification, prefer a machine-readable reporter (e.g., --reporter=json or equivalent for your test runner) over parsing human-readable output. Human-readable test output often includes serialized error objects that are extremely verbose and hard to parse for pass/fail counts.

CONVERGE Mode

For the shared CONVERGE pattern (the loop, classification, authority hierarchy, progressive loading, convergence criteria, same-session detection): ../_shared/references/converge-mode.md

CONVERGE is the default mode. Unless the user explicitly says "no converge" or "review only", always run with CONVERGE enabled.

When to skip CONVERGE: If a prior STANDARD review already found 0 FAILs, CONVERGE adds no value — the fix loop never activates. In this case, recommend running COMPREHENSIVE (for deeper traceability) without CONVERGE (the fix loop overhead). CONVERGE is most valuable when the prior review found FAILs that need automated fixing, or when no prior review exists.

Progressive loading waves (review-execute specific):

• Wave 1: Execution manifest + bead descriptions + changed files (catches most issues)
• Wave 2: Design docs (api-surface, data-model) for beads with findings
• Wave 3: PRD acceptance criteria + ADRs for deep traceability checks

MECHANICAL examples for review-execute: wrong HTTP verb, missing entity property, test verifying wrong thing, missing import. Even if a fix touches shared types (like adding a new result type), it's MECHANICAL if the design clearly specifies the expected behavior.

Pattern pre-check: Before writing any fix, verify the fix approach against the project's architectural constraints (ADRs, pattern docs, CLAUDE.md). A fix that violates the project's patterns (e.g., injecting DbContext into an endpoint when the project forbids it) creates a new finding. Check constraints BEFORE writing code.

Cascade check: After fixing a file, run the project's build and test commands. If tests fail, diagnose first — determine whether the failure is from your fix (revert) or from a pre-existing bug your fix exposed (fix the bug). Do NOT automatically revert and reclassify. Only revert if your fix genuinely caused the regression.

Update manifest after fixes: Update docs/execution/{feature}/manifest.md with new commit hashes, changed file lists, and any corrected FR/UC coverage claims. A stale manifest after CONVERGE undermines future reviews.

Same-session fresh-eyes mitigation (review-execute specific): Same-session reviews share the executing agent's context and biases. To compensate:

1. Re-read design docs from scratch — do NOT rely on conversation context. Re-read the api-surface.md and data-model.md line-by-line against the implementation.
2. Load one doc the executing agent didn't read — spot-check 2 claims against it (e.g., if the agent didn't read the PRD, verify 2 FR ACs against code).

Non-greenfield execution review: If the execution manifest or plan's Implementation Status shows >70% of design elements already existed before execution:

• Verification beads ("verify X matches design") — the AC is "confirm existing code matches spec." Review by reading the code and checking against the spec, not by looking for newly written code.
• Modification beads ("update X to add Y") — verify the specific modification was made. Don't flag pre-existing code that wasn't part of the bead's scope.
• Gap-filling beads ("implement missing X") — treat as greenfield for the specific gap.
• Do NOT flag missing tests for pre-existing code that the bead didn't modify — only verify test coverage for new/changed code within the bead's scope.
• Wave 1 only for loading: design api-surfaces + changed files. Only load PRD/UCs if Wave 1 reveals coverage gaps. This cuts document loading by ~60% for modification-only bead sets.

---

Finding Classification

For the shared severity model (FAIL/WARN definitions), MECHANICAL vs DECISION heuristic, pre-existing drift handling, and finding quality standards, see ../_shared/references/review-finding-taxonomy.md.

Every finding has a class (what's wrong) and a severity (FAIL or WARN):

| Class | Meaning | Default Severity | |-------|---------|-----------------| | AC_NOT_MET | Acceptance criterion claimed but not satisfied in code | FAIL | | FC_VIOLATED | Failure criterion is violated by implementation | FAIL | | DESIGN_DRIFT | Implementation contradicts design doc (api-surface, data-model) | FAIL | | PATTERN_DEVIATION | Implementation doesn't follow referenced pattern doc | FAIL | | SCOPE_CREEP | Implementation includes work beyond bead objective | WARN | | TEST_GAP | Success criterion has no corresponding test | FAIL | | TEST_QUALITY | Test exists but doesn't verify the right thing | WARN | | MISSING_IMPL | Bead marked complete but core functionality absent | FAIL | | FR_GAP | FR acceptance criteria not satisfied despite bead claiming coverage | FAIL | | UC_GAP | Use case scenario step not traceable through implemented code | FAIL | | ARCH_VIOLATION | Implementation violates architecture constraint (multi-tenancy, auth, CQRS) | FAIL | | CROSS_MODULE_GAP | Cross-module dependency not wired or shared contract not imported | FAIL | | MANIFEST_STALE | Manifest claims don't match actual git state | WARN | | ADR_VIOLATION | Implementation violates an architectural decision record | FAIL | | UPSTREAM_DOC | Issue is in the upstream doc, not the implementation | WARN (note separately; create br issue) |

Severity model and pre-existing drift rules: See ../_shared/references/review-finding-taxonomy.md for FAIL/WARN definitions, calibration examples, and the PRE_EXISTING tagging rules for non-greenfield reviews.

---

Critical Sequence

Phase 0: Load Execution Context

Step 0.1 — Load Execution Manifest:

Read docs/execution/{feature}/manifest.md.

If manifest exists: Parse:

• List of completed beads with IDs
• Files changed per bead
• FRs addressed per bead
• ACs claimed per bead
• Design elements implemented per bead
• Commit hashes per bead

If manifest is missing or is a stub (fallback): Reconstruct it as Step 0 rather than flagging and deferring:

1. Note MANIFEST_STALE as a WARN finding: "/execute did not write the required manifest"
2. Reconstruct and WRITE the manifest to docs/execution/{feature}/manifest.md:

• Identify the epic: br search "{feature}" or br list --status closed
• Get the bead list: br dep tree {epic-id}
• Map commits to beads: git log --oneline — match commit messages to bead titles
• For each bead, derive files changed: git diff {commit}^..{commit} --stat

1. Commit the reconstructed manifest so future reviews have it

Creating the manifest upfront (even reconstructed) is more efficient than working without it throughout the review. For mature modules where execution happened across many sessions, this reconstruction is the norm, not the exception.

Step 0.2 — Load Bead Descriptions:

Read bead descriptions from one of these sources (in preference order):

1. br show bd-{id} --json for each bead ID in the manifest — preferred (the .description field is the execution contract read by hive's BeadsAdapter)
2. docs/plans/{feature}/beads.md — human-readable authoritative record. Read in chunks if it exceeds read limits (common for 15+ beads). Read by track: backend beads first, then frontend, then gates.
3. Manifest bead summaries — last resort if br is unavailable; less detailed

Parse:

• Objective
• Success Criteria (each criterion individually)
• Failure Criteria (each criterion individually)
• Context to Load
• Pattern references
• FR references
• Design references

Step 0.3 — Identify Changed Files:

git log --oneline {first-commit}..{last-commit}  # commits from execution
git diff {pre-execution-commit}..HEAD --stat       # all files changed

Cross-reference against manifest file lists. Flag discrepancies.

Step 0.4 — Check for Prior Reviews:

Check docs/reviews/ for existing review-execute-{feature}-*.md files. If a prior review exists:

1. Read its verdict and finding count
2. Ask the user: "A prior {STANDARD/COMPREHENSIVE} review exists ({date}, {verdict}). Run a fresh review, or upgrade depth?"
3. If upgrading from STANDARD to COMPREHENSIVE, focus on the additional depth (FR AC, UC tracing, architecture) rather than re-verifying beads that already passed

This prevents wasted work re-reviewing modules and gives COMPREHENSIVE reviews a targeted scope.

Step 0.5 — Determine Mode:

If user hasn't specified:

• ≤6 beads, single module → BRIEF
• 7-20 beads, typical feature → STANDARD
• 20+ beads or multi-module → COMPREHENSIVE

---

Phase 1: Manifest Integrity Check

Step 1.1 — Verify Manifest Completeness:

| Check | Method | Severity | |-------|--------|----------| | All beads in issue tracker are in manifest | br list --status closed vs manifest bead list | FAIL if missing | | All manifest commits exist in git | git log --oneline vs manifest commit hashes | FAIL if missing | | File lists match actual git changes | git diff --stat per commit vs manifest file lists | WARN if mismatch | | FR coverage table is complete | Every FR from bead descriptions appears in table | WARN if missing |

Step 1.2 — Verify Build & Tests Pass (MANDATORY — do NOT skip):

This is the single most important mechanical check in the review. Do NOT trust the manifest's "all tests passing" claim — the manifest is the executing agent's assertion, not verified truth. A review that doesn't run tests can issue a false PASS.

Run the project's build and test commands. If they fail, triage quickly: stash your changes, re-run failing tests, pop the stash — this classifies failures as pre-existing vs introdu

…

Source & license

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

• Author: dirkkok101
• Source: dirkkok101/skills
• License: MIT

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