Merge pull request #232 from PhysShell/claude/plans-tasks-review-u77dfs

fix(extractor): curated app-scoped source exemption for the Application subscriber (Closes #228)

Author: PhysShell

.github/workflows/ci.yml

@@ -212,6 +212,7 @@ jobs:
             frontend/roslyn/samples/UsingFieldAcquisitionSample.cs \
             frontend/roslyn/samples/TemplatePartLocalCaptureSample.cs \
             frontend/roslyn/samples/EmptyDisposeSample.cs \
+            frontend/roslyn/samples/AppScopedSourceSample.cs \
             -o "$RUNNER_TEMP/facts.json"
           cat "$RUNNER_TEMP/facts.json"
       - name: Check facts through the core
@@ -849,7 +850,35 @@ jobs:
           # Codex P2: an empty SYNC Dispose but a real DisposeAsync (a `*Reader` here) must still leak.
           echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ar'.*'AsyncReader'" \
             || { echo "FAIL: a type with a real DisposeAsync (empty sync Dispose) must still leak (#225)"; exit 1; }
-          echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) + #218 DP old->new subscription rotation (silent; controls flagged) + #225 empty-Dispose local exemption (silent; controls flagged) at the C# location"
+          # issue #228 — an Application-derived subscriber on a CURATED app-scoped
+          # resolver result (PaletteHelper.GetThemeManager) with a method-group handler
+          # of the App class itself must be SILENT: the source is process-lived (bound
+          # to the app's own state), so nothing is promoted. Both receiver forms — an
+          # `is`-pattern local (App) and the direct invocation (DirectApp).
+          if echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+:.*('App'|'DirectApp')"; then
+            echo "FAIL: a curated app-scoped subscription inside the Application was wrongly reported (#228)"; exit 1
+          fi
+          # ...and the exemption must NOT over-widen — three controls STAY flagged:
+          # (1) the SAME curated shape from a NON-Application class (the subscriber
+          # gate stays clsIsApp, byte-for-byte);
+          echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'NotAnApp'" \
+            || { echo "FAIL: expected OWN001 on the non-Application subscriber (curated source alone must not exempt)"; exit 1; }
+          # (2) App + curated source, but a LAMBDA handler capturing a local — the
+          # exact hole that sank the rejected clsIsStatic broadening;
+          echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'LambdaApp'" \
+            || { echo "FAIL: expected OWN001 on the lambda-handler subscription inside the App (capture hole)"; exit 1; }
+          # (3) App + method-group handler, but a NON-curated resolver;
+          echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'CuratedOnlyApp'" \
+            || { echo "FAIL: expected OWN001 on the non-curated resolver source inside the App"; exit 1; }
+          # (4, Codex P2) the method group is qualified with ANOTHER instance of the
+          # same App type — right ContainingType, wrong delegate target;
+          echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'TwinApp'" \
+            || { echo "FAIL: expected OWN001 when the handler target is another instance, not this"; exit 1; }
+          # (5, Codex P2) the local is REASSIGNED to an injected publisher after the
+          # curated initializer — the stale declaration binding must not exempt.
+          echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'ReassignApp'" \
+            || { echo "FAIL: expected OWN001 when the resolver-bound local is reassigned before the +="; exit 1; }
+          echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) + #218 DP old->new subscription rotation (silent; controls flagged) + #225 empty-Dispose local exemption (silent; controls flagged) + #228 curated app-scoped source in App (silent; controls flagged) at the C# location"
       - name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2)
         run: |
           # Path-sensitive flow analysis of local IDisposables — bugs the flat D1

docs/notes/oracle-known-fps.md

@@ -308,6 +308,27 @@ which we have no reliable signal for. So those two findings stay in
 extractor rule. An in-code `ANTI-PATTERN` comment at the exemption site warns against
 re-adding `|| clsIsStatic`.
 
+### What DID ship next to it (issue #228) — and why it is not the same thing
+
+The #201 sweep's MaterialDesign `App.xaml.cs` FP (an `App : Application` subscribing
+to `PaletteHelper().GetThemeManager()`, an app-scoped instance that is process-lived
+without being a literal `static` member) is cleared by a narrowing that inverts every
+property that made `clsIsStatic` unsound:
+
+- the **subscriber** gate is unchanged — byte-for-byte the existing `clsIsApp`
+  ("the subscriber IS the process-lived object", the property `clsIsStatic` lacked);
+- the **source** check loosens only to a **curated allowlist** of resolver methods
+  whose result is verified app-scoped (`PaletteHelper.GetThemeManager`, verified at
+  `PaletteHelper.cs:22-26`) — grown one confirmed sibling at a time, like the #223
+  weak-event list, never inferred;
+- the **handler** must be a method group of the App class itself — a lambda is
+  rejected outright, so the "captures a shorter-lived local and pins it" hole that
+  sank `clsIsStatic` (and would NOT have been closed by it anyway) cannot pass.
+
+Pinned by `AppScopedSourceSample.cs`: two silent positives (pattern-local and direct
+receiver) and three flagged controls (non-App subscriber, lambda handler, non-curated
+resolver), each edge of the gate.
+
 ## How the baseline stays honest
 
 - **Matched by name, not line** — `(repo, file-basename, OWN code,

frontend/roslyn/OwnSharp.Extractor/Program.cs

@@ -895,6 +895,120 @@ static bool IsProcessLivedApplication(TypeDeclarationSyntax cls)
         && cls.Modifiers.Any(m => m.IsKind(SyntaxKind.PartialKeyword));
 }
 
+// P-004 / issue #228: the CURATED resolver allowlist whose call RESULT is app-scoped
+// (bound to Application-owned state, hence process-lived) without being a literal
+// `static` member. One entry per CONFIRMED real-world sibling — same policy as the
+// #223 weak-event allowlist, never an inference. Matched by (containing-type simple
+// name, method name), mirroring the [OwnIgnore] simple-name precedent: the declaring
+// package usually does not resolve on the Linux runner.
+//   - MaterialDesign PaletteHelper.GetThemeManager(): returns the IThemeManager bound
+//     to the app's own merged ResourceDictionary (PaletteHelper.cs:22-26, verified in
+//     the issue #201 sweep — MahMaterialDragablzMashUp App.xaml.cs FP).
+static bool IsAppScopedResolver(IMethodSymbol sym) =>
+    (sym.ContainingType?.Name, sym.Name) is ("PaletteHelper", "GetThemeManager");
+
+// #228: does this `+=` receiver resolve (directly, or through a local bound by a
+// `var x = ...` initializer or an `is`-pattern designation) to the RESULT of a curated
+// app-scoped resolver call? Any unprovable step returns false — the subscription then
+// keeps today's honest "injected" warning, never the other way around.
+static bool IsCuratedAppScopedSource(ExpressionSyntax left, SemanticModel model)
+    => left is MemberAccessExpressionSyntax m
+       && ResolvesToAppScopedCall(m.Expression, model, depth: 0);
+
+static bool ResolvesToAppScopedCall(ExpressionSyntax expr, SemanticModel model, int depth)
+{
+    if (depth > 3)
+        return false;
+    expr = StripCasts(expr);
+    if (expr is InvocationExpressionSyntax inv)
+    {
+        var info = model.GetSymbolInfo(inv);
+        var sym = info.Symbol as IMethodSymbol
+                  ?? info.CandidateSymbols.OfType<IMethodSymbol>().FirstOrDefault();
+        return sym is not null && IsAppScopedResolver(sym);
+    }
+    if (model.GetSymbolInfo(expr).Symbol is not ILocalSymbol local)
+        return false;
+    // The binding is only trustworthy if the local still HOLDS it at the `+=`:
+    // a reassignment anywhere in the enclosing member (or a ref/out escape that
+    // could rebind it) makes the initializer stale — no exemption (Codex P2).
+    if (!IsNeverReassigned(local, model))
+        return false;
+    foreach (var r in local.DeclaringSyntaxReferences)
+        switch (r.GetSyntax())
+        {
+            // var tm = helper.GetThemeManager();
+            case VariableDeclaratorSyntax { Initializer.Value: { } init }
+                when ResolvesToAppScopedCall(init, model, depth + 1):
+                return true;
+            // helper.GetThemeManager() is { } tm   /   is ThemeManagerLike tm
+            case SingleVariableDesignationSyntax des
+                when des.Ancestors().OfType<IsPatternExpressionSyntax>().FirstOrDefault()
+                     is { Expression: { } scrutinee }
+                     && ResolvesToAppScopedCall(scrutinee, model, depth + 1):
+                return true;
+        }
+    return false;
+}
+
+// #228 (Codex P2): the declaration-site binding proves the local's value at the
+// `+=` only if nothing rebinds it. Conservative whole-member scan: ANY assignment
+// whose LHS is this local, or ANY `ref`/`out` argument passing it, kills the proof
+// (even one after the subscription — cheaper than flow order and precision-safe:
+// the worst case is keeping today's honest warning).
+static bool IsNeverReassigned(ILocalSymbol local, SemanticModel model)
+{
+    foreach (var r in local.DeclaringSyntaxReferences)
+    {
+        var scope = r.GetSyntax().Ancestors().FirstOrDefault(n =>
+            n is MethodDeclarationSyntax or ConstructorDeclarationSyntax
+              or AccessorDeclarationSyntax or LocalFunctionStatementSyntax
+              or AnonymousFunctionExpressionSyntax);
+        if (scope is null)
+            return false;
+        foreach (var asg in scope.DescendantNodes().OfType<AssignmentExpressionSyntax>())
+            if (SymbolEqualityComparer.Default.Equals(
+                    model.GetSymbolInfo(asg.Left).Symbol, local))
+                return false;
+        foreach (var arg in scope.DescendantNodes().OfType<ArgumentSyntax>())
+            if (!arg.RefOrOutKeyword.IsKind(SyntaxKind.None)
+                && SymbolEqualityComparer.Default.Equals(
+                       model.GetSymbolInfo(arg.Expression).Symbol, local))
+                return false;
+    }
+    return true;
+}
+
+// #228: the handler must be a METHOD GROUP declared on the subscribing class itself —
+// its delegate target is then the App singleton (already process-lived, nothing to
+// promote). A lambda/anonymous method is rejected outright: it may capture an
+// enclosing LOCAL, and pinning that local to the app's lifetime is exactly the leak
+// the rejected `clsIsStatic` broadening would have swallowed (oracle-known-fps.md,
+// "Rejected approaches").
+static bool IsOwnMethodGroupHandler(ExpressionSyntax right, SemanticModel model,
+                                    INamedTypeSymbol? cls)
+{
+    if (cls is null)
+        return false;
+    // The delegate TARGET must be `this` — a bare identifier (implicit this) or an
+    // explicit `this.X`. A method group qualified with ANOTHER instance of the same
+    // App-derived type (`otherApp.OnTheme`) has the right ContainingType but pins
+    // that other instance, not the process-lived singleton (Codex P2).
+    var receiverIsThis = right switch
+    {
+        IdentifierNameSyntax => true,
+        MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax } => true,
+        _ => false,
+    };
+    if (!receiverIsThis)
+        return false;
+    var info = model.GetSymbolInfo(right);
+    var sym = info.Symbol as IMethodSymbol
+              ?? info.CandidateSymbols.OfType<IMethodSymbol>().FirstOrDefault();
+    return sym is not null
+        && SymbolEqualityComparer.Default.Equals(sym.ContainingType, cls);
+}
+
 // P-004 WPF MVVM ownership: a field read from `this.DataContext`, optionally through
 // an `as`/cast (`DataContext as VM`, `(VM)DataContext`). Combined with a view whose
 // own XAML CONSTRUCTS its DataContext, such a field is the view's owned view-model.
@@ -4063,6 +4177,19 @@ or ImplicitObjectCreationExpressionSyntax
                 // write-up: docs/notes/oracle-known-fps.md → "Rejected approaches".
                 if (!isTimer && source == "static" && clsIsApp)
                     continue;
+                // P-004 / issue #228: the same `clsIsApp` subscriber, but the source is an
+                // APP-SCOPED RESOLVER RESULT (curated: PaletteHelper.GetThemeManager) rather
+                // than a literal static member — genuinely process-lived, just reached through
+                // a call, so SubscriptionSourceKind honestly says "injected". This loosens
+                // ONLY the source check; the subscriber gate stays byte-for-byte `clsIsApp`,
+                // and the handler must be a METHOD GROUP of the App class itself — a lambda
+                // is rejected because it may capture an enclosing local, the exact hole that
+                // sank the `clsIsStatic` broadening above (see that comment + the "Rejected
+                // approaches" write-up; this narrowing is documented alongside it).
+                if (!isTimer && source == "injected" && clsIsApp
+                    && IsOwnMethodGroupHandler(a.Right, model, clsSymbol)
+                    && IsCuratedAppScopedSource(a.Left, model))
+                    continue;
                 // P-004 / issue #199: a NON-RETAINING handler on a static (process-lived)
                 // source promotes nothing, so OWN014's premise ("the subscriber is pinned
                 // for the source's life") does not hold -> silent. A static METHOD handler

frontend/roslyn/samples/AppScopedSourceSample.cs

@@ -0,0 +1,142 @@
+// issue #228 — an Application-derived subscriber whose event source is an APP-SCOPED
+// RESOLVER RESULT (curated: PaletteHelper.GetThemeManager), not a literal static member.
+// The source is process-lived (bound to the app's own state), so the subscription
+// promotes nothing — but SubscriptionSourceKind honestly tiers a call-result receiver
+// "injected", which used to warn. Real-world shape: MaterialDesignInXamlToolkit
+// MahMaterialDragablzMashUp App.xaml.cs:10,22 (found by the issue #201 oracle sweep).
+//
+// The exemption is deliberately NARROW (see the rejected `clsIsStatic` broadening in
+// docs/notes/oracle-known-fps.md): subscriber must be the Application class itself,
+// the resolver must be on the curated list, and the handler must be a METHOD GROUP of
+// that class — three negative controls below pin each edge.
+using System;
+
+namespace OwnSamples.AppScoped
+{
+    // Stand-in for System.Windows.Application: IsProcessLivedApplication matches the
+    // base NAME syntactically (WPF assemblies do not resolve on the Linux runner).
+    public class Application { }
+
+    public class ThemeManagerLike
+    {
+        public event EventHandler? ThemeChanged;
+        public void Raise() => ThemeChanged?.Invoke(this, EventArgs.Empty);
+    }
+
+    // Stand-in for MaterialDesignThemes.Wpf.PaletteHelper — the curated resolver,
+    // matched by (containing-type, method) = (PaletteHelper, GetThemeManager).
+    public class PaletteHelper
+    {
+        static readonly ThemeManagerLike Manager = new();
+        public ThemeManagerLike? GetThemeManager() => Manager;
+    }
+
+    // Same shape, NON-curated name -> never exempted.
+    public class OtherHelper
+    {
+        static readonly ThemeManagerLike Manager = new();
+        public ThemeManagerLike? GetOtherService() => Manager;
+    }
+
+    // POSITIVE (silent): App + curated resolver via `is`-pattern local + method-group
+    // handler of the App class — the exact MaterialDesign shape.
+    public class App : Application
+    {
+        public void OnStartup()
+        {
+            var helper = new PaletteHelper();
+            if (helper.GetThemeManager() is { } themeManager)
+                themeManager.ThemeChanged += ThemeManager_ThemeChanged;   // silent (#228)
+        }
+
+        void ThemeManager_ThemeChanged(object? sender, EventArgs e) { }
+    }
+
+    // POSITIVE (silent): the direct-invocation receiver form, no intermediate local.
+    public class DirectApp : Application
+    {
+        public void OnStartup()
+        {
+            new PaletteHelper().GetThemeManager()!.ThemeChanged += OnTheme;   // silent (#228)
+        }
+
+        void OnTheme(object? sender, EventArgs e) { }
+    }
+
+    // CONTROL 1 (flagged): the SAME curated shape from a class that is NOT the
+    // Application — the subscriber gate must stay `clsIsApp`, byte-for-byte.
+    public class NotAnApp
+    {
+        public void Wire()
+        {
+            var helper = new PaletteHelper();
+            if (helper.GetThemeManager() is { } themeManager)
+                themeManager.ThemeChanged += OnTheme;                     // OWN001 warning
+        }
+
+        void OnTheme(object? sender, EventArgs e) { }
+    }
+
+    // CONTROL 2 (flagged): App + curated source, but the handler is a LAMBDA capturing
+    // an enclosing local — pinning that local to app lifetime is the exact hole that
+    // sank the `clsIsStatic` broadening; a lambda must never pass the handler gate.
+    public class LambdaApp : Application
+    {
+        public void OnStartup()
+        {
+            var counter = new int[1];
+            if (new PaletteHelper().GetThemeManager() is { } themeManager)
+                themeManager.ThemeChanged += (s, e) => counter[0]++;      // OWN001 warning
+        }
+    }
+
+    // CONTROL 3 (flagged): App + method-group handler, but a NON-curated resolver —
+    // membership is a curated allowlist, not "any call result inside App".
+    public class CuratedOnlyApp : Application
+    {
+        public void OnStartup()
+        {
+            if (new OtherHelper().GetOtherService() is { } service)
+                service.ThemeChanged += OnTheme;                          // OWN001 warning
+        }
+
+        void OnTheme(object? sender, EventArgs e) { }
+    }
+
+    // CONTROL 4 (flagged, Codex P2): the method group is qualified with ANOTHER
+    // instance of the same App-derived type — right ContainingType, wrong delegate
+    // TARGET: the process-lived source pins `_twin`, not `this`.
+    public class TwinApp : Application
+    {
+        readonly TwinApp? _twin;
+
+        public TwinApp(TwinApp? twin) => _twin = twin;
+
+        public void OnStartup()
+        {
+            if (new PaletteHelper().GetThemeManager() is { } themeManager)
+                themeManager.ThemeChanged += _twin!.OnTheme;              // OWN001 warning
+        }
+
+        public void OnTheme(object? sender, EventArgs e) { }
+    }
+
+    // CONTROL 5 (flagged, Codex P2): the local STARTS as the curated resolver result
+    // but is REASSIGNED to an injected publisher before the `+=` — the declaration-site
+    // binding is stale, so the exemption must not trust it.
+    public class ReassignApp : Application
+    {
+        readonly ThemeManagerLike _injectedBus;
+
+        public ReassignApp(ThemeManagerLike bus) => _injectedBus = bus;
+
+        public void OnStartup()
+        {
+            var tm = new PaletteHelper().GetThemeManager()!;
+            tm = _injectedBus;
+            tm.ThemeChanged += OnTheme;                                   // OWN001 warning
+        }
+
+        void OnTheme(object? sender, EventArgs e) { }
+    }
+}