fix(pipeline): preserve review findings and bind attestations

[Blakeolson21]

Intent

Owner-approved port of two companion upstream items into no-slop on a feature branch, then test, gate, land, push, and verify. For merged upstream PR #797, close the cheap gap over no-slop's existing structured v1 step attestations by making the required workflow bind the attestation to the current PR head and require review, test, and document to be completed. For open upstream PR #704, adopt the worthwhile idea that unresolved review findings must survive silent or narrower later rounds, but do not blindly cherry-pick its 23-file head: write it against current no-slop and address the open P1 where a finding selected only at a later carried gate could be dropped before verification while older history still called it ignored. Preserve stable finding identity, the stricter action, evidence, recovery, statistics, and later selection semantics; persist the effective operator-visible gate truth so round history and recovery agree. Keep non-review step semantics unchanged. Produce separate item SHAs and verify the landed default branch. This recovery branch preserves every commit from the original pipeline's semantic rebase and CI repair, plus a narrow compatibility fix allowing the historical kunchenguid/no-mistakes marker only when a valid current v1 attestation is present; a historical signature without that attestation must remain rejected.

What Changed

• Preserve unresolved review findings across rereviews and recovery, including stable lineage, stricter actions, evidence, effective risk, statistics, and later selection semantics.
• Bind PR attestations to a unique publication and the current head, requiring review, test, and document to be completed and certified against that commit.
• Harden GitHub publication recovery and stale-check handling, while accepting the historical no-mistakes marker only alongside a valid current v1 attestation.

Risk Assessment

✅ Low: No material source defect or intent contradiction remains after the latest fixes; attestation ordering, review recovery, statistics, and historical-marker behavior are coherently enforced.

Testing

Diff and commit ancestry inspection, focused workflow/pipeline/database/type tests, a real CLI e2e journey, and direct execution of the required-workflow shell all passed. Evidence shows unresolved review findings remain visibly parked and that valid current attestations are accepted while stale, incomplete, or signature-only bodies are rejected.

Evidence: Required workflow acceptance/rejection matrix

Command: go run /private/tmp/ns-port-home.uuWWgE/evidence/01M0RWRC474AQ4315SBMJ8D4H7/workflow_acceptance_demo.go

CASE: current marker + current completed attestation
RESULT: accepted
Found no-slop signature in PR #797 body.
NO_SLOP_PUBLICATION_NONCE=00112233445566778899aabbccddeeff
Found compliant no-slop pipeline attestation.

CASE: historical marker + current completed attestation
RESULT: accepted
Found no-slop signature in PR #797 body.
NO_SLOP_PUBLICATION_NONCE=00112233445566778899aabbccddeeff
Found compliant no-slop pipeline attestation.

CASE: historical signature without attestation
RESULT: rejected
Found no-slop signature in PR #797 body.
::error::This PR has no valid no-slop publication identity. Re-run 'git push no-slop'.

CASE: attestation bound to stale PR head
RESULT: rejected
Found no-slop signature in PR #797 body.
::error::This PR must contain one unambiguous owned no-slop v1 pipeline attestation. Re-run 'git push no-slop'.

CASE: required review not completed
RESULT: rejected
Found no-slop signature in PR #797 body.
::error::This PR must contain one unambiguous owned no-slop v1 pipeline attestation. Re-run 'git push no-slop'.

CASE: required test not completed
RESULT: rejected
Found no-slop signature in PR #797 body.
::error::This PR must contain one unambiguous owned no-slop v1 pipeline attestation. Re-run 'git push no-slop'.

CASE: required document not completed
RESULT: rejected
Found no-slop signature in PR #797 body.
::error::This PR must contain one unambiguous owned no-slop v1 pipeline attestation. Re-run 'git push no-slop'.

Evidence: Operator-visible AXI parked review gate

Command: go test -overlay=/private/tmp/ns-port-home.uuWWgE/evidence/01M0RWRC474AQ4315SBMJ8D4H7/axi-evidence-overlay.json -tags e2e ./internal/e2e -run '^TestAxiYesBudgetParksUnresolvedFindings$' -count=1 -v

operator-visible axi output after unresolved findings survive the fix budget:
run: running
  intent: completed
  rebase: running
  rebase: completed
  review: running
  review: awaiting_approval
  review: fixing
  review: fix_review
  review: fixing
  review: fix_review
review gate parked by the convergence guard: review loop is not converging: findings per round have not decreased across the last 3 rounds (1,1,1); finding class "deref-nil-potential" recurred in rounds 1, 2, 3; leaving the run parked for explicit adjudication
run:
  branch: feature/yes-budget
  status: running
  awaiting_agent: parked 0s
  findings: 1 awaiting
  steps[9]{step,status,findings,duration_ms}:
    intent,completed,0,0
    rebase,completed,0,859
    review,fix_review,1,844
    test,pending,0,0
    document,pending,0,0
    lint,pending,0,0
    push,pending,0,0
    pr,pending,0,0
    ci,pending,0,0
gate:
  step: review
  status: fix_review
  summary: 1 outstanding finding
  risk: medium
  convergence:
    rounds: "1,1,1"
    recurring[1]: "deref-nil-potential (rounds 1,2,3) in feature.txt"
  findings[1]{id,severity,file,action,description}:
    review-79b7064b34ec1aded057a0407f748d7b,warning,feature.txt,ask-user,potential nil deref

--- PASS: TestAxiYesBudgetParksUnresolvedFindings

Pipeline

Updates from git push no-slop

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed (17) ✅

• 🚨 internal/pipeline/executor.go:322 - [rule-applied-in-one-place-not-sibling] The required criterion says to bind the attestation to the current PR head and require review, test, and document completion, but this invalidation helper has no production caller. A normal Document fix can advance HEAD from H1 to H2 after Review/Test certify H1; PR generation then emits top-level/Document H2 with Review/Test H1, which the new workflow rejects. Wire invalidation and execution-state restoration into both fresh and recovered executor loops after any step advances HEAD.
• 🚨 internal/pipeline/steps/ci_checks.go:18 - [rule-applied-in-one-place-not-sibling] Stale-attestation filtering is permanently disabled in production because expectedAttestationHeadSHA is only restored, never initially assigned or persisted; the test manually seeds it. Given a failed synchronize check and a newer compliant edited check on the same head, this guard returns both unchanged, so generic CI handling treats the stale failure as current and can launch another repair or park the run. Arm the state when a CI repair head is durably recorded and persist it across validation restart.
• ⚠️ internal/pipeline/steps/ci_autofix_test.go:107 - [vacuous-check] This test never sets Run.PRURL, so CIStep.Execute exits through the no-PR skip path. Its assertions accept that skipped outcome and therefore do not prove stale required gates avoid generic CI repair. Provide a PR/check fixture that reaches the claimed path and assert the actual rerun behavior.

🔧 Fix: Rerun stale gates and persist CI attestations
3 issues (2 errors, 1 warning) still open:

• 🚨 internal/db/step.go:299 - [rule-applied-in-one-place-not-sibling] The required criterion says “Keep non-review step semantics unchanged,” but this reset preserves findings_json for every step from Review onward. After Test/Document records H1 findings and a CI repair advances to H2, those steps become pending while still exposing H1 findings; if revalidation fails before producing a new outcome, the stale findings remain on the failed run. Preserve Review’s carried truth selectively and clear findings for non-review rows.
• 🚨 internal/pipeline/steps/ci_fix.go:137 - [rule-applied-in-one-place-not-sibling] Stale-attestation filtering is armed only after a CI repair. For an existing PR, an ordinary new run can push H2 while its body still attests H1, producing a failing synchronize check; PRStep then publishes an H2 attestation and produces a newer passing edited check. Because no CI repair armed this state, both checks reach generic CI handling and the stale failure can trigger an unnecessary repair or park. Arm the expected head at the shared PR-attestation publication boundary for every existing-PR update.
• ⚠️ internal/pipeline/steps/ci_fix.go:153 - [scope-expansion] refreshPRAttestation has no production caller; only tests use it, while its supporting SCM interface/backend method broaden the implementation and the docs claim an intermediate CI attestation refresh that never occurs. Remove this dead path and correct the documentation, since the live flow restarts validation and lets PRStep publish the compliant body.

🔧 Fix: Preserve review truth and arm PR attestations
3 errors still open:

• 🚨 internal/types/findings.go:220 - [rule-applied-in-one-place-not-sibling] The criteria require that “unresolved review findings must survive silent or narrower later rounds” with stable identity, but any reviewer-supplied prior ID/token pair is accepted without semantic corroboration. A later reviewer can mistakenly assign A’s visible lineage to unrelated B while omitting A; normalization accepts the claim and the carry merge replaces A’s file, description, and evidence with B without error. Preserve both findings when the claimed continuation cannot be independently corroborated.
• 🚨 internal/pipeline/steps/ci_checks.go:62 - [rule-applied-in-one-place-not-sibling] The stale-attestation filter has no recorded boundary identifying the PR update it armed. Before a passing edited run appears, it discards every terminal failure—including a newer current failure—and emits synthetic pending until timeout. Conversely, an older same-head pass can make a stale failure appear current and send it to generic CI repair. Persist the pre-update attempt cutoff and suppress only attempts at or below it.
• 🚨 internal/pipeline/steps/round_history.go:231 - [rule-applied-in-one-place-not-sibling] Later-selection reconciliation keys only on raw finding ID, bypassing the new ID-plus-token identity rule. Recovered legacy rounds can reuse review-1 for unrelated findings; selecting the later finding then removes the earlier operator-ignored finding from history, allowing it to be re-reported. Match later selections using corroborated lineage, with exact/unique structural fallback for legacy rows.

🔧 Fix: Corroborate finding lineage and bound stale CI attempts
6 issues (5 errors, 1 warning) still open:

• 🚨 internal/types/findings.go:254 - [rule-applied-in-one-place-not-sibling] The intent requires unresolved findings and their stable identity to survive ambiguous rereviews, but an unrelated finding at the same nonzero file/line is treated as semantic corroboration. For example, a new “SQL injection” at loader.go:42 can claim the token of an unresolved “nil dereference” at that line and replace its description/evidence. Require description-level corroboration in addition to location; otherwise preserve both lineages.
• 🚨 internal/pipeline/steps/round_history.go:74 - [rule-applied-in-one-place-not-sibling] Current-run history reconciles later selections, but uncertified prior-run history still calls renderRoundHistoryEntry, which supplies no later-selection context. Thus a finding ignored in prior round 1 and selected in prior round 2 is still rendered as ignored during recovery, contradicting the required recovery/later-selection semantics. Apply the same reconciliation across UncertifiedPriorRounds.
• 🚨 internal/pipeline/steps/round_history.go:302 - [rule-applied-in-one-place-not-sibling] The exact legacy structural match bypasses the uniqueness checks used by the fingerprint fallback. If two earlier legacy findings have identical file/line/description and one matching later finding is selected, both earlier ignored entries are suppressed. The intent requires exact unique structural fallback and preservation of ambiguous history; require uniqueness on both sides before accepting this match.
• 🚨 internal/pipeline/steps/ci_transient.go:535 - The recorded “pre-update cutoff” is only a non-atomic snapshot taken before UpdatePR. A synchronize run can be queued but absent from GetChecks; after the snapshot it appears as run K+1 and fails before the edited run K+2 registers. Because K+1 is above the stored cutoff, CI treats the stale-body failure as current and can repair or park unnecessarily. Establish the boundary from an authoritative provider sequencing point that cannot race registration of the pre-update event.
• 🚨 internal/pipeline/steps/ci_transient.go:503 - [fail-open-default] Expected-attestation recovery shares loadRerunBudget, which logs and continues when the persisted row cannot be read or decoded. On the first CI execution or daemon recovery, that leaves the new cutoff fields empty, disables stale-attempt filtering, and lets an old required-check failure reach generic repair logic. Return the restoration error now that this state protects correctness rather than only a rerun allowance.
• ⚠️ internal/pipeline/findings.go:218 - [rule-applied-in-one-place-not-sibling] The review contract excludes pipeline-owned-delivery findings from risk, but the effective-risk merger takes the carried top-level rank and every finding severity before considering scope. A carried pipeline-delivery error therefore raises the operator-visible risk to high even when source/external risk is low. Compute the rank only from source/external findings and qualifying top-level assessments.

🔧 Fix: Harden review lineage, recovery, risk, and CI boundaries
4 errors still open:

• 🚨 internal/pipeline/findings.go:144 - [rule-applied-in-one-place-not-sibling] Exact legacy structural matching bypasses uniqueness checks. With two identical unresolved legacy carried findings and one matching fresh finding, both old entries select the same merged item and collapse into one. This contradicts the requirement that ambiguous unresolved findings survive; require uniqueness on both sides before any legacy fallback merges lineages.
• 🚨 internal/pipeline/steps/pr.go:96 - The attestation boundary remains non-atomic: UpdatePR completes, then a separate mutable updatedAt read establishes the cutoff. A label, assignee, or other unsubscribed PR mutation between those requests advances the boundary beyond the valid edited workflow event, causing that passing attempt to be suppressed until CI times out. Return an operation-specific identity atomically from the PR update or embed a publication nonce.
• 🚨 internal/pipeline/steps/ci_transient.go:504 - [rule-applied-in-one-place-not-sibling] Expected-attestation restoration now shares a fail-closed path with the pre-existing transient-rerun budget. Consequently, any DB read or legacy budget decode failure aborts CI instead of retaining the prior best-effort rerun behavior, contradicting “Keep non-review step semantics unchanged.” Separate correctness-critical attestation state from the legacy rerun allowance so only the former fails closed.
• 🚨 internal/pipeline/executor.go:337 - [rule-applied-in-one-place-not-sibling] Recovery restores carried findings only from the current run’s step row. If a prior run finds A/B, fixes selected A, then dies before rereview, its replacement run receives prior rounds only as prompt metadata and starts with no carried gate truth; a silent review can certify the head and clear the uncertified range while unresolved B disappears. This contradicts the required recovery semantics; restore the prior run’s durable effective findings at the uncertified-range boundary.

🔧 Fix: Harden finding recovery and CI attestation publication
4 errors still open:

• 🚨 internal/pipeline/uncertified.go:65 - [fail-open-default] Intent requires unresolved findings to survive recovery, but a failed range write is only logged after the fixer commit and branch head are adopted. If the daemon exits before rereview, the next run has no source-run mapping and a silent review can erase the unresolved gate truth. Make range persistence mandatory before exposing the repaired branch head, or roll back/refuse adoption when it fails.
• 🚨 internal/pipeline/uncertified.go:254 - [fail-open-default] Optional round-history loading and correctness-critical findings restoration share one failure path. If GetRoundsByStep fails, the already-readable step.FindingsJSON is discarded; a silent replacement review can then complete and clear the uncertified range. Restore effective findings independently and fail closed when that gate truth cannot be read.
• 🚨 internal/pipeline/steps/ci_checks.go:57 - The PATCH response timestamp is atomic but not operation-unique. Two edits within GitHub's timestamp precision can share T: an earlier stale-body edited check at T is accepted as current after no-slop's PATCH also returns T, so its failure can trigger repair or parking before the compliant event registers. Persist a publication nonce or immutable event/run identity instead of timestamp equality.
• 🚨 .github/workflows/no-slop-required.yml:87 - [rule-applied-in-one-place-not-sibling] Intent allows the historical marker when a valid current v1 attestation is present, but validation examines only the first attestation comment. User intent is rendered before the generated Pipeline section and can legitimately quote an old or malformed comment; that comment masks the valid generated attestation later in the body and rejects the PR. Scope parsing to the owned Pipeline attestation or handle all comments unambiguously.

🔧 Fix: Harden recovery truth and attestation publication
7 issues (6 errors, 1 warning) still open:

• 🚨 internal/pipeline/executor.go:1114 - [rule-applied-in-one-place-not-sibling] The intent requires preserving the stricter action and effective risk until verification, but selecting finding A removes it from carriedFindings. If rereview corroborates A's ID/token while weakening it to no-op/low risk, normalization restores only identity, so no old A remains for the carry merge to restore stricter semantics. Preserve prior semantics when a selected lineage reappears; absence alone should clear it.
• 🚨 internal/pipeline/uncertified.go:261 - [rule-applied-in-one-place-not-sibling] Recovery still disagrees with the live selected-finding path. If A/B are persisted, A is selected and fixed, and the daemon dies before rereview, the step row still contains A/B; this loader restores both without applying the durable selection. A silent replacement rereview therefore cannot clear fixed A, although the uninterrupted path carries only B. The regression test avoids this sequence by manually inserting a second round containing only B. Reconstruct the post-selection effective carry from the durable round decision.
• 🚨 internal/pipeline/uncertified.go:152 - [fail-open-default] The durable recovery invariant remains optional on the rebase sibling path. A replacement run can rewrite an uncertified fixer head, adopt the rebased branch at rebase.go:518, then have this remap read, mapping, or write fail and merely log. Review subsequently sees the old tip outside the new lineage and skips prior findings/provenance, allowing a silent review to erase unresolved truth. Make remapping mandatory before adopting the rewritten head, or refuse/roll back adoption.
• 🚨 .github/workflows/no-slop-required.yml:95 - [rule-applied-in-one-place-not-sibling] The required owned-attestation parser can still be masked by generated Pipeline details. Finding descriptions are rendered after the real attestation and preserve newlines; a legitimate description containing \n## Pipeline\n\n becomes the last heading, so rfind parses it instead and rejects an otherwise valid generated body. Locate a unique structural tuple of heading, owned marker, and immediately following v1 comment instead of assuming the last heading is owned.
• 🚨 internal/pipeline/steps/ci_checks.go:44 - [fail-open-default] A nonce mismatch proves that two bodies differ, not that the check is older. After no-slop publishes nonce N and its check passes, a later user edit with nonce O can correctly fail; this filter discards that newer authoritative failure and retains the older N pass, allowing CI to report green while the current body is invalid. The same rule hides a current attempt cancelled before it prints a nonce. Order attempts using immutable event/run identity and suppress only attempts proven older than N's publication event.
• 🚨 internal/scm/github/github.go:429 - [scope-expansion] Nonce filtering no longer uses the timestamp/action parsed from the workflow display title, yet every required-check attempt must still match the new title format here. Existing same-head attempts created before this change use the former title, so encountering one aborts CI before its missing nonce can classify it as stale. Remove the obsolete title dependency or tolerate legacy titles and require only identity used by filtering.
• ⚠️ docs/src/content/docs/reference/pipeline-steps.md:227 - The attestation reference now documents a payload that the required workflow rejects because it omits publication_nonce, and the sequencing paragraph still claims timestamp-based persistence/filtering. Update this contract owner to describe the nonce and current attempt-ordering behavior.

🔧 Fix: Preserve review semantics and order attestation attempts
4 issues (3 errors, 1 warning) still open:

• 🚨 internal/pipeline/findings.go:230 - [rule-applied-in-one-place-not-sibling] Intent requires preserving “stable finding identity [and] the stricter action,” but structural fallback is disabled when both findings have generated lineages. If selected A/ask-user is removed from carry and the rereviewer reports the same unique file/description as no-op without prior_id/token, normalization assigns lineage C; this merge cannot match C to A, so A’s stricter semantics disappear. Apply one-to-one structural corroboration for selected prior lineages and preserve both when ambiguous.
• 🚨 internal/pipeline/steps/ci_checks.go:56 - [rule-applied-in-one-place-not-sibling] The publication run boundary is learned only from terminal workflow logs containing the nonce. If publication run N is cancelled before printing it, its nonce is empty—or log retrieval errors—and PublicationRunID remains zero. The filter then hides N and any later authoritative failure behind synthetic pending until timeout. Bind the provider event/run identity independently of job output so cancelled and later attempts remain visible.
• 🚨 internal/pipeline/steps/common_fix.go:174 - [rule-applied-in-one-place-not-sibling] Intent requires unresolved review findings to preserve recovery semantics, but uncertified provenance is persisted only for Review fixes. Concrete path: Review leaves B unresolved at H1; Document/Test/Lint or CI commits H2; the branch and run adopt H2; the daemon exits before validation restarts. A replacement run has no source-run mapping, so a silent review can erase B. Persist review gate truth before every post-review head adoption, while leaving non-review execution semantics unchanged.
• ⚠️ internal/scm/github/github.go:270 - [rule-applied-in-one-place-not-sibling] AGENTS.md requires GitHub UpdatePR to use the shared prSelector exact-PR boundary, but this path introduces prAPIIdentity as a competing selector owner. Derive the API repository/number after invoking prSelector so number/URL/fail-closed targeting remains owned by one boundary.

🔧 Fix: Preserve post-review truth and bind attestation runs
4 issues (3 errors, 1 warning) still open:

• 🚨 .github/workflows/no-slop-required.yml:2 - [test-capitulation] This replaces the documented consumer-facing display_title identity (PR, action, run_number, and run_id) with the PR body alone, breaking the explicit Wheelhouse integration contract in CONTRIBUTING.md:13. The changed test at workflow_no_slop_required_test.go:238 simultaneously removes those assertions and accepts only the nonce prefix. Preserve the existing identity fields alongside the leading publication marker, or explicitly approve and document the breaking contract change.
• 🚨 .github/workflows/no-slop-required.yml:110 - [rule-applied-in-one-place-not-sibling] The intent requires the historical marker to work when a valid current v1 attestation exists, but any quoted syntactically valid tuple in ## Intent is also appended to candidates. For example, a quoted owned marker plus {"steps":[]} followed by the real generated current attestation produces two candidates and fails at line 114. Validate candidate semantics before deciding ambiguity, or identify the generated owned section without allowing quoted user content to mask it.
• 🚨 internal/pipeline/executor.go:1000 - [rule-applied-in-one-place-not-sibling] The required stricter lineage semantics are applied only after ReviewStep has stripped pipeline-owned-delivery findings. If a selected source finding reappears with the correct prior ID/token but is mistakenly reclassified as pipeline-owned-delivery, review.go:284 removes it before this merge sees it; its prior action, severity, evidence, and source risk then disappear despite the reviewer reporting the lineage. Reconcile known prior lineages before phase filtering, retaining the prior source semantics for corroborated continuations.
• ⚠️ docs/src/content/docs/reference/pipeline-steps.md:87 - [rule-applied-in-one-place-not-sibling] This contract still says lint and document fixer commits do not persist uncertified ranges, while common_fix.go:194-200 now does so for Test, Document, Lint, and CI. Update the documentation to describe persistence before every post-review head adoption and its fail-closed behavior.

🔧 Fix: Restore attestation identity and review lineage ordering
7 errors still open:

• 🚨 internal/types/findings.go:241 - [rule-applied-in-one-place-not-sibling] The required “preserve both on ambiguity” behavior still fails for selected findings. If fresh finding B claims selected finding A’s prior ID/token but has a non-corroborating description, normalization assigns B a new lineage and clears the rejected claim. Reconciliation can no longer recognize the ambiguity, and because A was removed from carried findings when selected, A disappears. Retain rejected-claim provenance so reconciliation preserves both A and B.
• 🚨 internal/pipeline/executor.go:1251 - [rule-applied-in-one-place-not-sibling] User-added findings receive user-N IDs but no continuity token and are never added to KnownReviewLineages. If an added finding remains after its fix, rereview creates a new agent lineage, losing its stable identity, source:user, instructions, and potentially stricter action; recovery likewise cannot restore it from effective review truth. Normalize and durably register additions before rereview.
• 🚨 .github/workflows/no-slop-required.yml:2 - [rule-applied-in-one-place-not-sibling] The workflow validates only the anchored leading publication marker, but the provider reader scans the entire display title and requires exactly one marker. A valid body whose Intent quotes an older publication marker passes the workflow, yet its run title contains both markers and parsePublicationNonce aborts CI identity resolution. Encode a unique bounded metadata prefix or parse only that owned prefix.
• 🚨 internal/pipeline/steps/ci_checks.go:29 - [rule-applied-in-one-place-not-sibling] Publication ordering is still learned only from attempts returned by GetChecks. If the publication workflow is cancelled or action-required before creating a job check, then a later visible authoritative failure cannot establish PublicationRunID; this branch removes that real failure and substitutes synthetic pending until timeout. Resolve the immutable workflow event/run independently of job-check visibility.
• 🚨 internal/pipeline/findings.go:351 - [fail-open-default] The required effective-risk preservation fails for a schema-valid inconsistent payload containing a retained source error, a dropped delivery finding, and top-level risk_scope: pipeline-owned-delivery. Filtering drops the delivery item and unconditionally rewrites risk to low despite the remaining source error. Recompute source/external risk from retained findings instead of trusting the conflicting top-level scope.
• 🚨 internal/pipeline/uncertified.go:35 - [fail-open-default] Recovery ancestry checks collapse every git failure—including timeout or unreadable-object errors—into “not an ancestor,” then continue review without prior findings. A transient merge-base failure can therefore let a silent replacement review erase unresolved truth; the same boolean also permits range extension to overwrite an existing range after an indeterminate probe. Return and fail closed on probe errors, distinguishing only a proven non-ancestor result.
• 🚨 internal/pipeline/steps/common_fix.go:179 - [rule-applied-in-one-place-not-sibling] Rebase remapping rolls back its range write when branch adoption fails, but ordinary Review/Test/Document/Lint and CI fixes do not. With an existing H0..H1 unresolved range, persisting H0..H2 and then losing the branch-ref compare-and-swap overwrites the recoverable H1 mapping with an unadopted H2; the next run on a concurrent H1 descendant skips it and loses prior truth. Roll back the persisted range on adoption failure in both fix paths.

🔧 Fix: Harden review lineage recovery and CI publication ordering
2 errors still open:

• 🚨 internal/pipeline/findings.go:314 - [rule-applied-in-one-place-not-sibling] Intent requires preserving stricter evidence and effective operator-visible gate truth, but subset reconciliation imports aggregate evidence and risk from the entire prior set whenever any lineage matches. If prior A is a warning, B is an error with B-only reproduction evidence, and rereview returns only A, B is cleared but its evidence and high risk remain. Carry only semantics attributable to surviving lineages or recompute aggregates after cleared lineages are removed.
• 🚨 internal/db/stats.go:151 - [rule-applied-in-one-place-not-sibling] Intent requires statistics and operator-visible gate truth to agree, but statistics read only FindingsJSON. A normalized user-added finding stored in UserFindingsJSON can be selected, fixed, and absent on rereview without ever entering FindingsJSON, so reported/fixed counts omit it while round history records it. Fold uniquely lineaged user-added findings into reported statistics.

🔧 Fix: Align reconciled evidence and user finding statistics
3 errors still open:

• 🚨 internal/pipeline/executor.go:1015 - [rule-applied-in-one-place-not-sibling] The required “Preserve ... evidence” invariant still fails in the composed executor path. With prior A/B evidence, selecting A leaves B in carriedFindings, but excludeFindingsJSON retains the aggregate A+B tested/artifact/summary fields. If rereview clears A and returns B, subset reconciliation drops A’s evidence, then this merge immediately restores the carried A+B aggregates. Conversely, selecting all findings leaves no carry and drops evidence attributable to a surviving lineage whenever another lineage clears. Evidence needs durable lineage attribution at the selection/carry boundary so only surviving evidence is merged.
• 🚨 internal/scm/github/github.go:469 - The documented earliest-publication boundary fails when more than one legitimate workflow event carries the same unchanged body nonce. After no-slop’s edited run N, a later title edit or reopen produces run N+1 with the same head and nonce; this branch reports ambiguity instead of selecting the earliest matching run, so CI aborts despite both events being orderable. Select the minimum matching immutable run ID and retain later runs as authoritative attempts.
• 🚨 internal/pipeline/steps/ci_transient.go:518 - [fail-open-default] Legacy attestation recovery still ignores a read failure. If ci_attestation_state is empty for a timestamp-bound legacy run and reading its shared ci_rerun_state fails, this clears the expected boundary and lets generic CI processing consume stale required-check failures. Migrate/read the legacy attestation through a correctness-critical boundary that fails closed while keeping a proven rerun-only allowance best-effort.

🔧 Fix: Preserve lineage evidence and harden CI publication recovery
3 errors still open:

• 🚨 internal/db/stats.go:188 - [test-capitulation] The intent requires stable identity and correct statistics, but tokenless id_generated IDs are treated as durable lineage. The added test at internal/db/stats_test.go:257 confirms the bug: two unrelated findings reuse one tokenless ID and are counted as 1 reported/0 fixed instead of 2 reported/1 fixed. Require ID plus continuity token for lineage; otherwise use unique structural legacy matching.
• 🚨 internal/db/stats.go:158 - [rule-applied-in-one-place-not-sibling] Statistics collapse identical legacy findings through a boolean structural map even though carry/history preserve ambiguous findings separately. Two identical outstanding legacy entries produce 1 reported finding while currentCount is 2. Preserve multiplicity unless a one-to-one continuation proves identity.
• 🚨 internal/pipeline/executor.go:1303 - [fail-open-default] Review certification commits before uncertified-range deletion, which only logs failures. A crash or delete failure between these operations leaves an approved finding mapped as uncertified; the next run restores and parks on gate truth the operator already approved. Atomically delete the matching range within the shared CompleteReviewStep transaction for live and recovered completion paths.

🔧 Fix: Fix finding statistics and atomic review certification
5 errors still open:

• 🚨 internal/types/findings.go:106 - [rule-applied-in-one-place-not-sibling] Exact legacy identity matches bypass the required one-to-one uniqueness rule. With two identical tokenless carried findings and one identical latest-round finding, retainMatchingFindingsJSON matches both carried occurrences to that single candidate, so automatic review fixing can select both even though continuation is ambiguous. Apply the same both-sides uniqueness requirement used by the fingerprint fallback.
• 🚨 internal/pipeline/findings.go:157 - [rule-applied-in-one-place-not-sibling] Ambiguous legacy findings multiply during composed reconciliation. If prior A/B are identical and another finding is selected, a rereview reporting one identical C makes mergeReappearedFindingsJSON preserve C+A+B; this merge then cannot uniquely match carried A/B and appends both again, yielding five findings. Preserve each ambiguous lineage exactly once across the reappeared-and-carried boundary.
• 🚨 internal/pipeline/steps/common_fix.go:175 - [rule-applied-in-one-place-not-sibling] Recovery truth is persisted only after a fix advances HEAD. If findings are selected and the fixer makes no commit—or the daemon crashes before committing—no uncertified range is created; the fixing run is failed and a replacement run at the unchanged head receives no prior findings, allowing a silent review to erase them. Persist unresolved gate truth at the durable selection boundary independently of head movement.
• 🚨 internal/pipeline/findings.go:111 - [rule-applied-in-one-place-not-sibling] Multi-finding legacy review payloads still lose aggregate evidence when one finding is selected. If A/B share top-level tested, summary, or artifact evidence, excluding selected A cannot rebuild per-item attribution and clears every aggregate field, leaving unresolved B without its durable evidence. Attribute or preserve shared legacy evidence at the selection boundary instead of deleting it.
• 🚨 internal/db/stats.go:246 - [rule-applied-in-one-place-not-sibling] Legacy statistics reuse cleared ambiguous occurrences without proving continuation. Two identical tokenless findings in round 1, an empty round 2, and two newly reported identical findings in round 3 are paired with the historical occurrences here, producing 2 reported/0 fixed instead of 4 reported/2 fixed. Track active occurrences between rounds and continue legacy identity only through a unique one-to-one match.

🔧 Fix: Harden finding occurrence recovery and statistics
2 errors still open:

• 🚨 internal/pipeline/steps/round_history.go:299 - [rule-applied-in-one-place-not-sibling] Intent requires fixing the P1 where older history still calls a later-selected finding ignored and preserving “later selection semantics.” Identical legacy A/B receive distinct occurrence tokens at selection persistence, but if round 2 selects A, this reconciliation ignores those tokens and structural ambiguity leaves round 1 rendering A under user_chose_to_ignore. Match unique occurrence tokens before structural fallback.
• 🚨 internal/pipeline/findings.go:139 - Intent requires preserving evidence, but this copies the complete aggregate evidence union onto every evidence-less legacy occurrence. If A has A-only evidence and B has none, selecting and clearing A leaves A’s evidence falsely attributed to B. Attribute only aggregate evidence not already owned by another item, or persist it through an explicit shared evidence owner.

🔧 Fix: Preserve shared evidence and occurrence history
4 errors still open:

• 🚨 internal/pipeline/findings.go:648 - [rule-applied-in-one-place-not-sibling] The required “Preserve … evidence” invariant remains violated. subtractEvidenceSummary can only subtract exact blank-line-delimited blocks; if A owns “A reproduced” while the aggregate says “A and B reproduced,” it treats the owned block as unmatched and returns an empty shared summary. Selecting A then permanently removes evidence needed by unresolved B. Persist explicit evidence ownership when evidence is created, or conservatively retain ambiguous aggregate evidence under a durable shared owner.
• 🚨 internal/pipeline/steps/pr.go:120 - [rule-applied-in-one-place-not-sibling] Existing-PR updates persist the expected attestation publication boundary, but successful GitHub PR creation does not. If the same head already has a terminal required-check failure from another PR, CI can consume that stale failure before the new PR’s compliant opened-event run registers, causing an unnecessary repair or parked run. Persist the publication nonce after successful GitHub creation at the same boundary used for updates.
• 🚨 internal/pipeline/uncertified.go:45 - [rule-applied-in-one-place-not-sibling] The required survival of later-selected unresolved findings still fails during recovery. When an existing range spans H0..H1, persisting a selection before the fixer runs retains H0 as FromSHA; if the daemon dies before a fixer commit, recovery infers preserveSelected=false from H0 != H1 and excludes the selected finding as though it had been fixed. A silent rereview can then erase it. Persist explicit selection/fixer-progress state instead of inferring completion from range width.
• 🚨 internal/pipeline/steps/round_history.go:301 - [rule-applied-in-one-place-not-sibling] The required later-selection semantics remain incorrect when the same occurrence is selected in multiple later rounds. Its durable occurrence token appears more than once in selectedLater, so the aggregate uniqueness check rejects the token match; structurally identical legacy findings then make fallback ambiguous, and the original occurrence is rendered under user_chose_to_ignore despite its later selection. Treat an exact durable occurrence token as identity across rounds, or apply uniqueness per round rather than across repeated selections.

🔧 Fix: Harden evidence, publication, recovery, and history state
2 errors still open:

• 🚨 internal/pipeline/steps/ci_checks.go:79 - The attestation filter orders workflow events by immutable RunID, while the documented provider contract defines increasing run_number as the order of distinct events. For example, a passing publication event with RunID 1002/RunNumber 102 remains authoritative while a later failing edit with RunID 1001/RunNumber 103 is discarded here, producing a false green result. Persist and compare the publication RunNumber, and select the earliest publication by RunNumber at the shared ordering boundary.
• 🚨 internal/db/stats.go:187 - [rule-applied-in-one-place-not-sibling] The intent requires “Keep non-review step semantics unchanged,” but non-review statistics now identify findings only by file, line, and description. A Test finding changing from warning/source to error/external at the same location is counted as 1 reported/0 fixed instead of the previous 2 reported/1 fixed. Preserve the legacy statistics key for non-review steps and restrict the new lineage semantics to Review.

🔧 Fix: Order attestation events and restore legacy stats
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

• git log --oneline --decorate --no-renames 9f9296ea331874b949ec9cbae0fbf7df9cdd99be..623d4bd311a3fa62c537493d6fd96037b59c93d0 and targeted diff inspection
• go test . -run 'TestNoSlopRequiredWorkflow(ChecksSignatureMarker|AcceptsHistoricalLegacyMarker|EnforcesCompletedPipelineAttestation)$' -count=1
• go test ./internal/pipeline -run 'TestExecutor_(UnselectedReviewFindingSurvivesSilentRereview|ReviewUserFixPersistsRecoveryTruthBeforeFixer|LaterSelectedCarriedFindingClearsAfterVerification|CarriedFindingKeepsIdentityAndStricterAction|HeadMutationsInvalidateRequiredGateCertifications|RecoveredRemainderRerunsRequiredGatesAfterHeadMutation)|TestLoadUncertifiedPriorReview(PreservesSelectedTruthBeforeFixAdoption|KeepsEffectiveFindingsWhenRoundsFail)$' -count=1
• go test ./internal/db -run 'Test(InsertEffectiveReviewStepRoundRollsBackWhenFindingsUpdateFails|PersistReviewFixSelectionRollsBackWhenRecoveryMarkerFails|StepFindingStatsDoesNotCountSelectedFindingsAsFixed|StepFindingStatsPreservesLegacyIdentityForNonReviewSteps|StepFindingStatsCountsNonReviewReclassificationAsNewFinding)$' -count=1
• go test ./internal/pipeline/steps -run 'TestRoundHistoryPromptSection_(LaterSelectionSupersedesEarlierIgnore|RepeatedOccurrenceSelectionsRemainAuthoritative)|TestPRStep_FailsWhenExistingPRAttestationCannotBePublished$|TestCIStep(FailsClosedWhenAttestationStateCannotBeRestored|RejectsLegacyTimestampAttestationState)$' -count=1
• go test ./internal/types -run 'TestNormalizeFindings(PersistsGeneratedIDProvenance|KeepsNonReviewIdentitySemantics|RequiresCorroboratedPriorLineageClaim|PreservesRewordingAtSameLocation)$' -count=1
• go test ./internal/pipeline -run 'TestExecutor_ResumeCarriesUnselectedReviewFinding$' -count=1
• go test ./internal/pipeline/steps -run 'TestBuildPipelineSummary_EmitsStructuredStepAttestation$' -count=1
• go test -tags e2e ./internal/e2e -run '^TestAxiYesBudgetParksUnresolvedFindings$' -count=1 -v
• go run /private/tmp/ns-port-home.uuWWgE/evidence/01M0RWRC474AQ4315SBMJ8D4H7/workflow_acceptance_demo.go
• go test -overlay=/private/tmp/ns-port-home.uuWWgE/evidence/01M0RWRC474AQ4315SBMJ8D4H7/axi-evidence-overlay.json -tags e2e ./internal/e2e -run '^TestAxiYesBudgetParksUnresolvedFindings$' -count=1 -v
• git merge-base --is-ancestor 7f25b51 623d4bd311a3fa62c537493d6fd96037b59c93d0 && git merge-base --is-ancestor d729ed5 623d4bd311a3fa62c537493d6fd96037b59c93d0
• git status --short confirmed testing left the worktree clean

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.