Skip to content

refactor(review): retire FINALIZE through last-event atomic closure #3587

Description

@Alan-TheGentleman

Pre-flight Checklist

  • I have searched existing issues and this is not a duplicate.
  • I understand that PRs will be rejected if the linked issue does not have status:approved.

📝 Bug Description

Ordinary atomic review still exposes review finalize as a separate mutation after all reviewer or validator inputs have already been admitted. That extra lifecycle round reconstructs candidate views, rebinds workspace state, collects general final evidence, and can enter recovery even though review is informational and delivery is unmanaged.

In the current atomic architecture, FINALIZE owns no independent product decision. CaptureAdmittedReviewerResult already admits each exact candidate-bound lens result, while ReviewCore.finalize primarily verifies that all selected lenses are present and reduces the captured set to approve or escalate. Keeping those operations separated creates a failure surface after the substantive work is complete.

This is the root class behind multiple FINALIZE, correction-evidence, targeted-validation, intended-untracked, and recovery dead ends. The fix must shrink the lifecycle: the last admitted event becomes the atomic reducer and burn trigger.

🔄 Steps to Reproduce

  1. Start an ordinary high-risk review for an exact workspace candidate and admit all four selected reviewer results.
  2. Observe that STATUS still requires a separate FINALIZE operation instead of reducing the completed captured set.
  3. Enter the bounded correction path, capture repository verification evidence, and obtain the requested targeted validator result.
  4. Invoke FINALIZE to submit the already-bound validator result.
  5. Observe that candidate-view reconstruction or intended-untracked/index reconciliation can fail after review and verification have completed, leaving STATUS to request recovery for review plumbing rather than a candidate defect.

Concrete sanitized failures observed on current main:

candidate-view-invalid: candidate view context is missing or ambiguous for FINALIZE
mutation_performed: false

review/recover: intended-untracked path "<path>" is already tracked
mutation_outcome: unknown

The failed FINALIZE also changed intended-untracked paths to staged entries while reporting no mutation. Candidate bytes and the current candidate tree remained unchanged.

✅ Expected Behavior

The lifecycle closes on the last admitted event:

  • The last reviewer capture acquires the authority lock, verifies that every selected lens is present, reduces the complete admitted set exactly once, returns advisories, and atomically burns authority when no candidate-caused severe finding survives admission.
  • Candidate-caused severe findings transition directly to the one bounded correction.
  • After a correction, the admitted targeted-validator result directly burns or escalates the lineage.
  • Clean 4R completion requires no general final-evidence collection and no separate FINALIZE round.
  • STATUS remains read-only and never offers review.finalize.
  • Review remains informational and never governs commit, push, PR, archive, release, or other repository delivery.

❌ Actual Behavior

The lifecycle has two closing authorities:

  1. capture operations admit the exact results;
  2. FINALIZE reconstructs context and decides whether those already-admitted results may close.

The second authority can fail or mutate client workspace/index state after the candidate has already passed SDD verification, repository tests, and independent review. Recovery then diagnoses review transport state instead of candidate correctness.

🖥️ Environment

  • Gentle AI Version: gentle-ai dev, main@51a0d60b
  • Operating System: Linux (Other)
  • AI Agent / Client: Other — Pi
  • Affected Area: CLI (commands, flags)

Acceptance Criteria

Clean path

  • review capture-result admits one exact lens result and, when it is the final outstanding selected lens, elects exactly one closer under the existing authority lock.
  • A complete admitted set with no candidate-caused BLOCKER/CRITICAL finding returns terminal advisories and atomically burns authority in the same operation.
  • Concurrent final-lens submissions cannot create two closers, duplicate outcomes, or surviving authority.
  • No clean 4R path requests repository verification evidence or a separate finalize invocation.

Correction path

  • A complete admitted set with candidate-caused severe findings transitions directly to the existing single bounded correction.
  • The exact requested targeted-validator result is the terminal event: pass burns; failure escalates.
  • General final-evidence capture is not a second review authority. Repository tests remain ordinary SDD/repository evidence.
  • A validator that refutes the original finding cannot be converted into approval by caller-authored interpretation.

Public contract removal

  • Delete gentle-ai review finalize as a public lifecycle command and capability.
  • Delete FINALIZE from negotiated transitions, schemas, help, prompts, adapters, runtime assets, sanctioned-exit messages, fixtures, journeys, and docs.
  • No compatibility command, parser, alias, migration reader, or dual route remains.
  • Pi and other adapters transport capture events opaquely; shared Go contracts own reduction and burn semantics.
  • Driven evidence proves consent → freeze → four captures → terminal response + burn and correction → validator → terminal response + burn/escalation.

Delivery and cleanup boundaries

Root-Cause Cluster

This issue is the canonical architectural line. During implementation, audit and disposition the open symptom reports involving separate FINALIZE/correction closure, including #2478, #2503, #3014, #3134, #3194, #3205, #3427, #3450, #3467, and #3476. Close or supersede only when named driven tests prove each reported symptom is unreachable.

Additional Context

The implementation must preserve the atomic foundation:

consent → freeze → lenses → [bounded correction → targeted validator] → terminal response + burn

“Finalize” may remain as an internal pure reducer name temporarily during implementation, but it must not survive as a public phase, command, transition, or caller-supplied decision point. The final architecture has one closing mutation: the last admitted event.

Activity

  1. Alan-TheGentleman commented on Aug 22, 2026

    @Alan-TheGentleman
    ContributorAuthor

    Implementation plan is locked. This will ship as one atomic PR with a maintainer-authorized size:exception; splitting the public contract across main would either break every adapter between slices or require the compatibility layer this change explicitly forbids.

    Work units inside the PR:

    1. Reducer semantics: a complete admitted review with no surviving candidate-caused severe finding becomes transiently approved from the admitted result set itself. Clean review no longer enters general final verification.
    2. Event-owned closure: the last required admitted event closes the lifecycle. This is the last lens capture for a clean review, refuter capture when one is required, and targeted-validator capture after a bounded correction. One closer wins under native authority locking; approval immediately enters the existing atomic burn.
    3. Public clean break: delete review finalize from command routing, negotiated transitions, capabilities, contracts, adapters, prompts, sanctioned exits, fixtures, and runtime journeys. No alias, compatibility parser, dual reader, or migration route remains.
    4. Parity and proof: add deterministic concurrent-last-capture coverage plus driven clean-4R and corrected-candidate journeys. STATUS must remain read-only and must never offer FINALIZE.

    Broad unreachable-provider deletion is deliberately deferred to #2423 after this functional cut. That keeps the architecture change reviewable as behavior rather than mixing it with a repository-wide dead-code sweep.

    Open mechanism patches will be dispositioned against this root line, not stacked onto it: #3576's one-writer burn invariant is absorbed by last-event closure, #2033's FINALIZE parser surface is deleted, and #3483's correction-forecast FINALIZE surface is deleted. Those PRs should not merge independently once named #3587 tests prove their reported states unreachable.

  2. added a commit that references this issue on Aug 23, 2026
    0ed9225
  3. Juancho1162 commented on Aug 27, 2026

    @Juancho1162

    Stable-channel occurrence on gentle-ai 2.4.0 (Darwin arm64, Pi via gentle-pi 2.2.0):

    • Correction repository evidence was submitted with outcome passed.
    • The next STATUS offered a bound targeted_validation_required transition and rendered --captured-evidence=true.
    • Submitting that exact bound validation failed in preflight with invalid_request: compact correction acceptance requires captured repository verification evidence.
    • Native reported mutation_outcome: not_started and retry_safe: true; the lineage and corrected candidate remained preserved. No blind retry was attempted.

    Expected: after passed repository evidence, targeted validation consumes that admitted evidence, or STATUS remains on evidence collection. Actual: STATUS advances to validation while validation denies the evidence record.

    I found the fix merged in #3627 and published in v2.5.0-rc.1; the latest stable release remains v2.4.0. This records the stable-channel occurrence only and does not request reopening.

  4. carolitascl commented on Aug 29, 2026

    @carolitascl

    New rc.1 recurrence: the correction-plan transition still names the retired review finalize verb.

    Environment

    • Gentle AI: 2.5.0-rc.1
    • Client: Pi controller wrapper
    • OS/architecture: macOS arm64
    • Contract: gentle-ai.review-integration/v2

    Observed

    1. A high-risk staged review captured all four reviewer results and entered correction_required with a frozen 170-line budget.
    2. Fresh STATUS returned collect/correction_plan_required with correction_lines; its submission descriptor named operation_token: finalize and included a bound request hash and repository-context handle.
    3. Submitting an in-budget 160-line forecast through the controller failed before mutation as review.finalize invalid_request.
    4. Executing the exact provider-issued correction descriptor directly against rc.1 failed with unknown review command "finalize".
    5. Fresh STATUS repeated the same correction-plan collect. No correction bytes, commit, push, PR, reset, or recovery operation occurred.

    Expected
    After review finalize was retired through last-event closure, STATUS must not emit a correction-plan descriptor that names that retired verb. A correction-required lineage needs a supported, revision-bound path to submit its forecast and continue.

    Actual
    STATUS requires correction_lines but routes it through the retired finalize operation, leaving the lineage unable to begin its one permitted correction.

    Minimal reproduction

    1. Start a high-risk staged review.
    2. Capture the required reviewer results, producing correction_required.
    3. Read fresh STATUS and submit the positive bounded correction_lines value from its descriptor.
    4. Observe controller invalid_request; run the descriptor directly and observe unknown review command "finalize".

    Preserved state

    • Lineage: review-4352dce3d06fa986
    • Authority revision: sha256:ca653cb650235a6496d3ccbc2f4dae355935f916f9754c3d09dbd61404d1c634
    • Candidate remains staged and uncommitted; the review remains correction_required.
  5. added a commit that references this issue on Sep 24, 2026
    838977f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority:highHigh prioritystatus:approvedApproved for implementation — PRs can now be openedtype:breaking-changeBreaking change (requires major version bump)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions