[P0][Spec-first] Bound PR review loops and preserve frozen intent

[seoseo-ai]

Change summary

Introduce a PR-lineage-wide bounded review lifecycle that preserves a frozen intent contract, prevents moving-goalpost findings and automatic intent erosion, and terminates safely when review/correction budgets are exhausted.

Size classification

Large — spec + clarify/analyze + plan + tasks + detached evidence lane required.

Problem / operator need

A review system can be individually fail-closed yet operationally unsafe when it has no global liveness bound. A recent independent-review incident outside A2A spent more than 24 hours on one PR, repeatedly regenerated multi-reviewer verdicts after each tree change, absorbed findings outside the original PR intent, and expanded the patch instead of converging. The failure was not that review existed; it was that local retry/fix limits did not compose into a PR-wide stop condition and reviewers could effectively move the goalposts.

A2A Nexus has stronger controls than that pipeline: advisory review lanes are evidence-only, the finalizer is separated from the author, ordinary retry policy hard-denies review/acceptance failures, and signed finalizer verdicts bind to an exact subject. However, current contracts still admit the same class of liveness and intent-drift failure:

• docs/specs/a2a-dialectic-review-mode/analyze.md explicitly records no maximum review duration.
• docs/implementation-pipeline.md requires a bounded fix list but does not bound total correction generations across verifier → A2AD → finalizer → GitHub review.
• docs/a2ad-round-dispatch.md limits one rebuttal pass but permits “another bounded round”; individually bounded rounds can compose into an unbounded PR lineage.
• packages/broker/src/worker-review.ts validates reviewer identity, pass/fail, and note, but the basic review receipt is not required to bind the reviewed HEAD/diff and frozen intent contract.
• packages/broker/src/worker-review.test.ts demonstrates that rejected completion evidence leaves the task running; repeated completion submissions are therefore possible even though review_verdict_failed is hard-denied by ordinary task retry policy.
• docs/operators.md records real false findings from synonym/config-flow misreads (C6-R: #1204 잔여 완성 — CHANGELOG 기재, per-process 한계 문서, 3-컴포넌트 e2e, 테스트명 정직화 (+ 부재-검출 게이트) #1209) and already classifies spec_ambiguity and scope_drift as distinct failure modes.
• Work-mode benchmark fixtures record rework amplification and superseded/late lanes even when wall-clock closeout remained short. Multi-lane review is not inherently convergent.

This matters directly to #1499. Enforcing finalizer/CodeQL gates is necessary, but a strict merge gate without a bounded resolution lifecycle risks producing a system that is safe against unreviewed merge yet unable to terminate or preserve the author's approved intent.

Required invariants

1. Frozen intent, not reviewer preference, is the oracle. Reviewers evaluate the original goal, non-goals, invariants, acceptance criteria, and declared scope. They cannot silently rewrite them.
2. Review is read-only. Reviewer/finalizer lanes return evidence and findings; they do not mutate the author branch. An optional fixer may only produce an isolated patch candidate and must never auto-push it.
3. One global budget per PR lineage. Preflight, implementation verifier, A2AD, finalizer correction, and GitHub review must not each reset independent correction counters.
4. Findings are stable objects. Blocking findings carry stable IDs, criterion/evidence references, introduced-at HEAD, and disposition. A new reviewer cannot restart the issue list from scratch.
5. Second-pass review is a resolution check, not a new design review. New blockers after the first correction are allowed only for regressions introduced by that correction, newly exposed critical security defects, or demonstrably unavailable evidence at the first pass.
6. Budget exhaustion is terminal and visible. The system returns blocked_needs_operator or intent_conflict; it never keeps editing until a model says PASS.
7. Exact subject binding remains fail-closed. These liveness controls must not weaken finalizer verdict HEAD binding, reviewer independence, evidence requirements, CodeQL, or approval boundaries.

Scope

In scope

• Define IntentContractV1 with at least:
  • goal;
  • non-goals;
  • invariants;
  • acceptance criteria with stable IDs;
  • declared/allowed paths;
  • original base/head SHA;
  • canonical intentHash.
• Define a PR-lineage review budget shared across all review/correction layers, with at least:
  • maxWallClockSeconds;
  • maxCorrectionGenerations (default recommendation: 1);
  • maxReviewerRuns (default recommendation: 2);
  • maxReviewerReplacements (infrastructure failure only);
  • repeated-finding/no-progress threshold;
  • terminal exhaustion behavior.
• Define a lifecycle/state machine such as:
  • reviewing_initial;
  • correction_pending;
  • reviewing_resolution;
  • passed;
  • blocked_needs_operator;
  • intent_conflict;
  • canceled.
• Bind basic independent-review receipts to headSha, diffHash, intentHash, reviewer identity, and stable finding ledger.
• Define blocking-finding eligibility:
  • mapped acceptance/invariant reference;
  • concrete repository/test evidence;
  • severity/category;
  • first-seen and resolved-at HEAD;
  • non-blocking treatment for style, preference, scope expansion, and optional design improvements.
• Prevent moving goalposts after the initial review. Resolution review may reopen an existing finding or add only an introduced-regression / critical-security finding with explicit justification.
• Define an appeal/finalizer path for disputed findings. The same reviewer must not veto indefinitely; exactly one finalizer records the disposition.
• Add intent-drift/scope-drift guards comparing each correction generation with the frozen intent contract and declared paths.
• Keep the original author head/branch recoverable; correction candidates are additive child generations, not destructive rewrites of the only copy.
• Add operator-visible metrics:
  • elapsed wall time;
  • correction generation count;
  • reviewer-run/replacement count;
  • finding churn (new/reopened/resolved);
  • repeated/no-progress signature;
  • scope/intent-drift disposition;
  • terminal stop reason.
• Define rollout modes (off / record / enforce) so metrics and false-positive behavior can be measured before broad enforcement.
• Reconcile the design with [P0] Enforce finalizer verdict and CodeQL policy in the active main ruleset #1499 so finalizer enforcement and bounded resolution are both present without weakening either control.

Out of scope

• Weakening or bypassing finalizer verdict, CodeQL, reviewer-independence, required-check, or operator-approval gates.
• Automatically accepting reviewer-authored changes.
• Allowing reviewer/finalizer lanes to push, merge, deploy, restart, publish, or perform other side effects.
• Model/provider-specific prompting as the primary safety boundary.
• Production deploy/restart/canary, DB/outbox/ACK/replay/prune/migration, provider sends, release/tag/package publication, secret movement, visibility change, history rewrite, or force push.

Spec-first packet

Proposed path: docs/specs/bounded-pr-review-lifecycle/

• Spec: spec.md — intent contract, global lineage budget, lifecycle states, reviewer/finalizer authority.
• Clarify: clarify.md — what constitutes a new PR lineage, semantic vs metadata-only HEAD changes, finding eligibility, operator override semantics.
• Analyze: analyze.md — safety/liveness trade-offs, exact-HEAD freshness interaction, false-positive and reviewer-replacement risks, compatibility with [P0] Enforce finalizer verdict and CodeQL policy in the active main ruleset #1499.
• Plan: plan.md — additive schema/contracts first, record-mode telemetry, then enforcement.
• Tasks: tasks.md — required before implementation.
• Checklist: checklist.md — contract, broker, fixture, docs, CI, and detached-review closeout.

Affected repos/components

• a2a-nexus contracts/specs/conformance fixtures.
• packages/broker task lifecycle, review receipt validation, retry/terminal-state integration, read models, and metrics.
• scripts/a2ad-finalizer-gate.mjs and finalizer-verdict integration where lineage budget/finding ledger evidence is consumed.
• Docker runner / patch lanes only for isolated patch-candidate and immutable-original-head guarantees.
• CLI/API/gateway preset surfaces that initiate review.
• Operator docs, implementation pipeline, round dispatch docs, and benchmark/scorecard schema.
• GitHub workflow/ruleset documentation coordinated with [P0] Enforce finalizer verdict and CodeQL policy in the active main ruleset #1499; repository settings mutation remains separately approved.

Evidence / validation contract

Deterministic contract tests

• IntentContractV1 canonicalization produces a stable intentHash; any goal/non-goal/invariant/acceptance/scope change changes the hash.
• A basic review receipt with a mismatched headSha, diffHash, or intentHash fails closed.
• Reviewer identity equal to author remains rejected.
• Review/finalizer lanes cannot carry write-capable execution authority.
• A failed initial review permits at most the configured correction generation count.
• Exhausted wall-clock, reviewer-run, correction-generation, or no-progress budgets transition to blocked_needs_operator, not running or an automatic retry.
• A correction that changes the frozen intent transitions to intent_conflict and requires explicit operator disposition.
• Resolution review can resolve/reopen prior finding IDs but rejects a new preference/scope-expansion blocker.
• Resolution review may add an introduced-regression or critical-security blocker only with exact evidence and justification.
• Repeated identical unresolved finding signatures trigger early stop before the outer budget is consumed.
• Reviewer replacement is allowed only for classified infrastructure failure and does not reset the PR-lineage budget.
• Metadata/evidence-only HEAD changes follow the documented freshness path without weakening exact finalizer subject binding.
• Existing task retry hard-deny behavior and finalizer-verdict verification remain green.

Integration / simulation

• A deterministic fixture simulates the full initial-review → one correction → resolution-review flow and reaches PASS without creating a third generation.
• A non-converging fixture simulates repeated findings and reaches blocked_needs_operator within the configured budget.
• A moving-goalpost fixture proves that a second reviewer cannot introduce an unrelated design blocker.
• A scope-drift fixture proves that an auto-generated patch outside declared paths is rejected and the immutable original remains recoverable.
• Record-mode scorecard reports elapsed time, generation count, finding churn, and stop reason without private prompts or chain-of-thought.
• Focused broker tests, finalizer gate tests, conformance tests, npm run check, public-readiness scan, and CI pass.
• Detached independent review confirms the implementation preserves original intent and does not create a new auto-fix loop.

Suggested implementation order

1. Spec/clarify/analyze and machine-readable fixtures.
2. Additive IntentContractV1, review receipt, finding ledger, and lineage-budget schemas.
3. Broker record-mode state/read-model integration with no behavior change.
4. Early-stop and terminal exhaustion behavior.
5. Resolution-review finding restrictions and isolated patch-candidate boundary.
6. Enforce-mode conformance and integration with finalizer gate / [P0] Enforce finalizer verdict and CodeQL policy in the active main ruleset #1499.
7. Benchmark and scorecard readback before any broad default-on transition.

Relationship to existing work

• [P0] Enforce finalizer verdict and CodeQL policy in the active main ruleset #1499 remains required: this issue does not propose bypassing finalizer or CodeQL enforcement.
• Loop reliability gaps: early-stop, verifier regression guard, artifact-injection cap, cost-per-accepted metric, periodic permission/skill re-audit #1027 introduced task-level repeated-error early-stop direction; this issue extends the concept to the whole PR review lineage, where individually bounded layers can otherwise compose into an unbounded correction loop.
• Existing spec_ambiguity, scope_drift, and false-finding guidance in docs/operators.md should become machine-visible dispositions rather than only narrative guidance.

Safety / approval boundaries

• This issue does not itself approve production deploys, Gateway/broker/worker restarts, live canaries/provider sends, DB/outbox mutation, manual Terminal Brief ACK/replay, release/tag, secret movement, visibility changes, history rewrite, force push, or GitHub ruleset mutation.
• Any approval-sensitive action will be requested separately with exact scope and rollback/cleanup notes.
• Evidence will be redacted and will not include secrets, private endpoints, provider IDs, Telegram IDs, production data, raw session dumps, or runtime/bootstrap files.
• Exactly one broker/finalizer will own final closeout judgment.
• Reviewer and advisory lanes remain read-only; optional fixer output is proposal-only and cannot auto-push or silently alter the approved intent.

[jinon86]

Review lifecycle defect observed during PR #1548

Two exact-head source-only review attempts with payload.review.required=true produced structured reviewer validation but were rejected as review_not_independent:

• a2a-nexus-pr1548-final-review-20260713T062954Z:nosuk-worker-runtime
• a2a-nexus-pr1548-final-review-20260713T062954Z:yukson-runner-transport

Current cause: validateTaskCompletionEvidence() calls validateReviewEvidence(task, result) without a trusted author worker ID. The validator then defaults author to task.claimedBy / assigned worker, which is the reviewer for a self-contained review task, so it compares the reviewer against itself and always fails independence.

Separate pitfall confirmed: adding a harmless-looking payload.acceptance.command=["/usr/bin/true"] to analysis review tasks independently requires a smoke validation and fails as acceptance_evidence_missing when the analysis bridge does not execute acceptance.

For #1548, final evidence used signed/countersigned analysis-only PASS/FAIL tasks plus a separate non-author GitHub approval. A bounded #1518 slice should carry a trusted author identity into validateReviewEvidence, test author/reviewer independence, and keep review validation independent from smoke acceptance.