fix(#2484): add finding reconciliation step to review agent

[fullsend-ai-coder]

The review agent could emit contradictory findings within the same review — e.g., a High protected-path finding asserting "no authorization exists" alongside an Info implicit-authorization finding citing specific config that authorizes the change. This happened because orchestrator findings (step 6e) were added after the challenger pass without checking whether existing sub-agent findings negated their premise.

Add step 6e-1 (finding reconciliation) between the orchestrator checks (6e) and outcome determination (6f). When a sub-agent finding cites specific evidence (e.g., renovate.json, dependabot.yml) that directly authorizes changes flagged by an orchestrator finding, the orchestrator finding is downgraded to info severity. The finding is not suppressed — human approval is still required for protected paths — but the contradiction is resolved and the verdict is no longer escalated to request-changes based on a negated premise.

---

Closes #2484

Post-script verification

• Branch is not main/master (agent/2484-reconcile-contradictory-findings)
• Secret scan passed (gitleaks — 22500115e0f3a0f55972e2c31be79a61d94942e1..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[github-actions]

Site preview

Preview: https://cf3a5bd9-site.fullsend-ai.workers.dev

Commit: 8c00c2fe8fe02c391650b3e279d2c198b3c8ab33

[fullsend-ai-review]

🤖 Finished Review · ❌ Failure · Started 8:55 AM UTC · Completed 9:10 AM UTC
Commit: 8c00c2f · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

High

• [logic-error] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:688 — Step 6e-1 creates a contradiction with the Constraints section (line 809): "Never approve when any protected-path finding exists, regardless of severity." Step 6e-1 downgrades a protected-path finding to info severity. Step 6f maps info-only findings to approve. If the only blocking finding was a protected-path at high/medium and it gets reconciled to info, 6f would produce an approve outcome — but the constraint says protected-path findings block approval "regardless of severity." The post-review.sh safety net independently prevents actual approval on protected-path PRs, so the real-world impact is mitigated, but the document is internally inconsistent and an LLM following it must choose between conflicting instructions.
  Remediation: Either (a) update the constraint at line 809 to say "Never approve when any protected-path finding exists at medium or higher severity" (acknowledging reconciled info-level findings are exempt), or (b) add a sentence to step 6e-1 stating that a reconciled protected-path finding at info severity still triggers the never-approve-on-protected-path constraint.

Medium

• [logic-error] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:700 — The "What reconciliation does NOT do" section claims the finding "remains as an info-level notice" as if the outcome is unchanged, but downgrading from high/medium to info materially changes the verdict per step 6f. The next bullet correctly notes post-review.sh prevents approval independently, but the first bullet understates the impact of the severity change.
  Remediation: Rewrite to acknowledge the outcome impact: "The finding remains at info severity, which alone would map to an approve verdict under step 6f. However, the constraint at line 809 ('never approve when any protected-path finding exists') takes precedence."

• [edge-case] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:680 — The reconciliation algorithm triggers a downgrade when "a sub-agent finding at any severity cites specific evidence" of authorization, but does not handle the case where multiple sub-agent findings provide conflicting evidence about the same protected path (e.g., one says authorization exists via renovate.json, another disputes coverage). A single positive-evidence finding triggers the downgrade even if contradicted by other findings.
  Remediation: Add a clause: "If multiple sub-agent findings address the same protected path and their evidence conflicts (one claims authorization, another disputes it), do not downgrade — the orchestrator finding stands unchanged. Only downgrade when the authorization evidence is uncontradicted."

• [missing-doc] skills/pr-review/SKILL.md — The live skill file at skills/pr-review/SKILL.md is byte-identical to the scaffold template at internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md. This PR only updates the scaffold template, so the two files will diverge after merge. The live file is what this repo's review agent actually executes.
  Remediation: Apply the same changes to skills/pr-review/SKILL.md to maintain sync.

Low

• [design-direction] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:656 — The reconciliation step addresses the symptom (contradictory findings) rather than the root cause: the challenger pass (step 6d) never sees orchestrator findings because they are added at step 6e, after the challenger runs. Running the challenger after all findings are collected would naturally catch the contradiction without needing a separate reconciliation step.

• [doc-style] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:656 — The bold-paragraph headings (When to reconcile:, How to reconcile:, What reconciliation does NOT do:) are slightly inconsistent with the document's predominant pattern of using markdown headings for subsections, though bold lead-ins do appear elsewhere (e.g., Formatting rules:).

• [pattern-inconsistency] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:710 — Cross-reference (from 6e-1) omits the word "step"; existing references use (from step X) pattern.

• [pattern-inconsistency] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:656 — Numbered list items lack bold condition markers, inconsistent with the adjacent protected-path section's pattern (1. **Insufficient context**, 2. **Sufficient context**).

• [incomplete-doc] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:811 — The Constraints section mentions "PR-specific checks (step 6e) belong in the orchestrator only" but does not mention step 6e-1, which is also orchestrator-only.

---

Labels: Bug fix to the review agent skill file (harness component).