implementation-final-review

name

implementation-final-review

description

Perform a risk-tiered zero-base final review loop before an implementation is declared complete. Use only when the user explicitly invokes $implementation-final-review or repository instructions authorize automatic invocation after implementation. Audit the complete merge-base diff for requirement fit, contract-surface coverage, await-boundary and lifecycle gaps, released compatibility, security, protocol, and persistence boundaries, unnecessary complexity, package and generated public surfaces, and adversarial test coverage; use compact self-contained reviewer packets and two concurrent no-history independent reviewers per round, defer broad final repository verification until review is clean and observable host capacity is available, preserve clean evidence for unchanged semantic components, close repeated root-cause groups instead of accumulating local patches, and enforce bounded review cycles in one task-global ledger.

Implementation Final Review

Treat implementation and final review as separate phases. Reconstruct the change from the original requirement and the complete diff; do not defend the current design merely because it is implemented or tested.

Non-negotiable guarantees

• Review the exact final task content, including committed, staged, unstaged, and task-owned untracked deliverables. The only exceptions are the narrowly verified final-gate type-erasure and base-advance closures in step 20, which preserve clean credit through explicit identity evidence and still require the complete final verification stack on the resulting fingerprint.
• Use the merge-base three-dot diff for patch ownership and the latest release tag separately for released compatibility.
• Require independent review. A same-context self-review cannot satisfy the clean-review gate.
• Freeze task-owned content while reviewers inspect a fingerprint.
• Treat an exact normalized file path in the task and component manifests as authoritative even when ignore rules match that file. An existing exact file takes literal precedence over Git pathspec metacharacters; use explicit :(glob) magic when pattern semantics are intended. A directory or glob pathspec never promotes ignored operational files into the review.
• Repeat commit-hook inspection, every safe rewriting step, second-pass idempotence, and generated-provenance validation before every fingerprint freeze, including post-fix and delta-review rounds. Record the exact executable inspection and rewriting commands plus their results in packet preflight evidence; a prose label is not an executable command.
• Start independent reviewers without inherited conversation history. Fresh judgment does not require repeatedly replaying the implementer's context.
• Report only concrete, patch-scoped findings supported by requirements, released behavior, a durable boundary, explicit maintainer intent, user reliance, or a baseline regression.
• Never weaken final repository verification. Component-aware review invalidation reduces repeated review, not required build or test gates.
• Keep one task-global round ledger across pauses, compaction, handoff, renaming, resumed work, and post-completion feedback. Enforce a bounded budget for each active review cycle without discarding earlier history.
• Trust the active implementation control plane to record actual reviewer dispatches, waits, outputs, and verification executions. The local protocol helper validates those records but does not replace platform-issued cryptographic execution attestation.

Post-completion feedback boundary

An implementation review cycle is complete only after its clean-review gate, mandatory verification, any requested local commit, and final user-facing handoff are complete. Seal that cycle at this boundary. A pause, compaction, context change, agent handoff before completion, or ordinary request to continue unfinished work does not create a new cycle or reset its budget.

A later user message containing concrete actionable review feedback starts a post-completion feedback cycle. The feedback message itself authorizes implementing that feedback and running the repository-mandated focused tests, delta review, verification, and local commit or amendment needed to return the task to a completed state. Do not ask for separate review-budget authorization merely because the sealed implementation cycle exhausted its budget.

Keep the same task identity and ledger, preserve its canonical root-cause history and clean credit for unchanged components, and append a default budget of two fingerprint rounds for the new feedback cycle. Ask the user again only when the feedback materially widens the requested contract, changes a released or durable compatibility boundary, requires authority beyond resolving the feedback, or exhausts the feedback-cycle budget.

Workflow

1. Finish the initial implementation and focused tests. Apply formatting before review when formatting can rewrite the diff. Inspect the actual final commit-hook configuration and run the exact safe, non-committing equivalent of every hook step that can rewrite task-owned content before freezing the first review fingerprint. Run each rewriting step until a second execution is content-idempotent. Normalize generated files before computing embedded hashes or provenance so the hook cannot invalidate them later. Record any hook step that cannot safely run before review; if that step later changes task content, apply the normal invalidation rules without exception.
2. Re-read the original user request and the current implementation scope contract. If no contract exists, record the required behavior, compatibility requirements, intentionally unsupported cases and failure behavior, and supported alternative or none.
3. Resolve the intended target and merge base. If a supplied target or base is not an ancestor of HEAD, compute their common merge base and treat merge-base...HEAD as the task-owned diff. Use the latest release tag separately when released compatibility is the relevant boundary. Include committed, staged, unstaged, and untracked changes that belong to the task.
4. Read the complete task-owned three-dot diff from the resolved merge base. Never treat target-only commits between the merge base and an advanced or divergent target as deletions or regressions introduced by the patch. Check integration with the current target separately when relevant; report an actual conflict or semantic incompatibility, not mere absence of target-side changes. Do not limit review to the latest fix or files named in prior feedback. Record a complexity delta: runtime lines changed, new state fields, new synchronization or ownership mechanisms, affected subsystems, and test permutations.
5. Run the baseline-reset gate before accepting the current design:
   • Describe the required behavior without referring to branch-local helper types or state.
   • Identify the nearest released/base pipeline that already owns the behavior.
   • Compare patching the current diff with replacing task-owned branch-local machinery by a narrow change from the base implementation.
   • Treat unreleased implementation and tests as disposable. Preserve unrelated or user-owned changes.
   • Choose the narrower design unless concrete contract evidence requires the current machinery.
6. Select the relevant review dimensions below from the affected runtime boundaries and repository architecture references. Complete every selected dimension even after finding a blocker; the goal is a complete final review, not the first valid comment. Classify review risk before dispatch: normal when the change does not affect concurrency, cancellation, security, trust, persistence, durable state, released compatibility, package/runtime exports, protocol ownership, or cross-provider lifecycle; elevated when any of those boundaries changes or an earlier round produced P0/P1. Run the cheapest affected-boundary preflight broad enough to catch likely late fallout from a dependency, package surface, generated artifact, or cross-cutting runtime change. Prefer focused tests plus a narrowly targeted import, generated-surface, or static check. Run a targeted type check only when the change directly affects a typing boundary and the command is materially narrower than repository-wide . Do not run repository-wide lint, typecheck, builds, integration suites, , or merely to enter or iterate through the review gate. Run the focused preflight once for a semantic state and rerun only affected checks after fixes.

Maintain one compact round ledger throughout all review cycles and persist it as a durable, task-global artifact:

Round | component fingerprints | root-cause groups | highest severity | complexity delta | action | clean credit

Persist enough task identity, used and authorized round budgets, cycle boundaries, fingerprints, root-cause closure state, and clean credit to resume without reconstructing prior rounds. Update it only at a meaningful state transition: round start, accepted finding batch, complexity reset, clean result, verification result, sealed completion, or post-completion feedback-cycle start. Do not emit repeated waiting messages when neither reviewer state nor repository content changed.

Independent reviewer

An independent review uses a fresh no-history context that did not implement the fingerprinted content and is not given prior reviewer findings or implementer conclusions. Prefer a distinct agent and set fork_turns: "none" when the platform exposes that control. A same-context self-review or full-history fork is not independent and cannot satisfy the clean-review gate.

• Give the reviewer the original requirement, implementation scope contract, base and head identifiers, canonical component manifest and fingerprints, raw repository state, and relevant architecture references.
• Give the reviewer the precomputed contract-surface and await-boundary or authority/data-flow inventories. These are coverage maps, not conclusions; require the reviewer to validate every row against the raw diff and surrounding source.
• Tell the reviewer which identifier is the intended target and require an explicit merge-base calculation. When target and head diverge, provide or request a three-dot diff; do not present a two-dot target-to-head diff as the patch.
• Do not give the reviewer the implementer's conclusions, suspected bugs, intended fixes, or a list of expected findings.
• Ask for exactly one read-only review round. The reviewer must not edit or stage files, run the autonomous review loop recursively, spawn another reviewer, or perform the final repository verification. The implementer owns finding validation, edits, loop control, and final verification.
• Give every reviewer for a round the same review-state fingerprint and keep the diff frozen until all of them finish. Reject output produced from a different or changing state instead of merging partial observations across revisions.
• Give every reviewer the compact self-contained control-plane brief, indexed evidence paths and digests, and one exact revalidation command. If any mandatory packet field is neither populated nor explicitly marked none or not applicable, the reviewer must report it and cannot return a creditable clean verdict. Tell reviewers not to inspect memory, rediscover workflow skills, rerun implementation strategy, search for the fingerprint helper, or rediscover the release tag unless supplied evidence is inconsistent or decision-relevant. Reopening source cannot replace missing packet contents. This preserves fresh judgment while avoiding repeated setup work.
• Use fresh reviewers for every round when possible. Do not reveal findings or conclusions from prior rounds; provide only the updated requirement, scope contract, raw final diff, component manifest, and relevant references.
• Use two concurrent fresh reviewers for every round. For the high-risk conditions in step 12, assign complementary high-risk specialties while requiring each reviewer to inspect the complete diff. Both reviewers of the same unchanged diff are one fingerprint round. Do not duplicate broad test execution.
• Concurrent reviewers receive the same fingerprint and raw context but different primary specialties. They must not communicate during the round.
• Give the reviewer existing verification commands and results as raw evidence. The reviewer should inspect code and tests, then run only focused probes needed to resolve a decision-relevant uncertainty. A probe must be demonstrably non-mutating or run in an isolated temporary checkout; any mutation of the reviewed worktree invalidates the round. Do not rerun the repository's broad test, typecheck, lint, build, or integration suites merely to reconfirm the implementer's evidence; the implementer runs the complete stack once after the clean-review gate.

When an independent reviewer is unavailable, rebuild context from the original request, scope contract, source, and complete diff before a best-effort self-review. Explicitly discard incremental-review assumptions, label the result non-independent, and do not count it toward the clean-review gate. Report the unavailable gate at handoff instead of silently weakening it.

Review dimensions

Choose dimensions based on the changed boundary; do not mechanically invent findings for every item.

Requirement and scope

• Verify that the smallest required caller-visible behavior works.
• Identify nearby constructible cases and confirm they are either intentionally supported or rejected before side effects.
• Require contract evidence before treating repeated, concurrent, reentrant, malformed, wrapped, or cross-provider combinations as blockers. Reproduce the same supported scenario on the baseline when claiming a regression.
• Check whether tests accidentally turn implementation permutations into public contract.
• Map every new abstraction, state field, branch, dependency, and cross-module change to a requirement, supported contract, or verified risk.

Compatibility and identity

• Compare released public signatures, field order, imports, names, serialized values, configuration, and wire behavior.
• Preserve exact caller-visible identity or spelling unless transformation is required.
• Distinguish unreleased branch-local machinery from released or durable compatibility boundaries.
• For every new or modified public field, enumerate all construction, forwarding, and consumption branches. Verify that normal, specialized, default, missing-value, and error paths either honor the field or reject it according to one coherent contract; do not validate only the motivating branch.
• Search public docs, examples, docstrings, configuration reference, and release metadata for claims made stale by the behavior change. Missing documentation can be an actionable omission even when no documentation file is in the diff.

Lifecycle and failures

• Trace ownership from acquisition through success, failure, cancellation, retry, replacement, and cleanup.
• When shared lifecycle state changes, build a compact operation-state matrix before concluding. Cover each affected public mutating operation against never-started, partial-failure, active, cleanup-in-progress, and terminal states as applicable.
• Trace repeated sequential calls and every relevant pair of overlapping public mutating operations. Identify the linearization point or manager-owned serialization mechanism; do not infer safety from per-resource deduplication alone.
• Check repeated cancellation, partial initialization, cleanup failure, retry through every supported public entry point, and primary-exception preservation.
• State the final survivor invariant: which tasks, workers, processes, sessions, listeners, files, or remote resources may remain.
• Review ordering when several validations or cleanup actions can short-circuit one another.
• Audit every check-await-side-effect sequence. State may change during the await; require revalidation or prove manager-owned serialization before cancellation, feedback, persistence, or cleanup.
• Distinguish current state from historical evidence. If a stale-result guarantee depends on whether a newer operation ever started, an active pointer that later returns to None cannot prove absence; use or require monotonic evidence unless the operation is serialized.

Security, trust, persistence, and protocol

• Trace caller-controlled data through logs, exceptions, causes, contexts, telemetry, model-visible output, and persisted state.
• Treat serialized state as authority only when the supported trust boundary explicitly allows it.
• Check fail-closed behavior for malformed or ambiguous sensitive inputs without returning or retaining the original value.
• Verify protocol capability ownership, pagination termination, cache ownership, retry and replay safety, wire validation, and tool or call identity when affected.

Behavioral parity

• Compare streaming and non-streaming, sync and async, initial and resumed, direct and wrapped, and provider-specific paths when the requirement crosses them.
• Verify that one path does not silently ignore, reshape, or hard-fail data that another path supports.

Tests and generated public surfaces

• Prefer public-boundary or caller-visible adversarial tests.
• Add controlled interleavings for concurrency instead of relying only on sequential tests.
• Test the required behavior, the nearest supported alternative, and one representative input per unsupported category.
• Do not accept passing existing tests as proof when they encode the same assumptions as the implementation.
• Import through intended consumer entry points and verify generated or distribution artifacts when public package behavior changes; runtime tests alone do not prove the published surface.

Complexity reset

Run a complexity reset when related findings keep expanding the same design, a narrow requirement requires recursive or cached classification, tests enumerate mechanics, representations are inferred in multiple places, or the diff spreads unexpectedly across subsystems.

1. Stop addressing findings one by one.
2. Group them by root cause and restate the original required behavior.
3. Compare the full diff with the merge base or release boundary.
4. Delete branch-local machinery that is not required.
5. Reuse the nearest existing source-of-truth pipeline.
6. Narrow unsupported behavior and reject it before side effects with a supported alternative when one exists.
7. Rebuild tests around caller-visible invariants and representative negative cases.
8. Compare the replacement's runtime and test complexity with both the previous round and the merge base. A reset that only renames or redistributes a growing state machine is not a reset.

Review-state workspace entries must use the exact key set emitted for their file, symlink, gitlink, directory, or missing kind; incomplete or unknown fields fail before dispatch.

For reusable verification credit, a receipt command must exactly match a structured command in verification.preflight_results; a different successful command cannot inherit verification credit.

Review output

Return exactly one JSON object using the schema in references/reviewer-brief.md. Put the verdict in verdict; put each actionable finding in findings with its priority, title, location, concrete failure scenario, user-visible consequence, support basis, baseline-versus-patch evidence when applicable, smallest safe correction, and stable root-cause ID. Account for every assigned inventory ID and keep unverified runtime uncertainty explicit. Do not claim implementation completion until both structured clean reviews and required verification apply to the exact final state.