Add AI memory-leak scanner + fixer agentic workflows - #36342
Conversation
Introduces two gh-aw agentic workflows that hunt and fix managed, cross-platform memory leaks in MAUI — no emulator/simulator needed for detection: - daily-leak-hunter: scheduled every 12h. Pass A hunts one runtime managed leak (shipped-package dotnet test, WeakReference + forced GC) and files a [leak-scan] issue only on empirical confirmation. Pass B (when Pass A is quiet) statically diffs the control surface vs MemoryTests.cs coverage and files a [leak-test-gap] issue proposing the missing test. - leak-fixer: manual (workflow_dispatch). Takes one [leak-scan] issue, writes a Controls.Core.UnitTests regression test that FAILS on main without the fix, implements the idiomatic managed fix (weak proxy / teardown), confirms it PASSES, and opens a draft [leak-fix] PR (Fixes #N). Builds MAUI from source to validate. Adapted to repo conventions (environment: gh-aw-agents, dotnet/maui guard, agentic-workflows label, dotnet network ecosystem). Compiled with gh-aw v0.79.8. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two enhancements to the fixer:
- Scheduled (every 12h) in addition to manual dispatch, matching the hunter and
ci-status-fix, so it works through the open backlog automatically.
- Now consumes BOTH issue types the hunter files:
- Track A ([leak-scan]) - runtime leak: regression test + managed fix, red->green,
opens a [leak-fix] PR (unchanged behavior).
- Track B ([leak-test-gap], NEW) - missing coverage: adds the proposed memory-leak
test (Core.UnitTests guard and/or DeviceTests HandlerDoesNotLeak InlineData),
validates as far as the runner allows, opens a [leak-test] PR. If a Core.UnitTests
guard unexpectedly fails, it escalates to Track A (real leak found).
Also: search issues/PRs by the agentic-workflows label + both title tags; allow the
DeviceTests test path; agent writes the full title tag ([leak-fix]/[leak-test]).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Highest-priority track, checked FIRST each run: if one of this workflow's own open [leak-fix]/[leak-test] PRs has an unaddressed CHANGES_REQUESTED review, the fixer works on THAT PR instead of opening new work — it applies the valid requested changes (pushing a commit to the PR branch, re-validated so the PR's test still holds) and/or posts a comment pushing back on any request that is wrong, would regress behaviour, or asks for a mute. Adds two safe-outputs scoped by required-title-prefix '[leak-' to its own PRs: push-to-pull-request-branch (target: '*') and add-comment (target: '*'). Loop-guard: only acts on a review newer than both the PR's last commit and the workflow's last comment, so once it pushes/comments the review is addressed and won't be re-processed. One action per run (Track C OR Track A/B, never both). Adds gh to the bash allowlist for review reads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fixer was hitting the default 5000/day AIC guardrail (activation aborted before the agent ran). Set max-ai-credits: -1 (unlimited), matching the hunter and the ci-status-* workflows, so scheduled/retry runs are not throttled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both the hunter and fixer were subject to gh-aw's daily AIC guardrail (default 5000/day, from the unset GH_AW_DEFAULT_MAX_DAILY_AI_CREDITS repo var), which aborted runs at activation once exceeded. Setting max-daily-ai-credits: -1 removes the 'Check daily workflow token guardrail' step entirely for both scanners so they are never throttled by the daily cap. (max-ai-credits: -1 already removed the per-run cap.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Boost [leak-scan] yield (per @kubaflo / @AdamEssenmacher's known-leak catalog): - Multi-leak per run: create-issue max 1 -> 8; Pass A now sweeps ALL focus areas and files one issue per DISTINCT empirically-confirmed leak (batched into one leakprobe project, one [Fact] per candidate). Pass B likewise files multiple coverage gaps up to the remaining cap. - Catalog-seeded focus table: each focus row now names proven MAUI leak signatures to hunt for new instances of — shared-publisher -> strong CollectionChanged/PropertyChanged (Picker/TableRoot/SelectedItems/GradientStops), ICommand.CanExecuteChanged (ListView.RefreshCommand/SwipeItemView/BackButtonBehavior), VSM state triggers -> DeviceDisplay.MainDisplayInfoChanged, static roots. - Model diversity: two sibling variants (daily-leak-hunter-gpt = gpt-5, daily-leak-hunter-gemini = gemini-2.5-pro) run the same hunt under different models; each files to the shared [leak-scan] queue with the same de-dup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…riants Multi-model variants added runtime uncertainty (Copilot-subscription model availability unverified) for little gain. Keep the single default hunter with the multi-leak-per-run + catalog-seeded-focus improvements. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36342Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36342" |
There was a problem hiding this comment.
Pull request overview
Adds two new gh-aw agentic workflows to automate (1) scanning for managed MAUI memory leaks and (2) turning confirmed leak issues / coverage gaps into draft PRs, along with their compiled .lock.yml artifacts.
Changes:
- Add Daily Memory Leak Hunter workflow to file
[leak-scan]and[leak-test-gap]issues. - Add Memory Leak Fixer workflow to open draft
[leak-fix]/[leak-test]PRs and optionally respond to review feedback on its own PRs. - Add the compiled gh-aw
.lock.ymlworkflow artifacts for both workflows.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| .github/workflows/daily-leak-hunter.md | New scanner workflow spec (two-pass leak hunt + coverage-gap scan) and safe-output configuration. |
| .github/workflows/daily-leak-hunter.lock.yml | Compiled workflow artifact for the leak hunter. |
| .github/workflows/leak-fixer.md | New fixer workflow spec (Track A/B PR creation + Track C review-response loop) and safe-output configuration. |
| .github/workflows/leak-fixer.lock.yml | Compiled workflow artifact for the leak fixer. |
| # This workflow's own open leak PRs (its create-pull-request output labels them agentic-workflows). | ||
| gh pr list --repo "$GITHUB_REPOSITORY" --state open \ | ||
| --search 'in:title "[leak-fix]" OR "[leak-test]"' \ | ||
| --json number,title,headRefName,url,updatedAt \ | ||
| | jq 'sort_by(.number)' > /tmp/gh-aw/agent/my-open-prs.json |
There was a problem hiding this comment.
🔍 AI-generated reply on @kubaflo's behalf.
Partially addressed on the current head: the R1 gh pr list query now post-filters select(.headRepositoryOwner.login == $owner) (same-repo only), and any write is further gated by push-to-pull-request-branch.required-title-prefix: [leak-fix] plus the fork-head push limitation. A stray same-repo [leak-fix]-titled PR from another author is still theoretically enumerable; adding an agentic-workflows label filter would fully close that. Leaving open to track the optional label hardening.
| <PackageReference Include="Microsoft.Maui.Controls" Version="10.0.0" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" /> | ||
| <PackageReference Include="xunit" Version="2.9.2" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" /> | ||
| </ItemGroup> | ||
| </Project> | ||
| ``` | ||
|
|
||
| (Use a `10.0.x` version that restores; `10.0.0` is known to work.) |
There was a problem hiding this comment.
🔍 AI-generated reply on @kubaflo's behalf.
This is an intentional floor: the repro pins a shipped baseline so a confirmed leak reproduces against a real release rather than an unreleased head. Agreed it drifts as versions advance — a good follow-up is to resolve the current shipped patch dynamically (e.g. from the latest 10.0.* NuGet) instead of a literal. Leaving open to track that improvement.
Per maintainer: the system should surface ONLY empirically-proven leaks, not "missing test" coverage gaps. - Hunter: remove Pass B (leak-test-gap coverage-gap scan) entirely. It now files ONLY [leak-scan] issues, each proven by a control/leaky/mitigation dotnet test. A run that confirms nothing files nothing (no fallback). - Fixer: remove Track B ([leak-test-gap] -> [leak-test] coverage PRs). It now produces only [leak-fix] PRs (Track A, proven leak + red->green regression test) and Track C (review response on its own [leak-fix] PRs). Safe-outputs no longer permit leak-test/** branches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Track C pushes fixes via push-to-pull-request-branch, which can't reach fork-hosted PR heads in scheduled runs. It was looping on the fork PR #36253 (re-commenting + filing an [aw] missing-tool report each run) and never reaching Track A. Now it considers only dotnet/maui-hosted [leak-fix] PRs and leaves fork-hosted ones for a human without emitting a missing-tool report. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| gh pr list --repo "$GITHUB_REPOSITORY" --state open \ | ||
| --search '"[leak-fix]" in:title' \ | ||
| --json number,title,headRefName,headRepositoryOwner,url,updatedAt \ | ||
| | jq --arg owner "$OWNER" '[.[] | select((.headRepositoryOwner.login // "") == $owner)] | sort_by(.number)' \ |
There was a problem hiding this comment.
🔍 AI-generated reply on @kubaflo's behalf.
Partially addressed on the current head: the R1 gh pr list query now post-filters select(.headRepositoryOwner.login == $owner) (same-repo only), and any write is further gated by push-to-pull-request-branch.required-title-prefix: [leak-fix] plus the fork-head push limitation. A stray same-repo [leak-fix]-titled PR from another author is still theoretically enumerable; adding an agentic-workflows label filter would fully close that. Leaving open to track the optional label hardening.
| You produce **only `[leak-fix]` PRs for empirically-proven leaks** — there is no coverage-gap / | ||
| "add a missing test" mode. Never open a `[leak-test]` PR or act on a `[leak-test-gap]` issue. | ||
|
|
There was a problem hiding this comment.
🔍 AI-generated reply on @kubaflo's behalf.
Good spot — the PR description mentions a [leak-test-gap] / coverage-gap mode that isn't in this iteration of the workflows. That's a description/scope drift, not a code issue; I'll trim the description to match what actually ships here. Leaving open to track the description edit.
The de-dup fetch used --label memory-leak, but issues are filed with --label agentic-workflows, so the 'my open issues' list was always EMPTY and nothing got de-duped — the multi-leak sweep re-filed leaks it had already filed in a prior run (with reworded titles). Fix the label, extract the rooting Type.Member from each open issue, and require titles to lead with the canonical Type.Member so de-dup is deterministic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The same leak can be filed under multiple [leak-scan] issue numbers (duplicate scans, or a pre-existing upstream issue). De-dupping only by 'Fixes #N' meant the fixer would open a second [leak-fix] PR for a leak already being fixed under a different number. Now it also skips a target whose rooting Type.Member already has an open [leak-fix] PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| # Enforced allowlist of paths a run may touch: a runtime-leak fix is a managed product | ||
| # change + a Core.UnitTests regression test (+ PublicAPI). | ||
| allowed-files: | ||
| - "src/Controls/src/**" | ||
| - "src/Controls/tests/Core.UnitTests/**" | ||
| - "src/Controls/tests/DeviceTests/**" | ||
| - "src/Core/src/**" |
There was a problem hiding this comment.
🔍 AI-generated reply on @kubaflo's behalf.
Deliberate headroom rather than a bug: the primary path writes a Controls.Core.UnitTests regression test, but some rooting leaks are only reproducible as a DeviceTest, so DeviceTests/** stays in the allow-ceiling (it doesn't mandate touching it). It's still test-only code and bounded by max-patch-size. Leaving open — happy to drop it if you'd prefer the tighter net-only scope.
| # This workflow's own open [leak-fix] PRs (its create-pull-request output labels them agentic-workflows). | ||
| # Track C pushes fixes via push-to-pull-request-branch, which can ONLY reach PR heads hosted on | ||
| # this same repo — scheduled runs have no credentials for FORK-hosted heads. So consider only | ||
| # same-repo (dotnet/maui-hosted) PRs here; skip fork-hosted [leak-fix] PRs entirely. | ||
| OWNER="${GITHUB_REPOSITORY%%/*}" | ||
| gh pr list --repo "$GITHUB_REPOSITORY" --state open \ | ||
| --search '"[leak-fix]" in:title' \ | ||
| --json number,title,headRefName,headRepositoryOwner,url,updatedAt \ | ||
| | jq --arg owner "$OWNER" '[.[] | select((.headRepositoryOwner.login // "") == $owner)] | sort_by(.number)' \ |
There was a problem hiding this comment.
🔍 AI-generated reply on @kubaflo's behalf.
Partially addressed on the current head: the R1 gh pr list query now post-filters select(.headRepositoryOwner.login == $owner) (same-repo only), and any write is further gated by push-to-pull-request-branch.required-title-prefix: [leak-fix] plus the fork-head push limitation. A stray same-repo [leak-fix]-titled PR from another author is still theoretically enumerable; adding an agentic-workflows label filter would fully close that. Leaving open to track the optional label hardening.
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Maui.Controls" Version="10.0.0" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" /> | ||
| <PackageReference Include="xunit" Version="2.9.2" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" /> | ||
| </ItemGroup> | ||
| </Project> | ||
| ``` | ||
|
|
||
| (Use a `10.0.x` version that restores; `10.0.0` is known to work.) | ||
|
|
There was a problem hiding this comment.
🔍 AI-generated reply on @kubaflo's behalf.
This is an intentional floor: the repro pins a shipped baseline so a confirmed leak reproduces against a real release rather than an unreleased head. Agreed it drifts as versions advance — a good follow-up is to resolve the current shipped patch dynamically (e.g. from the latest 10.0.* NuGet) instead of a literal. Leaving open to track that improvement.
… only src) The create_pull_request patch is rejected if it touches files outside the src/** allowlist. In pre-merge test runs on a branch that also modifies the workflow files, 'git add -A' could stage that delta. Defensively reset .github to origin/main and stage only src/** before committing — no-op on main, robust on the test branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen
left a comment
There was a problem hiding this comment.
Really like this — the leak-scan → validated [leak-fix] PR pipeline is clean, and Track C (respond to review feedback before taking on new work) is a nice touch. We're adopting the same review-response pattern on the ci-fix loop (#36317), so I've been reading Track C closely and wanted to flag one thing worth hardening before it runs live.
Track C ingests review content from any author and can act on it. R1 selects the newest CHANGES_REQUESTED review filtered only by state, then R2 reads its body + inline comments and may APPLY code changes from them. On a public PR, anyone can submit a "Request changes" review without write access — so a review body from an untrusted account could carry instructions the agent then actions, and the APPLY / PUSH BACK split is a prompt-level judgment, not a handler-level gate. (min-integrity: approved gates the trigger, not the review text you fetch mid-run.)
Cheap fix that matches the trust model you already rely on elsewhere: only action reviews whose author has write access — filter to author_association ∈ {OWNER, MEMBER, COLLABORATOR} before R2. One extra clause in the R1 jq:
gh api "repos/$GITHUB_REPOSITORY/pulls/$N/reviews" \
--jq 'map(select(.state=="CHANGES_REQUESTED"
and (.author_association | IN("OWNER","MEMBER","COLLABORATOR"))))
| sort_by(.submitted_at) | last' \
> /tmp/gh-aw/agent/review_${N}.jsonSame idea for the inline pulls/$N/comments fetch (it also carries author_association). This is the same "the prompt is a soft constraint, not a handler-level one" note you gave us on ci-fix #36317 — so we're taking it on both loops. Not blocking, your call; flagging it because we're about to mirror this exact flow.
…op issues
Address PureWeen review findings on the leak-fixer agentic workflow:
- SECURITY: Track C reads CHANGES_REQUESTED review bodies and inline comments
via raw 'gh api', a path NOT behind the MCP 'min-integrity: approved' gateway.
On a public repo anyone can submit a 'Request changes' review, so untrusted
review text could reach the APPLY step. Filter both the reviews and inline
comments to author_association IN {OWNER, MEMBER, COLLABORATOR} so only
write-access reviewers are actioned.
- Add 'noop: report-as-issue: false' so idle 12h runs (the common case, when
every open leak already has a [leak-fix] PR) stop filing 'no action taken'
issues — mirroring daily-leak-hunter.md.
Recompiled leak-fixer.lock.yml (gh aw v0.79.8, 0 errors).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Step 2 de-dup step calls 'gh issue list' to fetch this scanner's own open [leak-scan] issues, but 'gh' was missing from tools.bash, so the compiled lock omitted shell(gh:*) and the first scheduled run would fail before scanning. Add 'gh' to the allowlist (matching leak-fixer.md) and recompile the lock. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address the 🔴 de-dup logic finding (empirically confirmed by #36345/#36354) and title-prefix scoping findings: - daily-leak-hunter: extract the FIRST dotted Type.Member token anywhere in the title (awk match) instead of cutting at the first space/colon. Off-contract titles like '[leak-scan] Shell BackButtonBehavior.Command …' (#36345) and '[leak-scan] BackButtonBehavior.Command: …' (#36354) now both normalize to 'BackButtonBehavior.Command', so the same rooting API is no longer re-filed. - leak-fixer: tighten push-to-pull-request-branch required-title-prefix from '[leak-fix' to '[leak-fix]' (the missing bracket widened the write scope), and narrow add-comment's prefix likewise to match the stated 'own [leak-fix] PRs' intent. Recompiled both locks (gh aw v0.79.8, 0 errors). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Focus area 7 cited OrientationStateTrigger/DisplayRotationStateTrigger and DeviceDisplay.MainDisplayInfoChanged as a shared source, but Step 4 forbids platform Essentials APIs like DeviceDisplay (they throw NotImplementedInReferenceAssembly on plain net). Replace with net-testable state-trigger examples and an explicit 'no platform display sources' note so the scan guidance matches the workflow's net-only constraint. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen @copilot-pull-request-reviewer — ready for re-review. Pushed 5 commits addressing the review: Implemented + threads resolved
Left open (with replies) — cost cap All |
| | 4 | **Shared publisher → strong subscription with no weak proxy** — the #1 catalog pattern. A control subscribes to an *external/shared/long-lived* collection or element via `CollectionChanged` / `PropertyChanged` and never unsubscribes: `Picker.ItemsSource`, `TableView`/`TableRoot` section collection, `SelectableItemsView.SelectedItems`, `CarouselView` item source, etc. | | ||
| | 5 | **Shared `ICommand` → `CanExecuteChanged`** roots the control (strong subscription, no teardown): `ListView.RefreshCommand`, `SwipeItemView.Command`, `BackButtonBehavior.Command`, `MenuItem.Command`, toolbar/refresh commands. | | ||
| | 6 | Animation / `IAnimationManager` / tweener / ticker plumbing (tickers/animators not stopped/disposed on teardown; `AnimatableKey`/animation-cache retention) | | ||
| | 7 | `AttachedCollection` / triggers / behaviors / **VisualStateManager**: state triggers (`OrientationStateTrigger`, `DisplayRotationStateTrigger`, `CompareStateTrigger`, `AdaptiveTrigger`) that subscribe to a shared source (e.g. `DeviceDisplay.MainDisplayInfoChanged`) and stay subscribed when VSGroups are replaced; `Shapes`/`Geometry`/`Brush` change-notification (`StrokeDashArray`, `GradientBrush.GradientStops`, `Path` geometry). | |
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Maui.Controls" Version="10.0.0" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" /> | ||
| <PackageReference Include="xunit" Version="2.9.2" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" /> | ||
| </ItemGroup> |
| (Use a `10.0.x` version that restores; `10.0.0` is known to work.) | ||
|
|
||
| `/tmp/gh-aw/agent/leakprobe/LeakTest.cs`: **one `[Fact]` per candidate** (name them clearly, |
| Build and run **only your new test** on the .NET library TFM. Restrict to the `net` TFM with | ||
| the five platform-exclusion flags (no Android/iOS/etc. workload needed) and disable | ||
| `TreatWarningsAsErrors` (some test projects have pre-existing warning-as-error noise). The | ||
| project-reference graph **bootstraps the MAUI build tasks and builds `Core` → `Controls.Core` | ||
| → the test project automatically** — you do NOT need to build `Microsoft.Maui.BuildTasks.slnf` | ||
| first: | ||
|
|
||
| ```bash | ||
| FLAGS="-p:IncludeIosTargetFrameworks=false -p:IncludeAndroidTargetFrameworks=false -p:IncludeMacCatalystTargetFrameworks=false -p:IncludeWindowsTargetFrameworks=false -p:IncludeTizenTargetFrameworks=false -p:TreatWarningsAsErrors=false" |
| ## Step 8 — Rebuild + run the test — confirm it PASSES (green), no regressions | ||
|
|
||
| ```bash | ||
| FLAGS="-p:IncludeIosTargetFrameworks=false -p:IncludeAndroidTargetFrameworks=false -p:IncludeMacCatalystTargetFrameworks=false -p:IncludeWindowsTargetFrameworks=false -p:IncludeTizenTargetFrameworks=false -p:TreatWarningsAsErrors=false" |
| # SECURITY: only ACTION reviews/comments from an author with write access. On a public | ||
| # repo anyone can submit a "Request changes" review, and this raw `gh api` path is NOT | ||
| # behind the MCP `min-integrity: approved` gateway (that only filters MCP tool calls), | ||
| # so untrusted review text could otherwise reach Step R and influence a code push. | ||
| # Gate on author_association ∈ {OWNER, MEMBER, COLLABORATOR} before anything is actioned. | ||
| gh api "repos/$GITHUB_REPOSITORY/pulls/$N/reviews" \ | ||
| --jq 'map(select(.state=="CHANGES_REQUESTED" | ||
| and (.author_association | IN("OWNER","MEMBER","COLLABORATOR")))) | ||
| | sort_by(.submitted_at) | last' \ |
| # Inline review comments (may carry specific line-level requests) — same write-access filter: | ||
| gh api "repos/$GITHUB_REPOSITORY/pulls/$N/comments" \ | ||
| --jq 'map(select(.author_association | IN("OWNER","MEMBER","COLLABORATOR"))) | ||
| | map({path,line,body,user:.user.login,created_at})' > /tmp/gh-aw/agent/inline_${N}.json |
| OWNER="${GITHUB_REPOSITORY%%/*}" | ||
| gh pr list --repo "$GITHUB_REPOSITORY" --state open \ | ||
| --search '"[leak-fix]" in:title' \ | ||
| --json number,title,headRefName,headRepositoryOwner,url,updatedAt \ | ||
| | jq --arg owner "$OWNER" '[.[] | select((.headRepositoryOwner.login // "") == $owner)] | sort_by(.number)' \ | ||
| > /tmp/gh-aw/agent/my-open-prs.json |
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial re-review (round 2) — 3 independent reviewers, consensus
Methodology: three independent model reviewers under adversarial consensus (agree / dispute / discard), cross-checked against the existing Copilot inline comments and my round-1 review. The two .lock.yml files are compiler-generated — I verified facts against them (shell(gh:*) granted, noop report-as-issue:"false", required_title_prefix:"[leak-fix]", min-integrity: approved), not their content.
✅ Round-1 findings resolved (3/3 agree; de-dup reproduced locally)
ghmissing from the bash allowlist → now allow-listed; the lock grantsshell(gh:*). (bbe3fd1)- Brittle
sedde-dup key → replaced with aType.Memberawkextraction; I reproduced thatShell BackButtonBehavior.CommandandBackButtonBehavior.Command:now collapse to one key. (11d6b42) — one residual edge below. noopauto-issue →report-as-issue: falseon both workflows. (2feb1d4)- Raw-
ghinjection vector pastmin-integrity→ theauthor_associationgate is now applied to both the review fetch and the inline-comment fetch. (2feb1d4) - Title-prefix scoping →
[leak-fix]prefixes +required_title_prefixon push/comment. (11d6b42) DeviceDisplaycontradiction in focus area 7 → removed. (0543e28)
🆕 New this round (see inline)
daily-leak-hunter.md:145⚠️ the new de-dupawkover-collapses namespace-qualified titles → silent under-filing (2/3, reproduced).leak-fixer.md:353⚠️ the de-dup hardening from this PR was not mirrored into the fixer’s Track A selection → duplicate[leak-fix]PRs on the same off-contract titles (reproduced), plus an unescaped jq regex at L365.
🔁 Corroborating the existing Copilot inline comments (independent agreement — not re-posted inline)
- Track C can act on any same-repo
[leak-fix]PR (fixer ~226–229) — 3/3 agree this is the strongest remaining gap (one reviewer rated it must-fix).push-to-pull-request-branchis bounded only by title-prefix, not branch. Scope Step R1 to--label agentic-workflowsand/or addallowed-branches: ["leak-fix/**"]. author_association≠ write access (fixer ~250) — the gate correctly closes the injection vector; the residual is precision:MEMBER/COLLABORATORisn’t strictly write. Either queryrepos/{repo}/collaborators/{user}/permissionforwrite|maintain|admin, or soften the “write access” comment.- Probe scope vs Essentials refs (hunter ~229) / probe omits the
DispatcherProviderbootstrap (hunter ~235) — agree; both are bounded (wasted/always-dropped candidates and false-negatives, not false filings). TreatWarningsAsErrors=false(fixer 439/479) — contested: two reviewers noted the CI-fidelity gap, one judged it a deliberate, documented choice caught by full CI on the draft PR. Non-blocking.
⏳ Still open from round 1
max-ai-credits: -1(hunter 42–43, fixer 60–61) — unbounded spend. The environment gating these runs had no required-reviewer protection rules when I checked in round 1, so there is no approval gate acting as the real spend control. Acknowledged / deferred by the author.
Notes
- No unit tests — expected; these are workflow definitions.
- Not blocking merge from my side. COMMENT only — approval is a human decision.
…n fixer Follow-up to the round-2 de-dup fix (PureWeen review): - daily-leak-hunter: the first-Type.Member extraction over-collapsed fully-qualified titles — '[leak-scan] Microsoft.Maui.Controls.Picker.ItemsSource' keyed on the namespace head 'Microsoft.Maui', so distinct leaks (Picker.ItemsSource vs TableRoot.Section) collided to one key and the second was silently dropped. Now extract the LAST Type.Member pair of the first identifier chain (awk split), and allow '_' in identifiers (ItemsSource_Changed-style members). - leak-fixer: the fixer's de-dup still used the old prefix-cut the hunter abandoned, mis-keying the same off-contract titles and opening duplicate [leak-fix] PRs. Mirror the hunter's awk extraction. Also escape regex metacharacters in $API before the jq test() calls so 'Type.Member' matches literally (the '.' was a wildcard). Recompiled both locks (gh aw v0.79.8, 0 errors). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen addressed the round-3 de-dup findings in |
|
Re-reviewed
Lock regeneration is correct — the hash-only lock diff is expected because the workflow body is The remaining items from my earlier review are unchanged this round and non-blocking from my side — the strongest is still Track C acting on any same-repo |
<!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! > [!NOTE] > 🤖 **AI-generated PR.** These workflows, and this description, were produced with AI assistance. Please review carefully before merging. ## Summary Adds two [gh-aw](https://github.com/github/gh-aw) agentic workflows that proactively **find** and **fix** managed, cross-platform memory leaks in MAUI — turning the AI-driven leak-hunting flow several contributors already run by hand (scan → repro → measure → file → fix) into a repeatable, reviewable pipeline. **Design principle: only empirically-proven leaks.** The scanner files an issue **only** when a plain `dotnet test` demonstrates the leak; the fixer opens a PR **only** with a red→green regression test proving the fix. There is no "you're missing a test" / coverage-gap mode — every artifact is backed by a passing/failing test. **Key property:** neither workflow needs an emulator/simulator or a MAUI source build to *detect* a leak — the leak signature is proven by a `dotnet test` against the **shipped `Microsoft.Maui.Controls` NuGet package** on the plain library TFM, so detection runs on a standard GitHub-hosted runner. Only 4 files are added — the workflow source (`.md`) and its compiled artifact (`.lock.yml`) for each: ``` .github/workflows/daily-leak-hunter.md + .lock.yml .github/workflows/leak-fixer.md + .lock.yml ``` --- ## 1. `daily-leak-hunter` — the scanner (files `[leak-scan]` issues) **Trigger:** `schedule: every 12h` + manual (`workflow_dispatch`). Files **only empirically-proven `[leak-scan]` issues** — up to **8 per run**. 1. **Sweeps every focus area** of the managed surface (`src/Core/src`, `src/Controls/src/Core`, `src/Essentials/src`) in one run — it does not stop after the first find. 2. Each focus area is **seeded with proven MAUI leak signatures** (from the community's known-leak catalog) so the hunt targets where leaks actually live: shared publisher → strong `CollectionChanged`/`PropertyChanged` (e.g. `Picker.ItemsSource`, `TableRoot`, `SelectedItems`, `GradientStops`), shared `ICommand` → `CanExecuteChanged` (`ListView.RefreshCommand`, `SwipeItemView.Command`, `BackButtonBehavior.Command`), VSM state triggers → `DeviceDisplay.MainDisplayInfoChanged`, and static/`ResourceDictionary` roots. 3. For each candidate it writes a standalone control / leaky / mitigation xUnit repro (referencing the shipped package, no source build) and measures retention with `WeakReference` + a forced GC. All candidates go in **one test project** (one `[Fact]` each, one restore/run). 4. It files a `[leak-scan]` issue **only for each leak the test confirms** — the leaky scenario retains while **both** the control and the mitigation release. A false positive is treated as worse than a quiet run; unconfirmed candidates are dropped. **If a run proves nothing, it files nothing** (no fallback mode). **De-dup & safety:** skips a leak already covered by one of *its own* open `[leak-scan]` issues; never touches product or test code (read-only + `create-issue`); `noop: report-as-issue: false`. --- ## 2. `leak-fixer` — the fixer (opens draft `[leak-fix]` PRs) **Trigger:** `schedule: every 12h` + manual (`workflow_dispatch`). At most **one action per run**, with de-dup + a 3-attempt cap. It picks the **higher-priority** of two tracks: ### Track C — respond to review feedback (checked first) Before doing new work, it checks whether one of *its own* open `[leak-fix]` PRs (**hosted on this repo** — fork-hosted heads it can't push to are left for a human) has an **unaddressed `CHANGES_REQUESTED` review**. If so it works on that PR: **applies** the valid requested changes (pushes a commit, re-validated so the PR's own test still holds) and/or **posts a comment pushing back** on any request that is wrong, would regress behaviour, or asks for a mute. A loop-guard (act only on a review newer than the PR's last commit *and* the workflow's last comment) prevents re-processing the same review. ### Track A — `[leak-scan]` runtime leak → `[leak-fix]` PR For the selected proven leak it: 1. Writes a focused regression test in `Controls.Core.UnitTests`. 2. Builds MAUI **from source** and confirms the test **FAILS on `main`** — proving it catches the leak. 3. Implements the minimal, idiomatic managed fix — a weak subscription / teardown mirroring the existing `WeakEventManager` / `WeakNotify*Proxy` patterns. 4. Rebuilds and confirms the **same test now PASSES**, no neighbouring regressions. 5. Opens a **draft `[leak-fix]` PR** with `Fixes #N` and the red→green evidence. ### Safety - Enforces red→green **in both directions** — a fix without a demonstrated failing-then-passing test is rejected. - **Never** mutes / skips / disables a test; rejects its own attempt if the only thing that goes green is a mute — **including when a review asks for one** (it pushes back with a comment instead). - **One action per run** — Track C review-response *or* one new Track A `[leak-fix]` PR, never both. - Writes only via scoped safe-outputs (agent job is read-only); PR branches are limited to `leak-fix/**`, changes to managed `src/**` (+ `PublicAPI.Unshipped.txt`). - Skips cleanly if already fixed on `main`, out of scope (needs a device fix), or attempt-capped. --- ## Validated end-to-end on **dotnet/maui itself** (pre-merge) Both workflows were executed on this repo's own Actions infrastructure (via short-lived `push`-triggered test branches, since `workflow_dispatch` only lights up once a workflow is on `main`). Real outputs, all on **dotnet/maui**: - **Multi-leak in one run** — a single hunter run filed **5** distinct proven `[leak-scan]` issues via the catalog-seeded sweep: dotnet#36343 `SwipeItemView.Command`, dotnet#36344 `ListView.RefreshCommand`, dotnet#36345 `Shell BackButtonBehavior.Command` (all `ICommand.CanExecuteChanged`), dotnet#36346 `Picker.ItemsSource`, dotnet#36347 `IndicatorView.ItemsSource` (`CollectionChanged`). dotnet#36344 and dotnet#36345 are leaks not previously tracked upstream. - **Proven-only** — a later run filed `[leak-scan]` dotnet#36350 `CollectionView.SelectedItems` and (correctly) **nothing else**, having de-duped the already-filed leaks. - **Track A fix** — dotnet#36309 `[leak-fix] Fix ResourceDictionary MergedDictionaries memory leak` (regression test red→green + managed fix). - **Track C review-response** — on dotnet#36253 the fixer independently re-read the code, agreed with the reviewer's findings, validated the reviewer's `WeakEventManager` approach (24/24, red→green), and posted the concrete fix. --- ## Notes for reviewers - Everything the workflows file/open is **AI-generated** and clearly labelled as such; treat them as high-quality drafts for human review. - The workflows are self-contained (`.md` source + compiled `.lock.yml`); merging them enables the `schedule` triggers on `main`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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!
Note
🤖 AI-generated PR. These workflows, and this description, were produced with AI assistance. Please review carefully before merging.
Summary
Adds two gh-aw agentic workflows that proactively find and fix managed, cross-platform memory leaks in MAUI — turning the AI-driven leak-hunting flow several contributors already run by hand (scan → repro → measure → file → fix) into a repeatable, reviewable pipeline.
Design principle: only empirically-proven leaks. The scanner files an issue only when a plain
dotnet testdemonstrates the leak; the fixer opens a PR only with a red→green regression test proving the fix. There is no "you're missing a test" / coverage-gap mode — every artifact is backed by a passing/failing test.Key property: neither workflow needs an emulator/simulator or a MAUI source build to detect a leak — the leak signature is proven by a
dotnet testagainst the shippedMicrosoft.Maui.ControlsNuGet package on the plain library TFM, so detection runs on a standard GitHub-hosted runner.Only 4 files are added — the workflow source (
.md) and its compiled artifact (.lock.yml) for each:1.
daily-leak-hunter— the scanner (files[leak-scan]issues)Trigger:
schedule: every 12h+ manual (workflow_dispatch). Files only empirically-proven[leak-scan]issues — up to 8 per run.src/Core/src,src/Controls/src/Core,src/Essentials/src) in one run — it does not stop after the first find.CollectionChanged/PropertyChanged(e.g.Picker.ItemsSource,TableRoot,SelectedItems,GradientStops), sharedICommand→CanExecuteChanged(ListView.RefreshCommand,SwipeItemView.Command,BackButtonBehavior.Command), VSM state triggers →DeviceDisplay.MainDisplayInfoChanged, and static/ResourceDictionaryroots.WeakReference+ a forced GC. All candidates go in one test project (one[Fact]each, one restore/run).[leak-scan]issue only for each leak the test confirms — the leaky scenario retains while both the control and the mitigation release. A false positive is treated as worse than a quiet run; unconfirmed candidates are dropped. If a run proves nothing, it files nothing (no fallback mode).De-dup & safety: skips a leak already covered by one of its own open
[leak-scan]issues; never touches product or test code (read-only +create-issue);noop: report-as-issue: false.2.
leak-fixer— the fixer (opens draft[leak-fix]PRs)Trigger:
schedule: every 12h+ manual (workflow_dispatch). At most one action per run, with de-dup + a 3-attempt cap. It picks the higher-priority of two tracks:Track C — respond to review feedback (checked first)
Before doing new work, it checks whether one of its own open
[leak-fix]PRs (hosted on this repo — fork-hosted heads it can't push to are left for a human) has an unaddressedCHANGES_REQUESTEDreview. If so it works on that PR: applies the valid requested changes (pushes a commit, re-validated so the PR's own test still holds) and/or posts a comment pushing back on any request that is wrong, would regress behaviour, or asks for a mute. A loop-guard (act only on a review newer than the PR's last commit and the workflow's last comment) prevents re-processing the same review.Track A —
[leak-scan]runtime leak →[leak-fix]PRFor the selected proven leak it:
Controls.Core.UnitTests.main— proving it catches the leak.WeakEventManager/WeakNotify*Proxypatterns.[leak-fix]PR withFixes #Nand the red→green evidence.Safety
[leak-fix]PR, never both.leak-fix/**, changes to managedsrc/**(+PublicAPI.Unshipped.txt).main, out of scope (needs a device fix), or attempt-capped.Validated end-to-end on dotnet/maui itself (pre-merge)
Both workflows were executed on this repo's own Actions infrastructure (via short-lived
push-triggered test branches, sinceworkflow_dispatchonly lights up once a workflow is onmain). Real outputs, all on dotnet/maui:[leak-scan]issues via the catalog-seeded sweep: [leak-scan] SwipeItemView.Command leaks the control via strong ICommand.CanExecuteChanged (no weak proxy, no unload teardown) #36343SwipeItemView.Command, [leak-scan] ListView.RefreshCommand leaks the control via strong ICommand.CanExecuteChanged (no weak proxy, no unload teardown) #36344ListView.RefreshCommand, [leak-scan] Shell BackButtonBehavior.Command leaks via strong ICommand.CanExecuteChanged (no weak proxy, no detach teardown) #36345Shell BackButtonBehavior.Command(allICommand.CanExecuteChanged), [leak-scan] Picker.ItemsSource leaks the control via strong CollectionChanged subscription (no weak proxy, no unload teardown) #36346Picker.ItemsSource, [leak-scan] IndicatorView.ItemsSource leaks the control via strong CollectionChanged subscription (no weak proxy, no unload tear [Content truncated due to length] #36347IndicatorView.ItemsSource(CollectionChanged). [leak-scan] ListView.RefreshCommand leaks the control via strong ICommand.CanExecuteChanged (no weak proxy, no unload teardown) #36344 and [leak-scan] Shell BackButtonBehavior.Command leaks via strong ICommand.CanExecuteChanged (no weak proxy, no detach teardown) #36345 are leaks not previously tracked upstream.[leak-scan][leak-scan] CollectionView.SelectedItems leaks the control via SelectionList's strong CollectionChanged subscription (no weak pr [Content truncated due to length] #36350CollectionView.SelectedItemsand (correctly) nothing else, having de-duped the already-filed leaks.[leak-fix] Fix ResourceDictionary MergedDictionaries memory leak(regression test red→green + managed fix).WeakEventManagerapproach (24/24, red→green), and posted the concrete fix.Notes for reviewers
.mdsource + compiled.lock.yml); merging them enables thescheduletriggers onmain.