[thomhurst]

DI ownership currently leaves permanent marks on resolved objects and changes ownership while traversing their graphs. Store resolved roots and ownership snapshots on row scope handles instead. Tracking discovery now enumerates objects without changing their disposal ownership.

Rows, static leases and session trackers retain the handles. A weak handle registry lets other data sources and sessions find the owner of a shared service; existing reference-count callbacks retain an owner through overlapping consumers. Ownership disappears once the last handle is released, rather than lasting for the object's entire lifetime. Scope-owned initializers are still initialized, while TUnit-injected fixtures and their aliases remain tracker-owned and are disposed once.

Carry forward and adapt the direct ownership tests from #7634, delivering #7376 against the replacement implementation. The suite explicitly checks generated and reflection property registries and respects reflection execution disabling generated property injection. Add regressions for weak owner lifetime, overlapping consumers, cloned rows, pure discovery, sibling aliases and changing dependency graphs. Also correct the stale discovery-error XML comment.

Closes #7378
Closes #7376

Preserve #7633's scope ownership opt-out and failed-factory cleanup on the current base. Ownership snapshots retain known injected identities when a foreign row refreshes a smaller child graph, so injected aliases remain tracker-owned. Add a forced-GC regression for a counted consumer retaining a completed owner and a no-owner lookup fast path.

Validation on SDK 10.0.403, macOS arm64, net10.0:

• Eight focused unit classes: 78 passed in source-generated execution and 78 with --reflection.
• _7375 integration filter in generated, reflection and native AOT: expected 10 results (9 passed, 1 intentional factory failure).
• _7205 integration filter in generated, reflection and native AOT: expected 25 results (17 passed, 5 intentional failures, 3 skipped).
• Native AOT Release publish succeeded with one MSBuild worker after an initial parallel MSB4166 interruption.
• git diff --check passed. Existing build warnings are tracked in Analyzer and code-fixer projects emit release-tracking and nullable build warnings #7410; existing AOT test-project warnings remain.
• A standalone Release benchmark (tiering disabled, five samples) measured median no-owner lookup time of 228.401 ms before versus 166.852 ms after for 100 million calls. This is a lookup-only result.

Engine tests are left to CI per repository instructions. No additional public API or generator output changes.

Summary by CodeRabbit

• Bug Fixes
  • Improved cleanup of objects shared between data-row scopes and test execution, preventing resources from being disposed while still in use.
  • Preserved ownership of scope-managed dependencies as object graphs change, including nested dependencies and injected values.
  • Improved disposal handling for resources shared through static properties, ensuring resources are disposed when their owners and consumers are finished.

Review follow-up (1f4708d):

Use weak historical ownership identities, revoke ownership for later injected dependencies, release tracker handles per completed test, and index same-singleton owner registration. Full unit suite: 910 passed / 3 skipped in each generated and reflection mode (SDK 10.0.403, macOS arm64/net10.0). Registration-only 40k-owner median: 688.074 → 4.714 ms. Capture timings are mixed; detailed measurements and rationale are in the review disposition comment. Also fix the Windows CI null-metadata failure in synthetic contexts. New-head CI/reviews are pending.

Review follow-up (7f402cd): fix netstandard2.0 weak-identity enumeration, refresh external ownership during failed-factory cleanup with partial-graph error handling, and copy clone mappings without tracker re-entry under the collection lock. Add changed-dependency/getter-error and never-tracked pin GC regressions. Full unit suite: 913 passed / 3 skipped in each generated and reflection mode; netstandard2.0 core build: zero warnings/errors. Engine/AOT validation remains for CI.

Review follow-up (ddf57af): failed-row cleanup refreshes every live shared-root owner using a strong snapshot taken outside graph discovery; lookup prunes dead handles. New regression fails before the fix and verifies both owners protect the changed dependency. Full unit suite: 915 passed / 3 skipped in each generated and reflection mode; netstandard2.0 core build: zero warnings/errors. New-head CI/reviews pending; engine and AOT remain for CI.

Review follow-up (27204da, 1678913): refresh every live owner during normal tracking and static initialization; serialize owner pins with counter retirement and preserve references reacquired by zero-count callbacks. Extended multi-owner coverage and added forced-GC reentrant-consumer regression. Final full unit suite: 916 passed / 3 skipped in each generated and reflection mode; netstandard2.0 core build: zero warnings/errors. New-head engine/AOT/CI and reviews remain pending.

Concurrency review follow-up (16a5794): use the counter lock for static lease state as well, removing an inversion when a zero-count callback acquires a lease while another thread acquires one. Add a concurrent capture/registration/reentrant-counter-release/GC regression that verifies counted owner retention and collection after the final release. Full unit suite: 917 passed / 3 skipped in generated execution and 917 passed / 3 skipped with --reflection; netstandard2.0 core build: zero warnings/errors. New-head reviews and CI remain pending; Engine tests and AOT remain for CI.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: 1678913
• Latest commit, not reviewed: 16a5794

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• Resume reviews
• Trigger review

Walkthrough

Data-row scopes now capture ownership of discovered objects, excluding injected values. Object tracking retains scope owners while tracked references remain and checks ownership before disposal. Static-property initialization also captures and refreshes scope ownership.

Changes: Data-row scope ownership

Layer / File(s)	Summary
Scope ownership capture src/TUnit.Core/ExternalObjectOwnership.cs, src/TUnit.Core/Discovery/ObjectGraphDiscoverer.cs, src/TUnit.Core/DataRowScopeCollection.cs, src/TUnit.Core/Attributes/TestData/DependencyInjectionDataSourceSourceAttribute.cs, tests/TUnit.UnitTests/ExternalObjectOwnership*.cs, tests/TUnit.UnitTests/ExternalObjectOwnershipRegistrationTests.cs, tests/TUnit.UnitTests/ObjectGraphDiscovererOwnershipTests.cs, tests/TUnit.UnitTests/DataRowScopeCollectionTests.cs	Scope owners register roots and capture object-graph ownership. Discovery excludes injected objects. Data-source resolution refreshes ownership, including for partial graphs after errors. Tests cover graph changes, aliases, owner lifetime, and reflection and source-generated discovery.
Tracker ownership lifecycle src/TUnit.Core/Tracking/ObjectTracker.cs, tests/TUnit.UnitTests/ExternallyOwnedSingletonTests.cs, tests/TUnit.UnitTests/ObjectTrackerRollbackTests.cs	The tracker retains scope owners across tracked references and checks ownership before disposal. Cleanup releases retained owners. Tests cover consumer ordering, refreshed graphs, injected aliases, and owner retention.
Static-property ownership refresh src/TUnit.Engine/Services/StaticPropertyHandler.cs, tests/TUnit.UnitTests/StaticPropertyHandlerCleanupTests.cs, tests/TUnit.UnitTests/StaticPropertyScopeDisposalTests.cs	Static-property initialization captures scope ownership and refreshes ownership for discovered objects. Tests cover changed dependencies and ownership checks during cleanup.

Estimated code review effort: 4 (Complex) | ~40 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant DataRowScopeCollection
  participant ScopeOwner
  participant ObjectGraphDiscoverer
  participant ExternalObjectOwnership
  participant ObjectTracker
  DataRowScopeCollection->>ScopeOwner: Capture ownership of registered roots
  ScopeOwner->>ObjectGraphDiscoverer: Discover owned objects and injected identities
  ScopeOwner->>ExternalObjectOwnership: Register owned object identities
  ObjectTracker->>ExternalObjectOwnership: Check ownership before disposal
  ObjectTracker->>DataRowScopeCollection: Release retained owners during cleanup

Loading

Possibly related issues

• thomhurst/TUnit#7375 — The changes apply scope ownership policy when capturing ownership for data-source-created objects.
• thomhurst/TUnit#7472 — The changes extend data-row scope registration with ownership capture and object-graph tracking.
• thomhurst/TUnit#7382 — The changes replace process-wide ownership marks with per-scope ownership and exclude injected values from scope-owned results.
• thomhurst/TUnit#7640 — The changes add repeated ownership-capture paths that issue #7640 identifies as a follow-up concern.
• thomhurst/TUnit#7475 — The PR changes tracker cleanup and owner retention, which relate to session-end disposal of objects owned by another active session.
• thomhurst/TUnit#7383 — The static-property changes capture and retain ownership through scope handles.
• thomhurst/TUnit#7213 — The failure path refreshes ownership for resolved roots and discovered graph objects, including partial graphs.

Possibly related PRs

• thomhurst/TUnit#7633 — Both PRs use the disposable-scope ownership policy for data-source-created objects.
• thomhurst/TUnit#7476 — This PR extends synchronized data-row scope registration with graph ownership snapshots and tracker-held owner handles.
• thomhurst/TUnit#7634 — Both PRs cover scope-owned initializer dependencies and tracker-owned injected values.

Merge Risk: Low · previous review 16789

Uncommon initialization failures can leave injected fixtures undisposed. The change is mergeable with that cleanup risk understood.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "fix(core): track external ownership per row scope" matches the main change. The PR replaces process-wide external-ownership marks with per-row-scope ownership tracking in DataRowScopeCollec…
Linked Issues check		[#7378] ObjectTracker no longer uses process-wide ownership state. Row-scope handles and ownership snapshots provide per-owner tracking. Discovery does not transfer ownership. Scope-owned objects re…
Out of Scope Changes check		The production changes implement per-owner tracking, pure discovery, ownership refresh, weak owner lifetime, clone handling, and static-lease behavior for [#7378] and [#7376]. The added tests verify t…

Full details: Title check

Explanation

The title "fix(core): track external ownership per row scope" matches the main change. The PR replaces process-wide external-ownership marks with per-row-scope ownership tracking in DataRowScopeCollection, ExternalObjectOwnership, ObjectTracker, and related discovery code. The title is concise, specific, and follows a conventional commit style. It does not cover every detail, which is expected.

Full details: Linked Issues check

Explanation

[#7378] ObjectTracker no longer uses process-wide ownership state. Row-scope handles and ownership snapshots provide per-owner tracking. Discovery does not transfer ownership. Scope-owned objects remain initialized, and injected objects remain tracker-owned and are disposed once. The reported generated and reflection tests pass. [#7376] Direct discoverer tests cover scope-owned dependencies, injected dependencies, shared aliases, and both property registries. Shared objects remain tracker-owned and are disposed exactly once.

Full details: Out of Scope Changes check

Explanation

The production changes implement per-owner tracking, pure discovery, ownership refresh, weak owner lifetime, clone handling, and static-lease behavior for [#7378] and [#7376]. The added tests verify these behaviors and related cleanup paths. The discovery comment correction and test-context updates support the same objectives. No unrelated functional or public API changes are demonstrated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 39 functions across 14 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the scope-owned trail,
While injected friends escape the rail.
The tracker holds each owner near,
And lets go when rows disappear.
Changed graph branches join the play,
Then hop safely on their way.}

[thomhurst]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T12:54:49.213515Z	16a5794	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Review: per-row-scope external ownership

I read the diff and the surrounding ObjectTracker code. I did not build or run the tests. Moving ownership onto ScopeOwner handles is a clear improvement over the permanent ConditionalWeakTable marks. Discovery no longer mutates ownership, and ownership ends when the last handle is released.

Concerns

1. Lock ordering needs checking.

   • DisposeUnleasedTrackedObjectAsync holds s_sharedInstanceLock and then calls ExternalObjectOwnership.IsOwned, which takes Gate and then each owner's _ownershipLock.
   • RetainWhileCounted holds _ownershipLock and calls ObjectTracker.OnDisposed, which touches s_trackedObjects and the counter and may run the callback inline.
   • The two paths take locks in opposite order if OnDisposed or Counter ever takes s_sharedInstanceLock. I could not confirm it does not.
   • The safe fix is to do the Add under _ownershipLock and register the OnDisposed callback after releasing it. The callback only removes the entry.

2. Hot-path cost.

   • TrackObject and UntrackObject now call FindOwner for every tracked disposable. FindOwner takes a global lock and scans all owners linearly, taking each owner's lock. UntrackObject then calls IsOwned, which repeats the scan.
   • With DI in use and many rows, this is O(owners) per object, serialized on one Gate.
   • Suggestions:
     • Skip the lookup when no owner has been registered. A volatile flag would do, like the old s_hasExternallyOwned.
     • Reuse the externalOwner already found instead of calling IsOwned again.
     • Consider a ConditionalWeakTable keyed by object, with the owner set as the value. It keeps the weak behaviour and gives O(1) lookup.
   • AGENTS.md asks for measurement on hot paths, so a before/after number would help.

3. Prune trigger. Owners.Count % 64 == 0 is a fragile trigger. After pruning, the count can sit just above a multiple of 64, so dead weak references accumulate. It can also prune repeatedly at an exact multiple. A threshold such as Count >= nextPruneAt, then nextPruneAt = max(64, Count * 2), is more robust.

4. Session-end order. _externalOwners.Clear() must run after all untrack and dispose decisions for the session. Otherwise the strong handles no longer protect a late disposal. Please confirm the order.

5. Snapshot only grows. CaptureOwnership unions the previous snapshot into the new one. That is intentional, but it is unbounded for long-lived handles, so it is worth a note in the doc comment.

6. Style (non-blocking). Brace-less single-line if and foreach statements are dense. DiscoverOwnedObjects walks the graph twice with different callbacks.

Positives

• The tests cover weak-owner lifetime, overlapping consumers, cloned rows, pure discovery, sibling aliases and changing graphs, in both modes.
• The stale XML comment is fixed.

Please look at 1 and 2 before merging. The rest are optional.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 27ee1e3003

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[greptile-apps]

Confidence Score: 5/5

[High impact] The latest changes appear safe to merge; no new actionable issue was found.

Summary

The PR replaces permanent object ownership marks with weak ownership records tied to row handles.

• Rows keep scope ownership tied to their current dependency graphs.
• Tracked consumers keep row owners alive until their references end.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Static lease acquired] --> B[Lock shared lease and count state]
  B --> C[Increase lease and object counts]
  C --> D[Retain live owner]
  E[Object count reaches zero] --> F[Run count callbacks under the same lock]
  F --> G{Callback acquires another lease?}
  G -->|Yes| H[Keep counter and owner pin]
  G -->|No| I[Retire empty counter]
  I --> J[Leave scope-owned disposal to its owner]

Loading

_{Reviews (11) · Last reviewed commit: "fix: share static lease and counter lock..." · Reviewed by Greptile}

[thomhurst]

Addressed the review findings in cbd9c38.

• Replaced the process-wide owner scan with a weak identity index. This also eliminates the global prune trigger.
• Refresh existing ownership when a non-owning row supplies a shared root or discovers a shared service, preserving both old and new dependencies.
• Moved OnDisposed registration outside the ownership lock as Claude requested.
• Confirmed session-end ordering: DisposeAndClearStaticTrackingAsync untracks every session reference before clearing _externalOwners.
• Documented why ownership snapshots accumulate dependencies for the handle lifetime. The optional traversal/style suggestions remain unchanged.

Validation: all six focused classes passed, 33 tests in generated mode and 33 with --reflection, using SDK 10.0.403/net10.0 on macOS arm64. Classes: ExternalObjectOwnershipTests, ObjectTrackerRollbackTests, DataRowScopeCollectionTests, ObjectGraphDiscovererOwnershipTests, StaticPropertyScopeDisposalTests and StaticScopeCleanupTests. git diff --check passed. Engine tests remain for CI.

A standalone Release microbenchmark compared the previous registry implementation with the new index, using a minimal owner with the same lock/hash-set lookup. For 20,000 unrelated identity lookups: 100 owners, 23.690 ms vs 3.650 ms; 1,000 owners, 219.812 ms vs 0.122 ms; 10,000 owners, 1067.270 ms vs 0.042 ms. These are lookup-only timings and include JIT/tiering noise; they demonstrate removal of the owner-count scan, not an end-to-end suite speedup.

The PR remains draft for fresh reviews of this head.

[thomhurst]

@codex review

[github-actions]

Review: per-row-scope external ownership (#7635)

I read the diff and the new ExternalObjectOwnership / ScopeOwner code. I didn't build or run tests; the tests added in the PR look thorough (both source-gen and reflection modes covered).

Overall: Moving ownership from permanent marks onto ScopeOwner handles is the right direction. Discovery becomes pure, and ownership ends when the last handle is released. The earlier review items (lock ordering, process-wide scan, registering OnDisposed outside the lock) appear addressed in cbd9c38, and the weak identity index is a clear improvement.

Concerns / suggestions

1. Weak-list growth in ExternalObjectOwnership.Register. Dead entries are pruned only on Register/FindOwner for that same instance. A long-lived shared singleton touched by many rows keeps accumulating WeakReference objects until the next call. Pruning on every Register is O(n) per call, so n rows give O(n²) total. This is probably fine at normal test counts, but a high-cardinality data source against a shared service could show it. Consider compacting only when the list length crosses a threshold (e.g. doubles), which keeps amortised cost O(1).
2. FindOwner returns an arbitrary live owner (last registered). Callers that need any owner are fine, but IsOwned and FindOwner hide that semantics. If the intent is that any live owner is enough to keep the object from tracker disposal, say so in the XML doc. Otherwise a future caller may assume a unique owner.
3. Lock nesting. The code now has OwnerSet lock, _ownershipLock, _lock and s_sharedInstanceLock. The PR shows that ordering is checked, but it's only documented in comments. A short ordering note in one place (e.g. on ScopeOwner) would stop future regressions. FindOwner calls only take the leaf OwnerSet lock, which is good.
4. lock (owners) on a private class instance is fine here, but a dedicated lock object or Lock (net9+) would be more consistent with the rest of the file.

None of these block the merge; #1 is the only one with a measurable effect. Nice work on the regression coverage for weak-owner lifetime, overlapping consumers and cloned rows.

[thomhurst]

@codex review

[github-actions]

Review

I read the source diff and did not run the tests. The change replaces the permanent ConditionalWeakTable marks and the mutating MarkExternallyOwned traversal with ownership snapshots held on ScopeOwner handles. A weak registry (ExternalObjectOwnership) finds the owner of a shared service. This is a clear improvement:

• Graph discovery no longer changes ownership, so DiscoverOwnedObjects can be tested on its own.
• Ownership ends when the last handle is released. That is better than the old flag, which was never cleared.
• GC.KeepAlive(externalOwner) and the rule against taking the ownership lock while entering the counter both deal with the lifetime and lock-ordering hazards.
• The injected-value set is kept across the whole traversal, so a sibling alias of an injected fixture is still tracker-owned.

Concerns to consider:

1. Weak-handle lifetime is subtle. Disposal safety now depends on some strong reference to the ScopeOwner existing: a row, a static lease, _externalOwners, or a _pinnedObjects callback. If every strong reference is gone before the object's count reaches zero, IsOwned returns false and the tracker disposes the object a second time. The RetainWhileCounted callback is meant to prevent this, and the new overlapping-consumer tests cover it. I'd still add one test that forces GC.Collect() between the owner's last row ending and the consumer's release.
2. Snapshots only grow. CaptureOwnership unions each new snapshot with _ownedObjects. The comment justifies this, but a long-lived static lease could accumulate a large set. Consider bounding it, or documenting it as a known trade-off.
3. Cost on the hot path. RefreshOwner runs FindOwner for every tracked instance (a CWT lookup plus a lock). The old code skipped the lookup via s_hasExternallyOwned. A cheap global "any owner ever registered" short-circuit would keep tests without DI at zero cost.
4. Lock-per-object in ExternalObjectOwnership.Register. Each OwnerSet uses lock(owners) on a private object, which is fine, but the linear scan plus RemoveAt is O(owners). That is acceptable for small counts.

No blocking issues. I'd like to see a GC-based test for item 1 and a decision on item 3 before merge.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 390a4f1859

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Review: per-row-scope ownership (#7635)

I read the diff but did not build it or run the tests. This is a sound direction: it replaces permanent marks on resolved objects, and ownership changes made while walking a graph, with handle-scoped snapshots. Discovery is now side-effect free, which is a clear improvement. The lifetime and concurrency questions below are the ones I would want answered before merge.

Concerns

1. GC-dependent disposal. ExternalObjectOwnership holds owners through WeakReference. Whether an object is tracker-disposed or scope-disposed then depends on whether some strong handle is still alive: a row, a static lease, the _externalOwners set, or the RetainWhileCounted callback closure. If any path misses a retention, a collection decides ownership. The result is a flaky double dispose or a missed dispose, and neither would reproduce easily. The GC.KeepAlive(externalOwner) in UntrackObjectCore suggests this was already hit once. It would help to document the retention invariant in one place and add a stress test that forces GC.Collect() between the row's disposal and the last untrack.
2. Cost in the tracking hot path. TrackObjects now calls CaptureOwnership for every owner on every test. Each call does a full graph discovery (three traversals in DiscoverOwnedObjects) and then allocates a new HashSet. RefreshOwner adds more traversals for each root and for each tracked instance. RetainWhileCounted allocates a closure per object. Rows cloned from the same scope will repeat identical work. Please cache the snapshot per owner and invalidate it only when the root set changes. Also measure before and after, as AGENTS.md requires; the PR validation lists correctness runs but no performance numbers.
3. Locks and callbacks. The code relies on a hand-kept rule that _ownershipLock is never held while calling ObjectTracker.OnDisposed. RetainWhileCounted follows the rule, but _externalOwners is guarded by s_sharedInstanceLock, which is also held around count callbacks. A comment at the lock declarations stating the lock order, or a test that runs register, track and untrack concurrently, would protect against later regressions.
4. ScopeOwner has too many jobs. It is now a disposer, a root registry, a snapshot builder and a refcount-pin manager. It uses four sets, two locks and mutable public-ish state (OwnsCreatedObjects { get; set; }, assigned after construction). I would split out an OwnershipSnapshot type that is immutable and built once, so the readers of Owns never touch the builder. The owner would then hold only the scope, the current snapshot and the pin set. That also removes the "union with the previous snapshot" bookkeeping inside CaptureOwnership.
5. Monotonic accumulation. _ownedObjects and _injectedObjects only grow, by design (the comment on CaptureOwnership says so). For a long-lived static lease with a changing graph this means retained strong references and stale ownership claims. Please confirm that the entries are released when the handle is released, and that a stale entry cannot suppress disposal of an object that has since moved out of the scope's graph.
6. Small items.
   • injectedRoots ?? [] allocates on each call. Use Array.Empty<object?>() or a static.
   • DependencyInjectionDataSourceSourceAttribute.cs and DataRowScopeCollection.cs lost or gained a BOM. Keep the encoding consistent to avoid diff noise.
   • DiscoverOwnedObjects passes default as the cancellation token, so a deep graph can no longer be cancelled.

Positives: the removal of MarkExternallyOwned and MarkRowOwnedScope, the pure-discovery semantics, and the regression coverage (overlapping consumers, cloned rows, sibling aliases and changing graphs in both modes, plus an AOT check). The DiscoverStaticPropertyTrackableObjects wrapper is a pass-through with no added behaviour and could be removed.

Summary: the design is good. I would want the performance of concern 2 and the GC-retention invariant of concern 1 addressed or measured before this merges.

[thomhurst]

Rebased onto current main and pushed e06b3d5, preserving #7633's scope ownership opt-out and failed-factory cleanup. Known injected identities now survive smaller ownership refreshes, fixing Greptile's cleanup finding. The synthetic singleton and static-session test contexts now populate TestDetails, which ownership refresh requires.

Claude follow-up dispositions:

• Added Counted_Consumer_Retains_An_Owner_Across_Collection_After_Row_Completion: force collection after row completion while only an overlapping counted consumer protects the owner, then verify the consumer releases without disposing the scope-owned dependency.
• Added the suggested no-owner fast path. A standalone Release lookup benchmark with tiered compilation disabled measured a median 228.401 ms before versus 166.852 ms after for 100 million no-owner lookups (five samples, macOS arm64/.NET 10). This isolates lookup cost and is not an end-to-end suite claim.
• Snapshots intentionally retain previously exposed dependencies and injected exclusions for the handle lifetime; this remains documented. Identity-local weak owner lists and their private monitor remain unchanged; any live owner suffices, now stated beside FindOwner. Callback registration remains outside the ownership lock to preserve lock ordering.

Validation on SDK 10.0.403, macOS arm64/net10.0:

• Eight focused classes: 78 passed in generated execution and 78 with --reflection. Includes DataRowScopeCollectionTests, ExternalObjectOwnershipTests, ExternallyOwnedSingletonTests, ObjectGraphDiscovererOwnershipTests, ObjectTrackerRollbackTests, StaticPropertyHandlerCleanupTests, StaticPropertyScopeDisposalTests and StaticScopeCleanupTests.
• _7375 in generated, reflection and native AOT: expected 10 total (9 passed, 1 intentional factory-resolution failure).
• _7205 in generated, reflection and native AOT: expected 25 total (17 passed, 5 intentional failures, 3 skipped).
• Native AOT Release publish succeeded with one MSBuild worker. The initial parallel publish stopped with MSB4166 (worker exited); its diagnostic directory had disappeared. Serial publishing succeeded, including the final source version. Existing analyzer/build warnings remain tracked in Analyzer and code-fixer projects emit release-tracking and nullable build warnings #7410, and existing test-project AOT warnings remain.
• git diff --check passed. Engine tests remain for CI per AGENTS.md. No additional public API or generator output changes.

The PR stays draft for fresh reviews of this rebased head.

[thomhurst]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e06b3d5e09

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[thomhurst]

Pushed 5d665f2 to fix the shared static fixture ownership finding; the inline reply records the regression and validation (45 passed in each execution mode).

For Claude's snapshot caching suggestion: invalidating only when the root set changes would miss changed initializer dependencies on the same root, which the row and static regressions explicitly cover. Keeping refreshes is the correctness decision here. The earlier GC-retention regression and no-owner lookup measurement remain in this PR; the lookup benchmark does not measure full graph refresh cost. The suggested snapshot type split and lock/style changes remain optional follow-up design work, not part of this fix. Snapshot identities and injected exclusions intentionally remain with the handle for its lifetime: previously exposed container dependencies still belong to the container after its current graph changes. They are released with the handle. Cancellation behavior and collection-expression encoding cleanup are unchanged by this fix.

The PR remains draft for fresh Claude and Codex verdicts on this head.

[thomhurst]

@codex review

[github-actions]

Review of #7635

I read ExternalObjectOwnership and DataRowScopeCollection in full. I did not read the rest of the diff or run any tests. The design is sound: ownership now lives on per-row handles instead of permanent marks, discovery has no side effects, and the owner snapshot is published only after discovery succeeds. The lock ordering is also careful. RetainWhileCounted never calls into the tracker while holding _ownershipLock, and AddRange snapshots the source before locking the destination.

Things to check, none of them blocking:

1. Strong references through _pinnedObjects. ScopeOwner._pinnedObjects holds strong references to disposable instances until the ObjectTracker.OnDisposed callback fires. If an instance is registered here but never reaches the tracker's counted lifecycle, the owner pins it and the entry is never removed. Please confirm OnDisposed fires, or runs immediately, for objects the tracker never counted. A regression test for a registered but never-tracked disposable would cover this.

2. Snapshots only grow. _ownedObjects is unioned across CaptureOwnership calls and never shrinks. That is intentional, per the comment, and it avoids a premature-dispose bug. In a long-lived shared service whose graph keeps changing, the set grows until the handle is released. A short note on this bound in the XML docs would help.

3. FindOwner returns an arbitrary live owner. It returns the last live owner in the list. That is fine while ownership only suppresses tracker disposal. If a caller later needs the specific owner, for example to release at a particular row, this will be ambiguous. Naming it FindAnyOwner or documenting the contract on the method would prevent misuse.

4. s_hasOwners never resets. It is a cheap fast path and correct. Once any owner is registered, every lookup falls through to the ConditionalWeakTable, which is acceptable.

5. OwnsCreatedObjects is a public setter on an internal class. The scope-ownership opt-out from Allow DI scopes to opt out of resolved object ownership #7633 sets it after construction. If RegisterObject runs first, the root is registered before the opt-out. A constructor parameter or an init-only property would rule out that ordering.

Overall this is a good change. The tests listed in the description cover the hard cases: weak owner lifetime, overlapping consumers, cloned rows, and changing graphs. I'd like an answer on item 1 before merge.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 5d665f246e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".