Skip to content

[ci-scan] Broaden duplicate detection to catch sibling KBEs#128190

Draft
kotlarmilos wants to merge 1 commit into
mainfrom
kotlarmilos/ci-scan-broaden-dedup
Draft

[ci-scan] Broaden duplicate detection to catch sibling KBEs#128190
kotlarmilos wants to merge 1 commit into
mainfrom
kotlarmilos/ci-scan-broaden-dedup

Conversation

@kotlarmilos
Copy link
Copy Markdown
Member

@kotlarmilos kotlarmilos commented May 14, 2026

Description

Broadens the duplicate-detection search in ci-failure-scan.md so the agent catches sibling Known Build Errors filed for the same test class on a different platform or runtime variant, plus pre-existing area-team trackers that lack the Known Build Error label.

Motivation

Recent run 25835993942 filed #128175 for SocketBlockingModeTransitionTests.ConnectAsync_WithBuffer_Succeeds on android-x64, despite two existing issues covering the same failure family:

Two gaps caused the miss:

  1. The dedup queries were all keyed on the test method name (ConnectAsync_WithBuffer_Succeeds). GitHub best-match ranking returned only Assertion failure in ConnectAsync_WithBuffer_Succeeds on Android/MacCatalyst/tvOS (Mono/CoreCLR/NativeAOT) #125825 for that key, and [ci-scan] Known Build Error: SocketBlockingModeTransitionTests.ConnectAsync_WithBuffer_Succeeds on Android #128141 never surfaced. Searching by the test class name (SocketBlockingModeTransitionTests) returns all three issues plus a related sockets/Android KBE ([ci-scan] Known Build Error: System.Net.Sockets DualMode IPv6 tests fail on Android with Connection refused #127986).
  2. The single hit that did surface (Assertion failure in ConnectAsync_WithBuffer_Succeeds on Android/MacCatalyst/tvOS (Mono/CoreCLR/NativeAOT) #125825) was stripped by the MCP integrity guard (min-integrity: approved, MatousBot is author_association: NONE). The agent saw [Filtered] 1 item(s) ... removed by integrity policy and concluded "no dup found", then filed [ci-scan] Test failure: SocketBlockingModeTransitionTests.ConnectAsync_WithBuffer_Succeeds on android-x64 #128175. Lowering min-integrity is not on the table for security reasons, so the workflow needs to treat a filtered result as a likely-dup signal instead.

Change

The dedup step now:

  • Adds two new search variations: test class name + Known Build Error label, and test class name + area label without the KBE filter.
  • Instructs the agent to scan the first ~10 results of each variation rather than stopping at result 1 (GitHub best-match ranking is not deterministic for KBE-shaped queries).
  • Adds explicit [Filtered] handling: a filtered candidate is treated as skipped: integrity-filtered candidate, needs human review rather than "no dup".

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the CI failure scanner prompt to broaden duplicate issue detection for Known Build Errors, especially where related failures are filed under the same test class across different platform/runtime variants or where area-owned trackers lack the KBE label.

Changes:

  • Expands KBE search variations to scan multiple result candidates.
  • Adds test-class-based and area-label-based search strategies.
  • Adds explicit handling for integrity-filtered search results.
Show a summary per file
File Description
.github/workflows/ci-failure-scan.md Updates Step 4.2 duplicate-detection instructions for CI failure triage.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

4. Test class name + `label:"Known Build Error"`, e.g. `SocketBlockingModeTransitionTests label:"Known Build Error"`.
5. Test class name + area label, no KBE filter, e.g. `SocketBlockingModeTransitionTests label:area-System.Net.Sockets`.

Variations 4 and 5 catch sibling failures filed for the same test class on a different platform or runtime variant (e.g. android-x64 vs iossimulator), plus pre-existing area-team trackers that lack the `Known Build Error` label. If `search_issues` returns a `[Filtered]` marker on any variation, treat it as a likely existing-issue hit and record `skipped: integrity-filtered candidate, needs human review` instead of filing a fresh KBE. On any hit whose title or body references the same test class on any platform, record `existing-kbe #<n>` (or `linked-tracker #<n>` for variation 5 when the hit lacks the KBE label) and continue (the walk does not end; a hit changes the final action, not the inspection).
@github-actions
Copy link
Copy Markdown
Contributor

Caution

Security scanning requires review for Code Review

Details

The threat detection results could not be parsed. The workflow output should be reviewed before merging.

Review the workflow run logs for details.

🤖 Copilot Code Review — PR #128190

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: Well-justified. The PR description provides a concrete recent CI run that filed a duplicate issue, traces the root cause to two specific gaps in the dedup logic, and explains why the integrity-guard workaround (lowering min-integrity) is off the table.

Approach: The fix is appropriate — expanding search variations and handling filtered results defensively are both reasonable responses to the documented gaps. The change stays within the scope of the prompt file and doesn’t introduce unrelated modifications.

Summary: ✅ LGTM. This is a focused, well-motivated documentation change to a workflow prompt. The new search variations (test class name with and without KBE label) are logically sound extensions of the existing pattern, and the [Filtered] handling adds a sensible safety net. No correctness, performance, or compatibility concerns.


Detailed Findings

✅ Correctness — Search variations are well-structured

The new variations 4 and 5 logically complement the existing method-name–based searches (variations 1–3). Searching by test class name catches sibling failures for different methods in the same class or the same method on different platforms. The distinction between variation 4 (KBE-labeled) and 5 (area-label, no KBE filter) correctly maps to the two kinds of existing issues that were missed.

✅ Filtered-result handling — Conservative and safe

Treating a [Filtered] marker as “skipped: integrity-filtered candidate, needs human review” rather than “no dup found” is the correct defensive choice. It prevents filing duplicates while preserving the security posture of the integrity guard.

✅ Scope — Appropriately narrow

The change modifies only Step 4.2 in the prompt. Step 4.3 (area-team tracker search without KBE label) already exists and remains unchanged, which makes sense since variation 5 in Step 4.2 now overlaps slightly with 4.3 but serves a different purpose (dedup gating vs. cross-linking).

💡 Minor observation — Overlap between Step 4.2 variation 5 and Step 4.3

Variation 5 (test class name + area label, no KBE filter) partially overlaps with Step 4.3 (is:issue is:open in:title "<test-name>" without KBE label). The distinction is that 4.2/v5 searches by class name while 4.3 searches by method name, and their actions differ (4.2 gates filing; 4.3 cross-links). This is fine as-is but could be clarified with a brief note if future maintainers find it confusing. Not blocking.

✅ Instruction to scan ~10 results

The addition of “scanning the first ~10 results of each” is a pragmatic improvement over the implicit “stop at result 1” behavior, since GitHub’s best-match ranking is non-deterministic for these query shapes.

Generated by Code Review for issue #128190 · ● 1.5M ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants