fix(gate): dedupe the duplicate-only hold's warnings and repair #9206's engine fixture fallout

packages/loopover-engine/src/advisory/gate-advisory.ts

@@ -608,7 +608,11 @@ function evaluateGateCheckCore(advisoryResult: Advisory, policy: GateCheckPolicy
       title: `${LOOPOVER_GATE_CHECK_NAME} — held for manual review`,
       summary: blockers.map((finding) => sanitizeForCheckRun(finding.title)).join("; "),
       blockers: [],
-      warnings: [...gateWarnings, ...blockers],
+      // Held blockers move INTO warnings so the panel still shows them -- but `gateWarnings` is every
+      // warning-severity finding, and an escalated blocker (duplicate_pr_risk is authored `severity:
+      // "warning"`) is already in there, so appending it unfiltered listed it twice. Same
+      // `!blockers.includes(...)` exclusion the failure path below already applies, for the same reason.
+      warnings: [...gateWarnings.filter((finding) => !blockers.includes(finding)), ...blockers],
     };
   }
   // Name the exact blocker(s) + fix in the title so the contributor sees WHY at a glance.

packages/loopover-engine/test/iterate-loop.test.ts

@@ -22,8 +22,22 @@ function openIssue(number: number, title: string): IssueRecord {
   return { repoFullName: "acme/widgets", number, title, state: "open", labels: [], linkedPrs: [] };
 }
 
+/** The rival PR every duplicate-blocker fixture below needs. `changedFiles` is load-bearing since #9129:
+ *  a duplicate overlap corroborated only by body text now raises the always-advisory
+ *  `duplicate_pr_risk_unconfirmed` code instead of the blocking `duplicate_pr_risk`, so a rival with no
+ *  changed files would no longer block the predicted gate at all. Naming the same file the attempt itself
+ *  touches ("src/upload.ts") exercises the changed-file-overlap arm of hasDuplicateOverlapCorroboration. */
 function openPr(number: number, title: string, linkedIssues: number[] = []): PullRequestRecord {
-  return { repoFullName: "acme/widgets", number, title, state: "open", authorLogin: "someone-else", linkedIssues, labels: [] };
+  return {
+    repoFullName: "acme/widgets",
+    number,
+    title,
+    state: "open",
+    authorLogin: "someone-else",
+    linkedIssues,
+    labels: [],
+    changedFiles: ["src/upload.ts"],
+  };
 }
 
 const noopSlop: SelfReviewSlopAssessment = { slopRisk: 0, band: "clean", findings: [] };

packages/loopover-engine/test/self-review-adapter.test.ts

@@ -23,8 +23,22 @@ function openIssue(number: number, title: string): IssueRecord {
   return { repoFullName: "acme/widgets", number, title, state: "open", labels: [], linkedPrs: [] };
 }
 
+/** The rival PR the duplicate-blocker fixtures need. `changedFiles` is load-bearing since #9129: a duplicate
+ *  overlap corroborated only by body text now raises the always-advisory `duplicate_pr_risk_unconfirmed`
+ *  code instead of the blocking `duplicate_pr_risk`, so a rival with no changed files would no longer block
+ *  the predicted gate. Naming BASE_DIFF_STATE's own file exercises the changed-file-overlap arm of
+ *  hasDuplicateOverlapCorroboration. */
 function openPr(number: number, title: string, linkedIssues: number[] = []): PullRequestRecord {
-  return { repoFullName: "acme/widgets", number, title, state: "open", authorLogin: "someone-else", linkedIssues, labels: [] };
+  return {
+    repoFullName: "acme/widgets",
+    number,
+    title,
+    state: "open",
+    authorLogin: "someone-else",
+    linkedIssues,
+    labels: [],
+    changedFiles: ["src/upload.ts"],
+  };
 }
 
 const BASE_DIFF_STATE: AttemptDiffState = {
@@ -136,9 +150,16 @@ test("runSelfReview: a genuinely blocked synthetic diff (duplicate PR) matches c
   const context = baseContext({ pullRequests: [openPr(42, "Retry uploads on 5xx responses", [7])] });
   const result = runSelfReview(BASE_DIFF_STATE, context, { runSlopAssessment: () => noopSlop });
 
-  assert.equal(result.predictedGateVerdict.conclusion, "failure");
+  // #9129: a duplicate_pr_risk finding HOLDS the gate (neutral) rather than failing it under any
+  // duplicatePrGateMode -- the close-authority fix for the offensive-close attack, pinned identically by the
+  // host's golden corpus entry `duplicate-block-mode-holds`. `passesPredictedGate` stays false either way:
+  // a hold is still not a pass, which is what keeps the miner loop from handing off on a duplicate.
+  assert.equal(result.predictedGateVerdict.conclusion, "neutral");
   assert.equal(result.passesPredictedGate, false);
-  assert.ok(result.predictedGateVerdict.blockers.some((b) => b.code === "duplicate_pr_risk"));
+  // Held, so the finding moves out of `blockers` and into `warnings` -- exactly once (#9129 regression: the
+  // hold path spread the escalated blocker into a warning set that already contained it).
+  assert.deepEqual(result.predictedGateVerdict.blockers, []);
+  assert.equal(result.predictedGateVerdict.warnings.filter((w) => w.code === "duplicate_pr_risk").length, 1);
 
   const direct = buildPredictedGateVerdict({
     input: buildSelfReviewPredictedGateInput(BASE_DIFF_STATE),

src/rules/advisory.ts

@@ -824,7 +824,11 @@ function evaluateGateCheckCore(advisoryResult: Advisory, policy: GateCheckPolicy
       title: `${LOOPOVER_GATE_CHECK_NAME} — held for manual review`,
       summary: blockers.map((finding) => sanitizeForCheckRun(finding.title)).join("; "),
       blockers: [],
-      warnings: [...gateWarnings, ...blockers],
+      // Held blockers move INTO warnings so the panel still shows them -- but `gateWarnings` is every
+      // warning-severity finding, and an escalated blocker (duplicate_pr_risk is authored `severity:
+      // "warning"`) is already in there, so appending it unfiltered listed it twice. Same
+      // `!blockers.includes(...)` exclusion the failure path below already applies, for the same reason.
+      warnings: [...gateWarnings.filter((finding) => !blockers.includes(finding)), ...blockers],
     };
   }
   // Name the exact blocker(s) + fix in the title so the contributor sees WHY at a glance.

test/unit/gate-check-policy.test.ts

@@ -250,6 +250,22 @@ describe("AI fail-closed hold (#ai-fail-closed)", () => {
     expect(result.blockers).toEqual([]);
   });
 
+  it("REGRESSION (#9129): a duplicate-only HOLD lists each held blocker exactly once in warnings", () => {
+    // duplicate_pr_risk is authored `severity: "warning"`, so it is ALREADY in the warning set the hold path
+    // spreads; appending the escalated blocker unfiltered listed it twice on the panel. The failure path has
+    // always excluded blockers from that spread -- the hold path now does the same.
+    const advisory = missingIssueAdvisory();
+    advisory.findings.push({ code: "duplicate_pr_risk", title: "Possible duplicate PR", severity: "warning", detail: "Overlaps #9.", action: "Close the duplicate." });
+    const eff = resolveEffectiveSettings(settings({ linkedIssueGateMode: "advisory", duplicatePrGateMode: "block" }), parseFocusManifest(null));
+    const result = evaluateGateCheck(advisory, gateCheckPolicy(eff, null, true));
+
+    expect(result.conclusion).toBe("neutral"); // held for a human, never closed (#9129)
+    expect(result.blockers).toEqual([]);
+    expect(result.warnings.filter((finding) => finding.code === "duplicate_pr_risk")).toHaveLength(1);
+    // The non-blocker warning alongside it is still carried through exactly once, unaffected by the filter.
+    expect(result.warnings.filter((finding) => finding.code === "missing_linked_issue")).toHaveLength(1);
+  });
+
   it("REGRESSION (#9082): an incomplete secret scan HOLDS the gate (neutral), never one-shot-closes a PR ORB couldn't read", () => {
     const held: Advisory = {
       ...missingIssueAdvisory(),