feat(di): DI002 — scoped service captured weakly by a singleton (WeakReference is not a fix)

.github/workflows/ci.yml

@@ -317,7 +317,25 @@ jobs:
           nw=$(echo "$out" | grep -cE "DiCaptiveSample\.cs:[0-9]+:.*\[DI003\]")
           [ "$nw" = "1" ] \
             || { echo "FAIL: expected exactly 1 DI003 finding, got $nw"; exit 1; }
-          echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI003 (transient IDisposable captured by a singleton) at the C# location"
+          # P-006 DI002 (scoped service held by a singleton via WeakReference<T>, WARNING):
+          # the weak ref is the usual "fix" for a DI001 captive, but scoped AppDbContext is
+          # still root-resolved and app-lived — the lifetime contract is still violated. The
+          # weak edge is OFF the strong graph, so WeakCache is a DI002, NOT a 5th DI001 (the
+          # "exactly 4 DI001" count above proves it). A weak ref to a SINGLETON
+          # (WeakClockHolder -> Clock) is no mismatch -> silent.
+          echo "$out" | grep -qE "\[DI002\].*'WeakCache' weakly captures scoped service 'AppDbContext'" \
+            || { echo "FAIL: expected DI002 (WeakCache weakly captures scoped AppDbContext)"; exit 1; }
+          # a NULLABLE WeakReference<AppDbContext>? is the same weak captive — the `?` annotation
+          # is unwrapped, so the scoped service is still seen (CodeRabbit review on #63).
+          echo "$out" | grep -qE "\[DI002\].*'WeakCacheOpt' weakly captures scoped service 'AppDbContext'" \
+            || { echo "FAIL: expected DI002 on the nullable WeakReference (WeakCacheOpt)"; exit 1; }
+          nwk=$(echo "$out" | grep -cE "DiCaptiveSample\.cs:[0-9]+:.*\[DI002\]")
+          [ "$nwk" = "2" ] \
+            || { echo "FAIL: expected exactly 2 DI002 findings, got $nwk"; exit 1; }
+          if echo "$out" | grep -q "WeakClockHolder"; then
+            echo "FAIL: a weak ref to a singleton (WeakClockHolder) was wrongly flagged"; exit 1
+          fi
+          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) 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/di-captive-extractor.md

@@ -97,11 +97,29 @@ warning fires only where ownership is certain). Pinned end-to-end by `DiCaptiveS
 (`ConnectionWarmer` → transient `PooledConnection`) in the `wpf-extractor` CI job, and at the
 graph level by `tests/test_ownir.py`.
 
+## DI002 — scoped service captured *weakly* by a singleton (shipped)
+
+A singleton that holds a **scoped** service via **`WeakReference<T>`** is the usual "fix"
+for a DI001 captive — the weak reference stops the singleton pinning the scoped instance for
+the GC. But it does **not** fix the *lifetime contract*: the scoped service is still resolved
+from the root provider and lives for the application lifetime; the weak reference only hides
+the GC-retention symptom (and the target may go dead under the consumer). *"Your fix isn't a
+fix."* A **warning** (`severity="warning"` — real, shown soft), distinct from the strong
+DI001 capture. The extractor reads a `WeakReference<X>` constructor parameter (`WeakRefInner`)
+into a **separate `weak_deps`** list, deliberately kept **off** the DI001 strong graph, so the
+same scoped service is either a strong captive (DI001) or a weak captive (DI002), never both;
+`ownlang/di.py` `find_weak_captive_dependencies` flags a singleton whose `weak_deps` names a
+scoped service. Pinned end-to-end by `DiCaptiveSample.cs` (`WeakCache` →
+`WeakReference<AppDbContext>`, with `WeakClockHolder → WeakReference<Clock>` staying silent —
+a weak ref to a singleton is no mismatch) in the `wpf-extractor` CI job, and at the graph
+level by `tests/test_ownir.py`. It is a contract no general-purpose analyzer models — even the
+developer's WeakReference "fix" is still flagged, which is the key differentiation.
+
 ## Next (separate slices)
 
-- **DI002** — a singleton capturing a scoped dependency *weakly*
-  (`WeakReference<Scoped>`): a warning, since a weak reference fixes retention but
-  not the lifetime-contract violation.
+- **DI002, the transitive form** — a singleton holding a `WeakReference<Transient>` whose
+  transient *drags in* a scoped service (the weak edge is one hop above the scoped); the
+  shipped slice flags the common **direct** `WeakReference<Scoped>` shape.
 - **DI003, the explicit form** — a transient `IDisposable` resolved by hand from the
   **root** provider (`root.GetService<T>()`), which the graph form above does not see (it
   needs the resolution call sites, not just the registration graph).

docs/proposals/P-006-di-lifetimes.md

@@ -11,8 +11,9 @@
   singleton→singleton and clean registrations silent). See
   [docs/notes/di-captive-extractor.md](../notes/di-captive-extractor.md). **DI003**
   (a transient `IDisposable` captured by a singleton — promoted to application lifetime)
-  now also fires end-to-end as a **warning**, CI-validated on the same sample. Next:
-  DI002 (weak-ref).
+  and **DI002** (a scoped service held by a singleton via `WeakReference<T>` — still a
+  captive: the weak ref hides the GC symptom, not the lifetime violation) now also fire
+  end-to-end as **warnings**, CI-validated on the same sample.
 - **Depends on:** `spec/Lifetimes.md` (the region-ordering model behind OWN014),
   [P-001](P-001-csharp-extractor.md) (the C# seam). See
   [`docs/ROADMAP.md`](../ROADMAP.md) (Milestone 3).
@@ -39,12 +40,13 @@ to a longer-lived region) already models it.
 
 - **DI001 (error):** a singleton service captures a scoped dependency (directly,
   or transitively through the constructor graph).
-- **DI002 (warning):** a singleton captures a scoped dependency **weakly**
+- **DI002 (warning) — shipped:** a singleton captures a scoped dependency **weakly**
   (`WeakReference<Scoped>`). A weak reference fixes *retention* leaks, not a
-  *lifetime contract* violation — the scoped service is still invalid outside its
-  scope and may be disposed mid-use. Message: *"`WeakReference` does not make a
-  scoped service safe to use outside its scope; resolve it inside a fresh scope
-  via `IServiceScopeFactory`, or make the consumer scoped."*
+  *lifetime contract* violation — the scoped service is still root-resolved, lives
+  for the app lifetime, and may be disposed mid-use. The `WeakReference<X>` ctor
+  parameter is read into a separate `weak_deps` list (off the DI001 strong graph), and
+  `find_weak_captive_dependencies` flags a singleton whose `weak_deps` names a scoped
+  service. CI-validated on `DiCaptiveSample.cs` (`WeakCache`).
 - **DI003 (warning) — shipped:** a transient `IDisposable` **captured by a singleton**
   is resolved from the root (via the singleton), promoted to application lifetime, and
   disposed only at root disposal — held far longer than its `transient` registration

frontend/roslyn/OwnSharp.Extractor/Program.cs

@@ -937,6 +937,10 @@
 {
     // 1. class name -> its widest constructor's parameter type names (the DI ctor).
     var ctorDeps = new Dictionary<string, List<string>>();
+    // class name -> services injected via `WeakReference<T>` (held weakly). Kept apart
+    // from ctorDeps so the strong DI001 graph never sees a weak edge; a weakly-held
+    // scoped service is DI002 (the weak ref hides the GC symptom, not the lifetime bug).
+    var ctorWeakDeps = new Dictionary<string, List<string>>();
     // class name -> does it implement IDisposable/IAsyncDisposable (so the container
     // owns its disposal)? Syntactic — its OWN base list names it; an inherited
     // disposable (`: Stream`) is not seen, so DI003 fires only on an explicitly
@@ -962,14 +966,21 @@
                         || ctor.ParameterList.Parameters.Count > widest.Parameters.Count))
                     widest = ctor.ParameterList;
             var deps = new List<string>();
+            var weakDeps = new List<string>();
             if (widest is not null)
                 foreach (var p in widest.Parameters)
                 {
-                    var tn = p.Type is null ? null : DiTypeName(p.Type);
-                    if (tn is not null)
+                    if (p.Type is null)
+                        continue;
+                    // a `WeakReference<X>` parameter is a WEAK dep on X (not a strong dep):
+                    // it keeps X off the DI001 graph, but a weakly-held scoped X is DI002.
+                    if (WeakRefInner(p.Type) is { } weakInner)
+                        weakDeps.Add(weakInner);
+                    else if (DiTypeName(p.Type) is { } tn)
                         deps.Add(tn);
                 }
-            ctorDeps[cls.Identifier.Text] = deps;   // last decl wins (core dedups by name)
+            ctorDeps[cls.Identifier.Text] = deps;        // last decl wins (core dedups by name)
+            ctorWeakDeps[cls.Identifier.Text] = weakDeps;
             // OR across partial declarations: any part that names IDisposable makes the
             // type disposable, so a later `partial class C { }` (no base list, e.g. a
             // generated/designer file) cannot clear an earlier `partial class C : IDisposable`.
@@ -994,11 +1005,14 @@
                 continue;
             var deps = impl is not null && ctorDeps.TryGetValue(impl, out var d)
                 ? d : new List<string>();
+            var weakDeps = impl is not null && ctorWeakDeps.TryGetValue(impl, out var wd)
+                ? wd : new List<string>();
             services.Add(new
             {
                 name = service,
                 lifetime,
                 deps,
+                weak_deps = weakDeps,
                 // the IMPLEMENTATION's disposability — the container constructs and
                 // disposes the impl, so a transient-disposable impl captured by a
                 // singleton is held to app exit (DI003).
@@ -1062,9 +1076,33 @@
     GenericNameSyntax g => g.Identifier.Text,
     QualifiedNameSyntax q => DiTypeName(q.Right),
     AliasQualifiedNameSyntax aq => DiTypeName(aq.Name),
+    // a nullable annotation (`AppDbContext?`) does not change the injected service type —
+    // unwrap it so a nullable ctor param is still a real dep (CodeRabbit review on #63).
+    NullableTypeSyntax n => DiTypeName(n.ElementType),
     _ => null,
 };
 
+// If `t` is a `WeakReference<X>` (or `System.WeakReference<X>`, or a nullable
+// `WeakReference<X>?`), the simple name of its single type argument X; else null.
+// Syntactic, single-arg — matches how a singleton holds a captive dependency weakly.
+// (`System.WeakReference` non-generic has no element type and is not a DI dep.)
+static string? WeakRefInner(TypeSyntax t)
+{
+    if (t is NullableTypeSyntax nt)   // `WeakReference<X>?` -> unwrap the nullable annotation
+        t = nt.ElementType;
+    var g = t switch
+    {
+        GenericNameSyntax gen => gen,
+        QualifiedNameSyntax { Right: GenericNameSyntax gen } => gen,
+        AliasQualifiedNameSyntax { Name: GenericNameSyntax gen } => gen,
+        _ => null,
+    };
+    // the inner `X` (or a nullable `X?`) is resolved by DiTypeName, which unwraps `?`.
+    return g is { Identifier.Text: "WeakReference" }
+           && g.TypeArgumentList.Arguments.Count == 1
+        ? DiTypeName(g.TypeArgumentList.Arguments[0]) : null;
+}
+
 // DI's default IServiceProvider resolves through PUBLIC constructors only — an
 // explicit ctor with no access modifier defaults to private and DI never uses it.
 static bool IsPublicCtor(SyntaxTokenList modifiers)
@@ -1117,7 +1155,7 @@
     .Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
     .Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
     .ToList();
 var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);
 var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
 // P-004 WPF profile: widen the reference set with assemblies named by the
 // OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref

frontend/roslyn/samples/DiCaptiveSample.cs

@@ -49,6 +49,18 @@
     public sealed class PooledConnection : System.IDisposable { public void Dispose() { } } // transient, IDisposable
     public sealed class ConnectionWarmer { public ConnectionWarmer(PooledConnection c) { } } // singleton -> captures it
 
+    // DI002 — a singleton that holds a SCOPED service via WeakReference<T>. A weak ref is
+    // the usual "fix" for a DI001 captive (it stops pinning the scoped instance for the
+    // GC), but the scoped service is still resolved from the root and lives for the app
+    // lifetime — the lifetime contract is still violated. A warning. The weak ref keeps it
+    // OFF the DI001 strong graph (`deps`), so it surfaces as DI002, not DI001.
+    public sealed class WeakCache { public WeakCache(WeakReference<AppDbContext> db) { } }      // -> WeakReference<scoped> : DI002
+    // a NULLABLE weak reference (`WeakReference<AppDbContext>?`) is the same weak captive — the
+    // `?` annotation does not change the service type, so it is DI002 too (CodeRabbit review).
+    public sealed class WeakCacheOpt { public WeakCacheOpt(WeakReference<AppDbContext>? db) { } }
+    // control: a weak reference to a SINGLETON is no lifetime mismatch -> SILENT.
+    public sealed class WeakClockHolder { public WeakClockHolder(WeakReference<Clock> clock) { } }
+
     public static class Startup
     {
         public static void ConfigureServices(IServiceCollection services)
@@ -58,7 +70,7 @@
             services.AddScoped<IRepo, Repo>();                        // interface -> impl, scoped
 
             // FLAGGED — singleton captures a scoped service directly.
             services.AddSingleton<EmailSender>();
 
             services.AddTransient<UnitOfWork>();                      // transient
 
@@ -66,10 +78,10 @@
             services.AddSingleton<ReportService>();
 
             // FLAGGED — through the interface registration: singleton -> IRepo (scoped).
             services.AddSingleton<CacheService>();
 
             // FLAGGED — primary-constructor injection: singleton -> scoped AppDbContext.
             services.AddSingleton<PrimaryCtorService>();
 
             // SILENT — DI resolves the public parameterless ctor; the private ctor's
             // scoped dependency is never used (no false captive).
@@ -82,7 +94,18 @@
             // transient IDisposable PooledConnection: promoted to application lifetime,
             // disposed only at root disposal. NOT a DI001 (no scoped captured).
             services.AddTransient<PooledConnection>();
             services.AddSingleton<ConnectionWarmer>();
+
+            // FLAGGED (DI002, warning) — singleton holds a SCOPED service via WeakReference:
+            // the weak ref hides the GC-pinning symptom, but scoped AppDbContext is still
+            // root-resolved and app-lived (the captive lifetime violation remains). NOT a
+            // DI001 (the weak edge is off the strong graph).
+            services.AddSingleton<WeakCache>();
+            // FLAGGED (DI002) — a NULLABLE WeakReference<AppDbContext>? is the same weak captive
+            // (the `?` annotation is unwrapped, so the scoped service is still seen).
+            services.AddSingleton<WeakCacheOpt>();
+            // SILENT — a weak reference to the SINGLETON Clock is no lifetime mismatch.
+            services.AddSingleton<WeakClockHolder>();
         }
     }
 

ownlang/di.py

@@ -47,6 +47,12 @@ class Service:
     disposable: bool = False
     file: str = "?"
     line: int = 0
+    # services injected via `WeakReference<T>` — held WEAKLY, so they are NOT strong
+    # captive edges (DI001 must not see them), but a weakly-held scoped service is still
+    # a lifetime-contract violation: DI002. Declared LAST so the positional constructor
+    # contract (name, lifetime, deps, disposable, file, line) is preserved — callers pass
+    # `disposable`/etc. positionally, so a new field before them would shift their meaning.
+    weak_deps: tuple[str, ...] = ()
 
 
 @dataclass(frozen=True)
@@ -107,6 +113,58 @@ def find_captive_dependencies(services: list[Service]) -> list[CaptiveDependency
     return findings
 
 
+@dataclass(frozen=True)
+class WeakCaptiveDependency:
+    """A singleton that holds a **scoped** service via `WeakReference<T>` (DI002). A
+    weak reference is the usual "fix" for the DI001 captive leak — it stops the
+    singleton from pinning the scoped instance for the GC. But it does not fix the
+    *lifetime contract*: the scoped service is still resolved from the root provider
+    and lives for the application lifetime; the weak reference only hides the
+    GC-retention symptom, not the captive cause (and may go dead under the consumer)."""
+
+    singleton: str
+    captured: str
+    path: tuple[str, ...]
+    file: str
+    line: int
+
+    @property
+    def message(self) -> str:
+        chain = " -> ".join(self.path)
+        return (f"singleton '{self.singleton}' weakly captures scoped service "
+                f"'{self.captured}' (WeakReference): '{self.captured}' is still resolved "
+                f"from the root provider and promoted to application lifetime — the weak "
+                f"reference avoids pinning it for the GC but does not fix the "
+                f"captive-dependency lifetime violation ({chain})")
+
+
+def find_weak_captive_dependencies(
+        services: list[Service]) -> list[WeakCaptiveDependency]:
+    """Return every scoped service a singleton holds via `WeakReference<T>` (DI002).
+    The direct form: a singleton whose `weak_deps` names a scoped service. The weak
+    reference keeps it off the DI001 strong-capture graph, but the scoped instance is
+    still root-resolved and app-lived — a lifetime-contract violation, surfaced as a
+    warning. (Weakly-held transients that *drag in* a scoped are a separate, rarer
+    slice — not followed here, the direct weak-scoped edge is the common 'I wrapped my
+    captive in WeakReference' shape.)"""
+    by_name = {s.name: s for s in services}
+    findings: list[WeakCaptiveDependency] = []
+    for s in services:
+        if s.lifetime != SINGLETON:
+            continue
+        reported: set[str] = set()
+        for dep in s.weak_deps:
+            dnode = by_name.get(dep)
+            if dnode is None or dnode.lifetime != SCOPED or dep in reported:
+                continue
+            reported.add(dep)
+            findings.append(WeakCaptiveDependency(
+                singleton=s.name, captured=dep, path=(s.name, dep),
+                file=s.file, line=s.line))
+    findings.sort(key=lambda f: (f.file, f.line, f.singleton, f.captured))
+    return findings
+
+
 @dataclass(frozen=True)
 class CapturedTransientDisposable:
     """A singleton that captures a transient `IDisposable` service (DI003): the