Skip to content

bug(review): correction FINALIZE cannot bind its provider-owned targeted validation or its captured evidence #279

Closed as not planned

Description

@servohatred

Pre-flight checks

Summary

An ordinary bounded review that reaches correction_required cannot be finalized through the Pi wrapper. Three independent wrapper defects block the same single operation, in sequence — fixing any one alone leaves the lineage stuck, because the next one blocks at the same step.

The native runtime is not at fault. The installed binary is byte-identical to the published release asset, and the contract it enforces is unchanged across v2.2.2, v2.2.3, and v2.2.4. In all three defects the wrapper sends a request the native contract does not accept.

Frozen audit baseline

Component Audited revision
gentle-pi npm latest / installed 2.1.2
gentle-pi main 19b0ed7d8caa3f90657d5f460bc0f303c22ab00d (2026-08-01)
Gentle AI installed (pinned by 2.1.2) v2.2.2, binary SHA-256 149b97248552c5e03ebc4d991f86b1360fb847a40fc315555a8aa256f95baca0
Gentle AI pinned by main v2.2.3
Gentle AI latest release v2.2.4

The installed binary digest matches the published gentle-ai_2.2.2_darwin_arm64.tar.gz asset exactly, so no local provider modification is involved.

Real lineages used as evidence: review-b38b2df3d395f581 and review-ec5c73e3b693d4dd. Both were eventually approved by driving the native CLI directly with a hand-built request; neither could be finalized through gentle_review.


Defect 1 — the targeted validation document drops both provider-owned binding identities

Location: lib/review-compact-contract.ts:358-364 (on main at 19b0ed7d)

export function toNativeValidatorDocument(input: CompactValidationProofInput | CompactTargetedValidationInput) {
	return {
		original_criteria: input.original_criteria,
		correction_regression: input.correction_regression,
		follow_ups: "follow_ups" in input ? input.follow_ups.map((row) => ({ observation: row.summary, proof_refs: [...row.proof_refs] })) : [],
	};
}

The native --validation document requires exactly five fields:

{
  "targeted_validation_request_hash": "sha256:<64 lowercase hex>",
  "correction_target_identity": "sha256:<64 lowercase hex>",
  "original_criteria":      { "passed": true, "evidence": ["..."] },
  "correction_regression":  { "passed": true, "evidence": ["..."] },
  "follow_ups": []
}

Decoding is strict, so extra fields are rejected and the two binding identities are mandatory. Because the serializer emits only three fields, native FINALIZE fails with:

targeted validation result does not bind the provider-owned correction request

Both identities are already available to the wrapper on ReviewStatusV3.validationRequest (requestHash and correctionTargetIdentity). They must be read from provider status, never from the caller-supplied compact payload — the compact request_hash is a bare 64-hex value used only for matching, while the native binding fields must stay canonical sha256:-prefixed identities.

I tested all four prefix combinations against v2.2.2. Only prefixed/prefixed binds.

Defect 2 — negotiated FINALIZE never requests captured-evidence discovery

Location: lib/native-review-cli.ts:2158-2172 (negotiated FINALIZE argv). NativeFinalizeRequest has no capturedEvidence field at all.

The wrapper only ever emits --evidence <file>. It never emits the published --captured-evidence=true flag, so on a correction FINALIZE the provider is never told to discover the record it already persisted:

compact correction acceptance requires captured repository verification evidence

review capture-evidence succeeds and durably persists the record. The wrapper must then let the provider discover it via --captured-evidence=true — it must not retransmit the raw evidence, and it must not read the provider's authority storage itself. Supplying both transports at once should fail closed before launch.

Defect 3 — captured evidence identities are decoded without canonicalization

Location: lib/native-review-cli.ts:904-924, decodeNativeReviewVerificationEvidence

authorityRevision: requiredString(body.authority_revision),
targetIdentity: requiredString(body.target_identity),
pathsDigest: requiredString(body.paths_digest),
rawPayloadSha256: requiredString(body.raw_payload_sha256),

The provider emits these as bare 64-hex, while the wrapper later compares them against sha256:-prefixed status fields, so verification fails after a successful capture:

correction-evidence-binding-drift

A decoder that accepts either form and normalizes to canonical sha256:<64hex> resolves it. Note that candidate_tree is a Git tree ID, not a SHA-256 digest, and must be left untouched.


Steps to reproduce

  1. Install gentle-pi 2.1.2 (or build main at 19b0ed7d).
  2. Start an ordinary review whose selected lens returns one deterministic candidate-caused CRITICAL finding, so the lineage reaches correction_required.
  3. Submit a positive correction_line_forecast.
  4. Apply a bounded in-budget correction.
  5. Run the targeted validator once; it returns both checks passed.
  6. Call gentle_review finalize with the validation payload.

Actual behavior

FINALIZE is rejected in preflight with no authority mutation. Fixing each defect surfaces the next, in this order:

  1. correction-evidence-binding-drift (Defect 3)
  2. compact correction acceptance requires captured repository verification evidence (Defect 2)
  3. targeted validation result does not bind the provider-owned correction request (Defect 1)

The lineage stays in correction_required / targeted_validation_required. Reviewer and validator have each already run exactly once and cannot honestly be rerun, so the only escape is to abandon the lineage or drive the native CLI by hand.

Expected behavior

Given an already-captured evidence record and a passing targeted validation, negotiated FINALIZE mints the terminal receipt without rerunning any review actor.

Acceptance criteria

  • toNativeValidatorDocument emits the exact five-field native targeted shape, sourcing both binding identities from ReviewStatusV3.validationRequest.
  • Absent or non-canonical provider binding fails closed before native launch.
  • Legacy validation_proof keeps its existing three-field shape.
  • Negotiated FINALIZE emits --captured-evidence=true when the provider has already captured the correction evidence.
  • Supplying captured-evidence discovery together with a raw evidence document fails closed before launch.
  • Captured evidence identities are accepted in bare or prefixed form and normalized to canonical sha256:<64hex>; candidate_tree is excluded.
  • A test covers the fresh-process continuation: a restart between evidence capture and FINALIZE completes without recapture or actor rerun.
  • A test covers the same-process capture-then-finalize path.

Relationship to existing issues

#228 — fix(review): accept hashless validation proof in correction finalize. This issue states as fact:

The rejected hash has no native data-flow role: toNativeValidatorDocument() sends only original_criteria, correction_regression, and follow_ups to the native CLI.

The second clause is correct and is Defect 1. The first clause is falsified by the native contract: without targeted_validation_request_hash the provider refuses the FINALIZE. Implementing #228 as written — accepting a proof without the hash — would cement Defect 1 rather than fix it. The strings are present in v2.2.2, v2.2.3, and v2.2.4.

#200 — fix(review): expose validation_request during negotiated FINALIZE. Same visible symptom (a circular block in correction_required), different root cause: the wrapper not exposing validation_request at all. In 2.1.2 status does expose it. Defect 1 is the next layer: the wrapper holds validationRequest and discards its binding fields while serializing.

#261 references correction_target_identity only as a manual workaround for capture-evidence --target when driving the native CLI directly; it does not report any wrapper defect.

#256 track 4 (reviewer/validator recovery) is adjacent but covers none of these three.

Patch

A working patch against 2.1.2 is available (525 source lines + 491 test lines across 9 files, all inside the npm package; the vendored native binary is untouched). The lib/review-compact-contract.ts portion is 15 lines and self-contained. Happy to open a PR once this is status:approved.

Environment

  • gentle-pi: 2.1.2 (npm latest)
  • Gentle AI: v2.2.2 (verified against v2.2.3 and v2.2.4)
  • Platform: macOS darwin/arm64
  • Client: Pi

Activity

  1. hu9osaez commented on Aug 10, 2026

    @hu9osaez

    Is #279 still open for contribution, or is this expected to land through #256 or another in-progress branch?

    I checked current main: it still pins Gentle AI 2.2.3, and I couldn't find the captured-evidence transport, provider-owned targeted-validation binding, or the negotiated 2.3 protocol schemas. PR #291 appears limited to correcting the devbinary projection fixture and explicitly leaves the parity work open.

    I have a local 2.3.0-rc.2 bridge implementation with the focused suites passing (164 tests), the packed-runner E2E passing, and the remaining full-suite failure reproduced on the base tree as the existing path-sensitive orchestrator prompt-budget check. If this contribution is still useful, should I open the PR, and could a maintainer mark this issue status:approved?

  2. Alan-TheGentleman commented on Aug 14, 2026

    @Alan-TheGentleman
    Collaborator

    This remains a valid defect for the currently shipped release line, but Pi-side reconstruction of targeted-validator documents is not the durable v2.4+ direction.

    The complete parity contract is now #311. Gentle AI owns the targeted-validator task, schema, binding, admission, captured-evidence discovery, retry accounting, receipt, and gate. Pi must eventually relay the opaque provider task and return raw output only. It must not extend toNativeValidatorDocument into a second authority path.

    A narrowly scoped legacy fix may proceed only if it preserves current released compatibility and is independently removable. The long-term resolution is blocked on the first-class Pi/generic host relay and released provider bundle described in #311, followed by deletion of local reviewer, refuter, and validator semantics.

  3. GinoL221 commented on Aug 21, 2026

    @GinoL221

    Reproduced this correction-finalize cluster on gentle-pi 2.2.0 with package-local Gentle AI 2.4.0. The current failure is a narrower post-evidence binding bug that is not described in the issue body.

    The native status at correction_required correctly offered exactly one review.capture-evidence input with reason correction_repository_verification_required. The correction changed 2 paths while the frozen review projection contained 13 paths. Evidence capture succeeded and persisted a gentle-ai.review-verification-evidence/v2 record whose target identity, corrected candidate tree, 2 correction paths, paths digest, ledger ID, authority revision, and passed outcome exactly matched the provider-owned targeted-validation request.

    The combined wrapper submission then failed with:

    evidence-first-ordering: passed evidence did not produce one provider-bound targeted validation request
    

    After that successful capture, the two resumptions also dead-ended:

    validation only -> compact correction acceptance requires captured repository verification evidence
    evidence only   -> targeted validation was offered before correction evidence was captured
    

    The rejection is introduced by requireTargetedValidationAfterEvidence() in extensions/gentle-ai.ts:5471-5490. It requires request.correctionPaths and request.correctionPathsDigest to equal status.projection.paths and status.projection.pathsDigest. That compares the exact 2-path correction scope against the full 13-path frozen projection, even though the captured record and targeted request agree with each other. The provider ordering and bindings are internally consistent.

    The focused controller fixtures mask this shape: bindTargetedValidation() in tests/review-controller-native-routing.test.ts:486-499 sets correctionPaths directly from status.projection.paths, so the tests cannot represent a correction that touches a strict subset of the reviewed paths.

    A regression should use different projection and correction scopes, assert status -> capture-evidence -> status -> targeted validation, and cover fresh-process/post-capture continuation without recapturing evidence. The post-evidence guard should validate the targeted request against the captured correction evidence/binding, not require equality with the complete frozen projection.

  4. edwinsaavedran commented on Aug 22, 2026

    @edwinsaavedran

    Reproduced this on gentle-pi 2.2.0 with Gentle AI 2.4.0. This confirms the post-evidence dead end reported above and adds a target-binding failure mode after a bounded correction.

    The ordinary review reached correction_required, accepted a 24-line forecast, and captured a targeted request for a two-file correction. Real PostgreSQL validation passed 8/8 adversarial cases with zero skips, and the affected race suites passed. The wrapper then produced the same mutually exclusive failures:

    combined evidence + validation -> evidence-first-ordering: passed evidence did not produce one provider-bound targeted validation request
    validation only               -> compact correction acceptance requires captured repository verification evidence
    evidence only                 -> targeted validation was offered before correction evidence was captured
    

    A later one-line fixture correction, still within budget and leaving the reported candidate tree unchanged, changed the live snapshot identity. inspect continued to report applicability: current_target and action: finalize, while FINALIZE rejected the live target as stale against the original authority target. The lineage now has no receipt and cannot be committed without bypassing the gate.

    Could a maintainer confirm the supported non-destructive recovery for an already-stranded lineage, or whether the only safe action is to preserve it and wait for the #311/provider-relay fix? I have not reset, abandoned, edited authority records, or driven the native CLI manually.

  5. Alan-TheGentleman commented on Aug 23, 2026

    @Alan-TheGentleman
    Collaborator

    Closing as obsoleted: gentle-pi main already retired the delivery command gates, lifecycle wrappers, and the bash lifecycle guard (7214cec and the atomic review relay), and upstream gentle-ai removed FINALIZE, receipts, and delivery gates entirely with the last-causal-event refactor. The wrapper/gate/finalize/recovery surface this issue reports against no longer exists in either codebase. #404 (consume last-event closure and retire FINALIZE orchestration) is the canonical tracker for the remaining parity work; if a comparable defect survives that migration, please file a fresh report against it.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions