From 8c00c2fe8fe02c391650b3e279d2c198b3c8ab33 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 22 Jun 2026 08:51:03 +0000 Subject: [PATCH] fix(#2484): add finding reconciliation step to review agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../fullsend-repo/skills/pr-review/SKILL.md | 60 ++++++++++++++++++- 1 file changed, 58 insertions(+), 2 deletions(-) diff --git a/internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md b/internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md index 7d3226834..d0ec781ce 100644 --- a/internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md +++ b/internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md @@ -653,10 +653,66 @@ attention. If no protected files are modified, do not add a `protected-path` finding. +#### 6e-1. Finding reconciliation + +After all orchestrator checks (6e) have produced their findings, +reconcile them against the challenger-adjudicated sub-agent findings +before merging. The goal is to detect and resolve logical +contradictions — cases where one finding's evidence directly negates +another finding's premise. + +**When to reconcile:** Scan the combined set (sub-agent findings + +orchestrator findings) for pairs where: + +- One finding asserts that something is **missing** (e.g., "no + authorization exists for modifying protected paths") +- Another finding asserts that the same thing **is present** (e.g., + "authorization inferred from renovate.json configuration for + `.github/**` files") + +The most common pattern is a `protected-path` finding (from 6e) +claiming insufficient authorization while an `implicit-authorization` +or `missing-authorization` info-level finding (from a sub-agent) +cites specific configuration (e.g., `renovate.json`, `dependabot.yml`) +that explicitly authorizes the change pattern. + +**How to reconcile:** For each orchestrator finding, check whether any +existing sub-agent finding provides evidence that directly negates its +premise: + +1. If a sub-agent finding at **any severity** cites specific evidence + (a config file, a policy, a linked issue) that the changes to the + flagged paths are explicitly authorized, and the orchestrator + finding's premise is that authorization is missing or insufficient: + - **Downgrade** the orchestrator finding to **info** severity. + - Append to the description: "Note: [sub-agent-dimension] finding + cites [evidence source] as authorization for this change. Human + approval is still required for protected-path changes." + - Set `actionable: false` — the finding is now informational. + +2. If no sub-agent finding provides contradicting evidence, keep the + orchestrator finding unchanged. + +**What reconciliation does NOT do:** + +- It does not suppress `protected-path` findings entirely. Human + approval is always required for protected paths — the finding + remains as an info-level notice even when authorization evidence + exists. +- It does not override the `post-review.sh` downgrade behavior. + The post-script independently prevents approval on protected-path + PRs regardless of finding severity. +- It does not apply to findings with the same provenance. Two + sub-agent findings from the same dimension cannot contradict each + other in the reconciliation sense — intra-dimension consistency + is the sub-agent's responsibility. +- It does not re-run the challenger pass. Reconciliation operates + on the final finding set, not on intermediate results. + #### 6f. Determine overall outcome -Merge PR-specific findings into the challenger-adjudicated finding set -and evaluate: +Merge the reconciled PR-specific findings (from 6e-1) into the +challenger-adjudicated finding set and evaluate: - Any **critical** or **high** finding → `request-changes` - Multiple **medium** findings which could affect the intended outcome