Add AI memory-leak scanner + fixer agentic workflows#36276
Closed
kubaflo wants to merge 6 commits into
Closed
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>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36276Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36276" |
|
Cool concept! Would be also interesting to create one for performance! |
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>
jonathanpeppers
left a comment
Member
There was a problem hiding this comment.
Could we revive this PR?
It wouldn't need AI, but you could use AI to fix all the warnings that the deterministic analyzer finds.
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>
Collaborator
Author
mattleibow
added a commit
to mono/SkiaSharp
that referenced
this pull request
Jul 6, 2026
Add memory-leak-fixer skill + self-testing agentic workflow (#4330) Context: dotnet/maui#36276 Adapts MAUI's AI leak-scanner idea to SkiaSharp's actual leak surface — native ownership/disposal correctness (undisposed SKObject handles, wrong `owns:` flags, C-API ref-count mismatches, same-instance double-dispose, unremoved view/handler subscriptions, `fixed`-pointer lifetime) rather than MAUI's managed view-retention leaks. Everything is scoped to managed C# (binding/**, source/SkiaSharp.Views*/**); upstream Skia under externals/skia/** is out of scope. Two deliverables plus registration: - .agents/skills/memory-leak-fixer/SKILL.md (+ references/types-of-leaks.md): a reusable scan -> prove -> fix methodology grounded in documentation/dev/memory-management.md, with an 11-family leak catalogue as the single source of truth (each family carries a description, why it leaks, a generic example, the idiomatic fix, anti-patterns, and where to look). Golden rule: every fix is proven red->green — the regression test must fail without the fix and pass with it — before anything ships. Tests are never weakened or muted; generated files and upstream Skia are never touched. - .github/workflows/memory-leak-fixer.md (+ compiled .lock.yml): a gh-aw workflow (Copilot / claude-opus-4.8) that runs the skill on a 12h schedule. Every run does the same thing — hunt one leak (round-robin family focus by day-of-year + hour), prove it, fix it, then emit a linked pair of safe outputs: a `[memory-leak]` finding issue and a draft fix PR whose body ends with `Fixes #<temporary_id>`. gh-aw resolves the temporary id to the real issue number within the same run, so merging the PR auto-closes the finding. The only knob is `dry_run` (do everything but open nothing). - AGENTS.md: registers the skill as /memory-leak-fixer in the command and issue classification tables. The workflow self-tests: any PR that edits the workflow or skill re-runs the whole pipeline in forced DRY-RUN, emitting only a `noop` (never a real issue/PR), so the prompt/skill can be iterated on and watched live without side effects. Verified across several dry-runs — all green, all noop-only. The scanner already surfaced a genuine finding on otherwise-hardened code: SKRegion.SpanIterator does not root its parent SKRegion the way its RectIterator/ClipIterator siblings do, a real use-after-free risk. It is deliberately left unfixed here so the workflow has a real target to find and fix on its first scheduled run. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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. They mirror the AI-driven leak-hunting flow several contributors already run by hand (scan → repro → measure → file → fix) and turn it into a repeatable, reviewable pipeline.
Key property: neither the detection pass nor the fix-validation pass needs an emulator/simulator. The leak signature is proven by a plain
dotnet teston the .NET library TFM, so both run 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 issues)Trigger:
schedule: every 12h+ manual (workflow_dispatch). Files at most one issue per run.Pass A — runtime leak hunt →
[leak-scan]issuesrc/Core/src,src/Controls/src/Core,src/Essentials/src).Microsoft.Maui.ControlsNuGet package (no source build), and measures retention withWeakReference+ a forced GC.[leak-scan]issue only if the leak is empirically confirmed — the leaky scenario retains while both the control and the mitigation release. A false positive is treated as worse than a quiet run, so unconfirmed candidates are dropped.Pass B — leak-test coverage-gap scan →
[leak-test-gap]issueRuns only when Pass A files nothing (the expected case once the runtime surface is hardened). It statically diffs MAUI's concrete control surface against its existing memory-leak test coverage (
src/Controls/tests/DeviceTests/Memory/MemoryTests.csHandlerDoesNotLeak[InlineData]list + per-control*DoesNotLeaktests + the AppiumMemoryTests) and files a[leak-test-gap]issue proposing the precise missing test, with a copy-paste-ready stub. This keeps every run productive and closes the holes through which future leaks slip in untested.De-dup & safety
[leak-scan]/[leak-test-gap]issues.create-issue).noop: report-as-issue: false— quiet runs don't create noise.2.
leak-fixer— the fixer (opens draft PRs)Trigger:
schedule: every 12h+ manual (workflow_dispatch). At most one action per run, with de-dup + a 3-attempt cap. Each run it picks the highest-priority of three tracks:Track C — respond to review feedback (HIGHEST priority, checked first)
Before doing new work, it checks whether one of its own open
[leak-fix]/[leak-test]PRs has an unaddressedCHANGES_REQUESTEDreview. If so, it works on that PR: applies the valid requested changes (pushes a commit to the PR branch, 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 (only act on a review newer than the PR's last commit and the workflow's last comment) means it never re-processes the same review. Uses thepush-to-pull-request-branch+add-commentsafe-outputs, scoped byrequired-title-prefix: "[leak-"to its own PRs. Example this is built for: MauiBot requested changes on #36253 (a real ❌ duplicate-MergedDictionariesunsubscribe bug + suggestions) — Track C applies the sound fixes and replies re: any it disagrees with.Track A —
[leak-scan](runtime leak →[leak-fix]PR)For the selected leak it:
Controls.Core.UnitTests.main— proving it catches the leak.WeakEventManager/WeakNotify*Proxy/BindableLayoutControllerpatterns.[leak-fix]PR withFixes #Nand the red→green evidence.Track B —
[leak-test-gap](missing coverage →[leak-test]PR)For a coverage gap it adds the memory-leak test the issue proposes (test-only, no product change): a runner-run
Controls.Core.UnitTestsguard and/or aHandlerDoesNotLeak[InlineData]inMemoryTests.cs. It validates as far as the runner allows — aCore.UnitTestsguard must build and PASS onmain(device-test[InlineData]s can't run on the runner, so they're validated structurally and execute onmaui-pr-devicetests) — then opens a draft[leak-test]PR. If a runnable guard unexpectedly fails, that's a real live leak → the fixer escalates to Track A and writes the fix instead.Safety
[leak-.main, out of scope (needs a device fix), or attempt-capped.Examples already produced
These are real, human-reviewable outputs the workflows generated automatically.
End-to-end, running on dotnet/maui itself (pre-merge validation)
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). Every run succeeded; the outputs below are all on dotnet/maui:[leak-test-gap] FlexLayout has no HandlerDoesNotLeak test[leak-test] Add memory-leak coverage for FlexLayout(test-only: aCore.UnitTestsguard and the device[InlineData])[leak-scan] Rooted ResourceDictionary merged into a transient element[leak-fix] Fix ResourceDictionary MergedDictionaries memory leak(real product fix inWeakEventProxy.cs+ResourceDictionary.cs, source-built + red→green)[leak-test-gap] No test asserts Shell itself is releasedThis exercised all four paths on the real repo: Pass A (find leak), Pass B (find test
gap), Track A (fix leak), Track B (add coverage) — with zero run failures.
Fix PRs the
leak-fixeropened earlier (fork-built, promoted here)Each carries a regression test that fails on
mainwithout the fix and passes with it, plus the idiomatic weak-proxy fix. All are the same real leak family — a shared/long-lived value retaining a control via a non-weak subscription with no unload teardown:IndicatorView.ItemsSourceResourceDictionary.ValuesChangedBorder.StrokeDashArrayPath.Data/Path.RenderTransformPicker.ItemsSource(#36251 and #36272 are the issues this pipeline filed; #35775 / #35492 / #35860 were pre-existing reports the fixer linked to instead of duplicating.)
Raw scanner output (on the validation fork)
[leak-scan](Pass A) — e.g. [leak-scan] Repeating animations leak the animated VisualElement via the static AnimationExtensions.s_animations dictionary kubaflo/maui#86 (repeating animations), [leak-scan] SwipeItemView.Command leaks the bound view via non-weak ICommand.CanExecuteChanged subscription with no teardown kubaflo/maui#88 (SwipeItemView.Command), [leak-scan] ListView.RefreshCommand leaks the ListView via non-weak ICommand.CanExecuteChanged subscription with no teardown kubaflo/maui#89 (ListView.RefreshCommand).[leak-test-gap](Pass B) — e.g. [leak-test-gap] FlexLayout has no memory-leak (HandlerDoesNotLeak) test kubaflo/maui#95 (FlexLayouthas noHandlerDoesNotLeaktest), [leak-test-gap] AbsoluteLayout has no memory-leak (HandlerDoesNotLeak) test kubaflo/maui#96 (AbsoluteLayout), [leak-test-gap] SwipeItemView has no memory-leak (DoesNotLeak) device test kubaflo/maui#103 (SwipeItemView).Across the fork run the hunter filed ~18 confirmed
[leak-scan]+ ~10[leak-test-gap]issues, and the fixer converted several into the validated PRs above.Setup required — essentially none
These workflows add no new secrets, environments, or labels. They reuse the exact infrastructure the repo's existing agentic workflows already run on (
ci-status-main,ci-status-fix,daily-repo-status,agentic-labeler,copilot-evaluate-tests,rerun-review-scanner):environment: gh-aw-agents— the same environment all six current agentic workflows use. The referenced secret set is byte-identical toci-status-fix(COPILOT_GITHUB_TOKEN,GH_AW_*,GITHUB_TOKEN). No new secret.agentic-workflowslabelcreate-issue/create-pull-requestsafe-outputs.ci-status-fixalready opens PRs on this repo, so "Allow GitHub Actions to create and approve pull requests" is already on. Nothing to toggle.dotnetecosystem in each file allowlists nuget.org (hunter) and the dnceng feedspkgs.dev.azure.com(fixer's from-source restore). No repo/org change.So on this repo they're ready to run as soon as they land on the default branch — no environment or config changes needed. (The only place any setup would be required is a fork without the
gh-aw-agentsenvironment, where you'd add a singleCOPILOT_GITHUB_TOKENrepo secret.)Triggering / editing
gh workflow run "daily-leak-hunter.lock.yml"/"leak-fixer.lock.yml". The fixer acceptsissue_number(scope to one issue) anddry_run(do everything, emit no PR) inputs..lock.ymlis what Actions runs; the.mdis the source. After any.mdedit rungh aw compile <name>and commit the regenerated.lock.yml. Compiled with the repo's pinned gh-aw v0.79.8, soactions-lock.jsonis unchanged.Conventions
Adapted to match the existing
ci-status-*agentic workflows:environment: gh-aw-agents,if: github.repository == 'dotnet/maui',agentic-workflowslabel,dotnetnetwork ecosystem, safe-output-scoped writes.Cadence (please tune)
The hunter is scheduled every 12h (matching
ci-status-main). Both are scheduled every 12h (matchingci-status-main/ci-status-fix) and also runnable manually. The fixer works through the hunter's backlog (leaks first, then coverage gaps), one draft PR per run, with de-dup + a 3-attempt cap. Happy to change either interval — whatever the team prefers.