Fix regression scanner attribution source - #36686
Conversation
Fetch the fix PR body and author association from one REST issue response, keep search discovery structural-only, and cover the production attribution path with deterministic Pester tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 89f1c385-661b-4a3a-a48f-c7bf3f34ecb6
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36686Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36686" |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the regression-fix PR discovery/attribution flow used by the regression-corpus scanner to keep the initial GitHub search results structural-only while still enabling deterministic linked-issue discovery and trusted introducing-PR attribution.
Changes:
- Remove
bodyfromgh pr listresults inGet-MergedRegressionFixPRsto keep merged-fix discovery structural-only. - Add
Get-FixPrContextto fetch the fix PR’s REST issue body +author_associationtogether, and introduceGet-RegressionAttributionContextto centralize deterministic attribution orchestration. - Extend Pester coverage to validate that attribution works without relying on
gh pr listbodies, and that untrusted fix bodies are used for issue discovery but not attribution.
Show a summary per file
| File | Description |
|---|---|
| .github/scripts/Find-RegressionFixPRs.ps1 | Switches fix PR discovery to omit body from search results; adds REST-backed fix context + shared attribution orchestrator to keep attribution deterministic and trust-aware. |
| .github/scripts/Find-RegressionFixPRs.Tests.ps1 | Updates/extends unit tests to cover the new REST-backed context and attribution behavior, including coverage for “search results omit body” scenarios. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
kubaflo
left a comment
There was a problem hiding this comment.
🔍 AI-generated review — automated multi-model consensus (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), run on behalf of @kubaflo. Independent reviews → cross-pollinated → synthesized. This is a COMMENT (not an approval).
✅ LGTM — with two low, non-blocking notes
A faithful, security-improving refactor of the regression-scanner attribution path (follow-up to #35925). Opus dot-sourced the head functions, ran the full Pester suite (77/77 pass), and empirically exercised the null-context path; GPT concurred with no findings. Consensus verdict is LGTM.
Verified:
- Logic-preserving on the trusted path —
Get-RegressionAttributionContextfaithfully extracts the old inlined loop: same source ordering (comment-before-body), same trusted-only gating forpr-body/issue-body, same fix/linked-issue exclusion + first-reference break. The one divergence (new-and $fixPrBodyguard) is outcome-neutral (an empty body yields no references). - Integrity gap genuinely closed — discovery and the trust signal now both derive from the single REST issue fetch in
Get-FixPrContext, so differently-filtered data (search-list body vs REST body) is never combined.author_associationfrom the issue resource is the correct trust signal for a PR; untrusted PR body drives discovery only, and every attribution source (pr-body/issue-body/issue-comment) is trust-gated. The trusted path is sealed against spoofing. - No new bugs —
$null/ PSCustomObject / missing-Bodyall resolve to''safely; list flow returns correctly downstream.
Optional (non-blocking) — your call, @PureWeen
-
Fail-closed on a fix-PR REST hiccup drops discovery and attribution for that candidate.
.github/scripts/Find-RegressionFixPRs.ps1— whenGet-FixPrContextreturns$null(transient 5xx / rate-limit / network),$fixPrBody=''→ no linked-issue discovery and the candidate collapses toneedsHumanAttribution. Basemainran discovery off the search-list$pr.bodyindependently of the association fetch, so a transient failure there only dropped thepr-bodysource. All three models agree this new behavior is the correct fail-closed direction for a security-sensitive attribution pipeline and it self-heals (the scheduled scan re-lists merged PRs within the lookback window), so it's a conscious-sign-off item, not a defect. Consider a per-PR"fix context unavailable; skipping"log line for observability (Invoke-GhJson -AllowFailurealready warns). -
Add a null-context test.
.github/scripts/Find-RegressionFixPRs.Tests.ps1— the new tests cover the trusted-body and untrusted-body-discovery-only paths, but notGet-RegressionAttributionContext -FixPrContext $null(precisely the behavioral change above), nor comment-before-body precedence inside the extracted helper. Suggest anItasserting emptyLinkedIssueNumbers/ null attribution andShould -Invoke Get-IssueContext -Times 0.
Neither blocks merge.
ℹ️ Per policy this bot posts a comment, not a formal approval — merge/approve is @kubaflo's call.
Models: claude-opus-4.8 (LGTM), gpt-5.5 (LGTM), gemini-3.1-pro-preview (flagged #1 as medium; reconciled to low by the suite-run + null-path probe) · consensus: LGTM · confidence: high · findings: 2 low / 0 blocking
kubaflo
left a comment
There was a problem hiding this comment.
LGTM — approving.
Standing 3-model review (Opus 4.8 / GPT-5.5 / Gemini 3.1 Pro) at this head reached LGTM consensus: the regression-scanner attribution-source fix is correct and fail-closed behavior is preserved. Only two low, non-blocking notes were raised (fail-closed REST-hiccup sign-off; a null-context test) — neither blocks merge.
CI is clean (required checks green; only the advisory Build Analysis is pending).
🤖 AI-generated review (GitHub Copilot CLI, on behalf of @kubaflo).
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Summary
bodyfromgh pr list.author_associationatomically from the REST issue resource, then use that REST body for linked-issue discovery and trusted attribution.This is a focused follow-up to #35925. It avoids disabling gh-aw integrity filtering and leaves the workflow source and generated lock unchanged.
Production-shaped verification
A local 60-day scan with
MaxPRs=1now resolves fix PR #35768 to introducing PR #31931 ataf540589fc5a3b053cd8c08334d0ca5a0125451b, reports attribution sourcepr-body, and emitsusableCount: 1.Validation
gh aw compile regression-corpus-scannerwith source/lock parity0.10.0strict lint for.github/skills/code-review/tests/eval.vally.yamlgit diff --check