fix(P-004): capture-aware static-lambda tier — non-capturing lambda is not a region escape#211
Merged
Merged
Conversation
…s not OWN014 Issue #199 (subscription precision, lambda-handler tier). The four source-lifetime tiers already routed lambdas correctly (static->OWN014, injected->OWN001 warning, bounded/local->silent), but the general static path fired OWN014 on a NON-capturing lambda (`SomeBus.Pinged += (_,_) => StaticHandle()`) even though it retains no instance — a false positive against our own recorded policy (subscription-leaks-and-profiles.md: "static event + *capturing* handler -> error") and inconsistent with the static-METHOD exemption (a non-capturing method group is already silent). Fix — extractor (verified end to end with a local .NET SDK, not CI-only): gate the general static path through the existing conservative `HandlerRetainsNoInstance`, so a non-capturing lambda (the closure analog of a static-method handler) goes silent. It stays STRICTLY conservative: a lambda capturing `this` OR any enclosing local (the CsvHelper cts/resetEvent shape), or any delegate-typed value it cannot prove, is treated as retaining and STILL raises OWN014. Never widened syntactically (no `clsIsStatic`, no "all lambdas"). * Program.cs — the `source=="static" && HandlerRetainsNoInstance` drop. * AppDomainShutdownSample.cs — `NonAppDomainSubscriber` SPLIT (not deleted): now a CAPTURING lambda (captures `cts`) -> OWN014 (preserves the Codex-defended intent); new `NonCapturingStaticSubscriber` -> silent (the FP this removes). * ownir.py — the OWN014 (capture + DI-escape) branches now carry the "inline lambda — no '-=' handle" note, like OWN001 (the extractor already stamps lambda:true). Method-group captures are unchanged (no note). * LambdaTiersSample.cs — first-class silent tiers: bounded/local + self-owned lambda (the `-=`-impossible-but-bounded shape). * test_ownir.py — pins a lambda static capture -> OWN014 with the note (250/250). * ci.yml — split/added the wpf-extractor assertions accordingly. * subscription-leaks-and-profiles.md — ledgers "static + non-retaining -> silent; static + retaining -> OWN014"; field-notes-patterns.md records the deliberate non-goal (invocation-list growth from repeated non-capturing subscriptions — a call-count shape the region model does not, and should not, express; never covered for the static-method exemption either, so not a regression). Safety: no baselined/pinned finding vanishes silently. The CsvHelper oracle-baseline entries are the CAPTURING cts/resetEvent shape (still OWN014); every OWN014 corpus case (systemevents-region-escape, screentogif-systemevents-leak) is a method-group instance handler (retaining) and stays OWN014 (re-confirmed with the WindowsDesktop ref pack materialized); the one reclassified case (NonAppDomainSubscriber) is explicit in the diff. Closes #199 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| public NonAppDomainSubscriber(System.Threading.CancellationTokenSource cts) | ||
| { | ||
| SomeBus.Pinged += (_, _) => Handle(); // static event, lambda, not AppDomain -> OWN014 | ||
| SomeBus.Pinged += (_, _) => cts.Cancel(); // captures `cts` -> OWN014 |
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis change refines event-subscription analysis for lambda handlers. Static events now remain silent for proven non-retaining lambdas, while retaining handlers continue producing OWN014. Diagnostics identify inline lambdas without detachable ChangesLambda subscription precision
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RoslynSamples
participant OwnSharpExtractor
participant OwnIR
participant CI
RoslynSamples->>OwnSharpExtractor: Analyze lambda event subscriptions
OwnSharpExtractor->>OwnIR: Emit retaining subscription facts
OwnSharpExtractor-->>OwnIR: Suppress non-retaining static subscriptions
OwnIR->>CI: Produce OWN014 diagnostics and messages
CI->>CI: Validate capture-aware and silent cases
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 466-482: Update the CI checks around the silent fixtures to first
verify that NonCapturingStaticSubscriber, LocalBoundedLambda, and
SelfOwnedFieldLambda are present in $RUNNER_TEMP/facts.json, failing if any were
not extracted; then retain the existing assertions that their diagnostics are
absent from $out.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 713fecbd-97ab-48ba-abe0-032a6dec7dd8
📒 Files selected for processing (8)
.github/workflows/ci.ymldocs/notes/field-notes-patterns.mddocs/notes/subscription-leaks-and-profiles.mdfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/samples/AppDomainShutdownSample.csfrontend/roslyn/samples/LambdaTiersSample.csownlang/ownir.pytests/test_ownir.py
…ambda The silent-tier fixtures (LocalBoundedLambda, SelfOwnedFieldLambda) emit no OwnIR component by design, so their negative CI greps passed vacuously if LambdaTiersSample.cs ever stopped being extracted (CodeRabbit, PR #211). The file had no positive artifact to prove it reached the extractor. CodeRabbit's suggested jq presence-guard on facts.json components cannot work here: a silent subscription produces zero components, so the guard would fail CI. Instead, follow the codebase idiom (SemaphoreFieldSample, VoidSubscribeSample) and add a positive control in the same file: - InjectedSourceLambda: a lambda on a ctor-param (injected) publisher whose lifetime is unknown -> OWN001 warning, the injected tier that was missing from this file. It is the only component the extractor emits here, so its OWN001 assert proves the file was extracted and the two silent negatives are no longer vacuous. It also exercises the inline-lambda no-'-=' note on OWN001 (previously pinned only on OWN014). NonCapturingStaticSubscriber was already anchored: it shares AppDomainShutdownSample.cs with the positively-asserted NonAppDomainSubscriber / CapturingShutdownSubscriber OWN014s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG
|
|
||
| public InjectedSourceLambda(Publisher pub) | ||
| { | ||
| pub.Changed += (s, e) => _n++; // injected source -> OWN001 warning |
PhysShell
pushed a commit
that referenced
this pull request
Jul 11, 2026
…abbit) Blanket `|| true` + 2>/dev/null swallowed a real ownir error (rc >= 2): the jq assertion would still fail the job, but the log showed only the assertion text, not the Python error. Mirror the established rc-capture pattern (same file, proj.sarif step): keep stderr, allow the expected rc<=1, fail loud on rc>=2. Per the PR #211 learning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
Merged
8 tasks
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
3 participants
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.
Что и зачем
Issue #199 (subscription precision, lambda-handler tier). Проверив поведение против реального экстрактора локально (поднял .NET SDK в песочнице), обнаружил, что четыре тира по времени жизни источника уже работают для лямбд (static→OWN014, injected→OWN001 warning, bounded/local→silent), но общий static-путь флагал OWN014 на non-capturing лямбде (
SomeBus.Pinged += (_,_) => StaticHandle()), хотя она не удерживает ни одного инстанса — это false positive против нашей же записанной политики (subscription-leaks-and-profiles.md: «static event + capturing handler → error») и несогласовано с исключением static-method (non-capturing method group уже молчит).Почему меняется запиненное поведение — с ссылкой на политику: OWN014-посылка «сильная подписка пиннит подписчика к времени жизни источника» не выполняется для хендлера, который ничего не удерживает (static method с null-target, либо non-capturing лямбда — closure-аналог static-method исключения). Захватывающий (
thisили локал — CsvHelpercts/resetEvent) остаётся OWN014.Состав:
source=="static" && HandlerRetainsNoInstance. Строго консервативен: любой недоказуемо-non-retaining случай (захваченный локал, непрозрачный delegate-typed value) → считается retaining → OWN014. Никогда не расширяется синтаксически (неclsIsStatic, не «все лямбды»).NonAppDomainSubscriberрасщеплён (не удалён): теперь захватывающая лямбда (cts) → OWN014 (сохраняет намерение, которое защищал Codex — CsvHelper-шейп); новыйNonCapturingStaticSubscriber→ silent (FP, который убираем).-=handle», как OWN001 (extractor уже ставитlambda:true). Method-group captures без изменений.-=-impossible-but-bounded).Безопасность (условие «ничего запиненное не исчезает молча»): CsvHelper oracle-baseline записи — это захватывающий
cts/resetEventшейп (остаётся OWN014); каждый OWN014 corpus-кейс (systemevents-region-escape,screentogif-systemevents-leak) — method-group instance handler (retaining) и остаётся OWN014 (перепроверил с материализованным WindowsDesktop ref pack); единственный переклассифицированный кейс (NonAppDomainSubscriber) явно виден в диффе.Про CI: PR трогает
.github/workflows/ci.yml; #204 (тоже правил ci.yml) уже влит в main, ветка от свежего main — конфликта нет.Тип изменения
Как проверено
python tests/run_tests.py— зелено (ownir 250/250, wpf 9/9, corpus 32/32)ruff check .иmypy— чистоthis/локал) → OWN014 с нотой; injected → OWN001 warning; bounded/local/self-owned → silentСвязанные issue
Closes #199
Чеклист
fix:)🤖 Generated with Claude Code
https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
-=handle is available for detachment.Documentation
Tests