Prevent duplicate memory leak workflow fixes

[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!

🤖 AI-assisted change prepared by GitHub Copilot CLI.

Summary

• Prevents Daily Memory Leak Hunter from filing a new [leak-scan] issue when the canonical Type.Member already has an exact [leak-fix] PR merged to main or inflight/current.
• Adds merged/open fix and closed-attempt gates to Memory Leak Fixer, matching exact issue references plus canonical APIs so duplicate scanner issue numbers inherit existing fixes and attempt history.
• Runs the hunter's de-dup fetch as a fail-closed pre-agent step and resolves effective revert state recursively: a revert removes a fix, a revert-of-the-revert restores it, and deeper/multiple chains use parity.
• Preserves the mechanism-aware exception for distinct leaks sharing one Type.Member by persisting explicit decisions and rechecking every live API match before emission; the semantic comparison remains agent-authored and reviewer-visible.
• Re-checks live merged/open/revert metadata at a trusted default-branch safe-output boundary; the deterministic gate validates direct references, effective-revert filtering, and complete live-match decision coverage before mutation.
• Rejects unsupported empty API identities before build/test work, hardens title/reference parsing and Search API ceiling handling, and adds regressions for trusted-hook execution, mechanism exceptions, mutation-boundary enforcement, and recursive/parity revert chains.

This prevents repeats such as #36659 after #36369 had already merged the ListView.RefreshCommand fix into inflight/current, while still allowing distinct retention mechanisms such as the two GradientBrush.GradientStops leaks to be handled independently.

Validation

• Main-pinned compiler resolved dynamically from .github/aw/actions-lock.json: v0.85.4
• pwsh -NoLogo -NoProfile -Command Invoke-Pester... — 20 passed
• gh aw compile daily-leak-hunter --strict — 0 warnings
• gh aw compile leak-fixer --strict — 0 warnings
• gh aw compile --validate — all 14 workflows compiled; one pre-existing warning in copilot-evaluate-tests.md
• PowerShell AST parse for all four new script/module/test files
• git diff --check
• Targeted CI-pipeline security greps for token-wrapped product commands, worktree script fallbacks, and worktree verdict files — no findings

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36664

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36664"

[azure-pipelines]

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.

[Copilot]

Pull request overview

This PR updates the Daily Memory Leak Hunter and Memory Leak Fixer agentic workflow prompts to prevent duplicate [leak-scan]/[leak-fix] work by de-duping not only against open items, but also against equivalent fixes already merged into main or inflight/current. It also regenerates the corresponding gh-aw lock files and expands GitHub tool/permission scope needed to query PR metadata.

Changes:

• Add merged-fix de-dup gates based on canonical Type.Member extraction and live PR baseRefName (main/inflight).
• Update Daily Leak Hunter GitHub toolsets and workflow permissions to allow PR reads.
• Regenerate gh-aw lock files to reflect the prompt and permission/toolset changes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
.github/workflows/leak-fixer.md	Adds merged/open fix de-dup logic and canonical API extraction guidance to avoid duplicate fixer PRs.
.github/workflows/leak-fixer.lock.yml	Regenerated lock file to reflect updated leak-fixer prompt content hash.
.github/workflows/daily-leak-hunter.md	Adds merged-fix de-dup step and updates required GitHub toolsets/permissions in the prompt.
.github/workflows/daily-leak-hunter.lock.yml	Regenerated lock file + reflects new permissions/toolsets for MCP/GitHub reads.

[PureWeen]

Adversarial review

Finding

⚠️ Error Handling — The new merged-fix fetch can fail open when gh pr list errors because the pipeline has no pipefail or explicit fetch validation. jq can return success on empty input, causing the workflow to treat the merged-fix set as empty and create duplicate work. Consensus: 3/3 reviewers after dispute. See the inline comment for the concrete fix.

What looks right

The supported-branch filter uses live baseRefName, canonicalizes both sides consistently, and the generated lock changes faithfully mirror the source permission/toolset changes.

Test coverage

The workflow compilation coverage does not exercise upstream GitHub API/auth/rate-limit failures, which is the remaining behavioral gap.

Prior review status

One existing review raised empty/multiline-title concerns. Those were independently checked and did not produce the claimed false matches, so they were not duplicated here.

Methodology: 3 independent reviewers with adversarial consensus.

[kubaflo]

@PureWeen addressed all three review findings on the leak-dedup workflows (pushed fdf16ef9; both .lock.yml recompiled — body_hash only):

• ⚠️ Error handling / fail-open merged fetch — the gh pr list --state merged [leak-fix] fetch is now split from its jq filter and guarded (if ! … > …-raw.json; then … exit 1; fi), so a transient API/auth/rate-limit error aborts instead of leaving an empty merged-fix set that would re-file a duplicate. Applied the same fail-closed split to the identical fetch in leak-fixer.md step (a) for consistency.
• Copilot: multi-line issue titles — both title extractions now gsub("[\r\n]+";" ") so an embedded newline can't split into a second physical line and emit a spurious extra rooting-API key that over-suppresses an unrelated candidate.
• Copilot: empty rooting API — the same-API open-PR scan is now guarded on a non-empty $API; an empty $API_RE previously collapsed the regex to ^\[leak-fix\] +Fix +([. ]|$) and false-matched unrelated PRs.

Each failure mode was reproduced empirically before fixing. Ready for re-review — thanks!

🔍 AI-generated comment on @kubaflo's behalf.

[PureWeen]

Adversarial re-review

The head commit is unchanged, so no new inline comments were added.

⚠️ Unresolved prior finding — Error Handling: both new merged-fix gh pr list | jq pipelines can still treat an upstream API/auth/rate-limit failure as an empty dedup result because they lack pipefail or explicit fetch validation. This remains independently confirmed by 3/3 reviewers; the existing inline thread contains the concrete fix.

No additional findings survived adversarial validation. The proposed late-snapshot race and malformed-title fallback regressions were each rejected 2/3 after dispute.

Test coverage: workflow compilation validates schema and lock parity but does not exercise the failed-fetch path.

Prior review status: the author acknowledged and reproduced the fail-open behavior, but no fixing commit has been pushed.

Methodology: 3 independent reviewers with adversarial consensus.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[kubaflo]

@PureWeen re: the adversarial re-review (4746164920) — its snapshot predated the fix. fdf16ef9 was committed 15:14:35Z, ~4 min before the re-review posted (15:18:20Z), which is why it saw "head commit is unchanged"; that commit already added explicit fetch validation (if ! gh pr list … ; then exit 1) to the merged-fix pipelines in both workflows.

Copilot's fresh re-review of fdf16ef9 then correctly found that only the merged-fix gate was hardened while the other de-dup/cap fetches were still fail-open. Follow-up 9e6d1ee4 closes those out:

• leak-fixer.md — (b) open-PR-for-this-issue, (c) same-API open-PR, and (d) closed-unmerged attempt-cap are all now split-fetch + exit 1 on fetch failure (a fail-open on (d) would have reset the attempt cap to 0).
• daily-leak-hunter.md — the gh issue list open-scanner fetch is fail-closed too.

So all six de-dup/cap gates across both workflows are now fail-closed. Selection/work-list fetches (own-open-PRs, candidate issue list) are intentionally left fail-open — a transient failure there is a safe no-op, not a duplicate. Both .lock.yml recompiled (body_hash only). Ready for re-review — thanks!

🔍 AI-generated comment on @kubaflo's behalf.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[PureWeen]

Adversarial review

Findings

• ❌ Logic — The final mutation gate accepts a new PR title without the required [leak-fix] prefix, allowing it to become invisible to every later de-dup scan after merge. 3/3 reviewers after dispute.
• ⚠️ Regression — The gate's global exactly-one Refs: constraint conflicts with the workflow's required API-match citation path and can reject a legitimate different-mechanism fix. 2/3 reviewers, including the repo domain specialist.

Test coverage

The added Pester coverage does not exercise an untagged emitted PR title or a valid canonical reference plus an additional API-match citation.

Prior review status

No completed COMMENT review containing the Adversarial review and independent reviewer markers exists for this head.

Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist.

[kubaflo]

@PureWeen Both new inline findings are addressed in 3eab535: the final gate now enforces the literal [leak-fix] title prefix and permits additional Refs: citations while requiring exactly one canonical reference to the target issue. Targeted Pester validation passes (23/23). Ready for re-review.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/scripts/Assert-LeakFixSafeOutputGate.ps1:50

• Invoke-GhJson merges stderr into stdout (2>&1) before JSON parsing. If gh ever writes a warning/notice to stderr alongside JSON (even when exit code is 0), the combined stream will no longer be valid JSON and this final gate will fail unexpectedly. Capture stdout and stderr separately so success-path JSON parsing only sees stdout, while failure-path errors still include stderr for diagnostics.

    $output = & gh @Arguments 2>&1
    if ($LASTEXITCODE -ne 0) {
        throw "'gh $($Arguments -join ' ')' failed with exit code $LASTEXITCODE`: $output"
    }
    $raw = ($output -join [Environment]::NewLine)

[PureWeen]

Adversarial review

Findings

❌ Logic — The trusted gate permits a same-API exception based on an agent-authored state entry without requiring the generated PR to disclose the conflicting PR and mechanism comparison. The inline comment has the concrete failure path. Consensus: repo domain specialist + 2 reviewers after dispute.

❌ Regression — unresolved prior feedback — daily-leak-hunter snapshots merged fixes in pre-agent-steps, but can run for up to 90 minutes before Process Safe Outputs creates issues. A fix merged during that interval remains invisible and permits a duplicate [leak-scan] issue. The existing review feedback already identifies this mutation-boundary gap, so no duplicate inline comment was added.

What looks right

leak-fixer restores trusted scripts and executes its final gate before Process Safe Outputs; that is the appropriate mutation-boundary pattern.

Test coverage

The Pester additions cover direct matches, revert parity, and several gate success/failure paths, but do not require an emitted same-API exception citation or cover a final live refresh for the hunter.

Prior review status

Earlier feedback on the additional Refs: citation is addressed at this head: the gate now counts only the required target-issue reference. The stale hunter snapshot remains unresolved.

Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist. COMMENT-only review.

[kubaflo]

@PureWeen both findings from review 4964158437 are addressed in d2f0b65394c: live same-API exceptions now require an exact structured PR-body disclosure, and daily-leak-hunter has a trusted final live de-dup gate immediately before issue mutation. Focused Pester is 29/29; pinned gh-aw v0.85.4 strict compile/validation passed with 0 warnings. Ready for re-review.

AI-assisted response.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/scripts/Assert-LeakFixSafeOutputGate.ps1:41

• Invoke-GhJson captures gh stderr via 2>&1 and then parses the combined output as JSON. In PowerShell this can include stderr as ErrorRecord objects even when gh exits 0 (e.g., warnings/deprecation notices), which will corrupt $raw and cause ConvertFrom-Json to fail (false fail-closed). The repo already avoids this by splitting stdout/stderr before parsing (see .github/scripts/Query-CiFixPRs.ps1:142-158). Please adopt the same stdout/stderr separation here (and in the matching hunter gate) so successful JSON responses remain parseable even if gh emits warnings.

    $output = & gh @Arguments 2>&1
    if ($LASTEXITCODE -ne 0) {
        throw "'gh $($Arguments -join ' ')' failed with exit code $LASTEXITCODE`: $output"
    }
    $raw = ($output -join [Environment]::NewLine)

.github/scripts/Assert-LeakHunterSafeOutputGate.ps1:40

• Invoke-GhJson captures gh stderr via 2>&1 and then parses the combined output as JSON. If gh emits any warning/progress text to stderr while still exiting 0, $raw can include ErrorRecord text and ConvertFrom-Json will fail, blocking issue creation unnecessarily. The repo already handles this by separating stdout and stderr before parsing (e.g., .github/scripts/Query-CiFixPRs.ps1:142-158). Please apply that pattern here (and in the leak-fix gate) so benign stderr doesn’t break JSON parsing.

    $output = & gh @Arguments 2>&1
    if ($LASTEXITCODE -ne 0) {
        throw "'gh $($Arguments -join ' ')' failed with exit code $LASTEXITCODE`: $output"
    }
    $raw = ($output -join [Environment]::NewLine)

[PureWeen]

Adversarial review

❌ Config Impact — The Daily Memory Leak Hunter’s new final gate executes with a generated safe_outputs job that lacks pull-requests: read, so its required gh pr list calls fail whenever a real create_issue output is emitted.

⚠️ Logic — The hunter’s final gate treats any same-API match as a duplicate even though its de-dup contract also describes the retention path; a distinct mechanism on the same Type.Member cannot reach the fixer’s mechanism-aware exception.

⚠️ Logic — Revert parity is not branch-scoped, so a revert merged to another branch can mark an active main/inflight/current fix as reverted and cause unnecessary re-evaluation.

⚠️ Testing — The Pester gate tests stub gh, leaving the compiled safe-output permission boundary untested.

What looks right: the trusted default-branch gate checkout and fail-closed handling of unavailable or truncated de-dup metadata are correctly wired through the source and generated workflow.

Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist.
Consensus: permission gap 3/3 + specialist; mechanism mismatch 2/3 after dispute; branch-scoped revert gap 3/3 after dispute; permission-boundary test gap 1/3 + specialist.
Test coverage: decision logic is well covered, but the compiled job-permission contract for the new gate is not.
Prior review status: no qualifying adversarial review exists for this exact head; existing inline and issue comments were considered.

[kubaflo]

@PureWeen all four findings from review 4964976486 are addressed in dc8c4e11321: the hunter safe-output job has pull-requests: read with source/lock coverage, same-API distinct mechanisms use bounded visible overrides, and revert parity is base-branch scoped. I also fixed both gates to separate successful JSON stdout from benign gh stderr while preserving sanitized fail-closed diagnostics. Pester is 37/37; main-pinned gh-aw v0.85.4 strict compile/validation passed with 0 warnings. Ready for re-review.

AI-assisted response.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/scripts/Assert-LeakHunterSafeOutputGate.ps1:10

• Pin $PSNativeCommandUseErrorActionPreference to $false in this gate script for deterministic gh invocation behavior under $ErrorActionPreference = 'Stop'. The repo does this in other gh ... 2>&1 wrappers (e.g. .github/scripts/Query-CiFixPRs.ps1:34-40) to ensure native-command stderr/nonzero exits don’t surface as terminating errors that bypass structured error handling.

$ErrorActionPreference = 'Stop'
Import-Module (Join-Path $PSScriptRoot 'LeakWorkflowDedup.psm1') -Force

.github/scripts/Assert-LeakFixSafeOutputGate.ps1:11

• Pin $PSNativeCommandUseErrorActionPreference to $false in this gate script. Other repo scripts that wrap & gh ... 2>&1 do this explicitly (e.g. .github/scripts/Query-CiFixPRs.ps1:34-40) to prevent native-command stderr/nonzero exits from becoming terminating errors under $ErrorActionPreference = 'Stop' on future runner images. Without pinning, a change in the default preference can bypass Invoke-LeakGhJson’s exit-code handling and break the safe-output gate unpredictably.

$ErrorActionPreference = 'Stop'
Import-Module (Join-Path $PSScriptRoot 'LeakWorkflowDedup.psm1') -Force

[kubaflo]

Addressed the latest Copilot review in 16b08433ced.

Invoke-LeakGhJson now pins $PSNativeCommandUseErrorActionPreference = $false immediately before invoking gh, keeping nonzero exits in the helper’s explicit $LASTEXITCODE/stderr handling even if a future runner or profile changes the preference. The pin is centralized in the shared helper, so both fixer and hunter gates receive the protection without duplicated caller state. The focused suite passes all 38 tests.

Ready for re-review.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/scripts/LeakWorkflowDedup.Tests.ps1:36

• This test can pass even if the expected strings are missing: IndexOf(...) returns -1, and -1 will still satisfy Should -BeLessThan ... depending on the other index. Add explicit assertions that both substrings were found before comparing their order, so the test fails when the helper body changes unexpectedly.

        $helper.IndexOf('$PSNativeCommandUseErrorActionPreference = $false') |
            Should -BeLessThan $helper.IndexOf('$output = & gh @Arguments 2>&1')

[PureWeen]

Adversarial review

Finding

❌ Logic — The resolver treats two independent, still-active reverts of the same fix as if they restore it. With fix #100 and siblings #200 and #201 that both contain Reverts dotnet/maui#100, both direct reverters remain active, but line 282 computes an even count as active. Neither sibling reverts the other, so the original fix is still absent; the hunter and fixer continue treating #100 as proof and suppress a needed re-fix. The recursion already handles a real revert-of-revert, whose PR targets the first revert. Treat a target as active only when it has zero active direct reverters, and update the sibling-revert test that currently encodes the parity result. Consensus: repo domain specialist + 2 independent reviewers after dispute.

Test coverage

The added sibling-revert test enshrines the incorrect even-parity outcome; it should instead distinguish independent siblings from a true revert-of-revert chain.

Prior review status

Existing feedback was considered; no qualifying adversarial review existed for this exact head.

Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist. COMMENT-only review.

[PureWeen]

❌ Logic — Two independent active reverts of the same fix make this even-parity check report the fix as active, although neither revert restores it. The hunter/fixer then suppress a needed re-fix. A real revert-of-revert targets the first revert and is already handled by the recursion; only zero active direct reverters should keep the target active.

Flagged by: repo domain specialist + 2 independent reviewers after dispute