fix(extractor): curated app-scoped source exemption for the Application subscriber (Closes #228)#232
Conversation
…on subscriber (#228) An App : Application subscribing to PaletteHelper().GetThemeManager() — an app-scoped, process-lived instance reached through a resolver call instead of a literal static member — was tiered "injected" and warned OWN001 (the MaterialDesign MahMaterialDragablzMashUp App.xaml.cs FP from the issue #201 sweep). The subscription promotes nothing: the subscriber IS the process-lived object and the source is bound to the app's own state. The narrowing inverts every property that made the rejected clsIsStatic broadening unsound (oracle-known-fps.md "Rejected approaches"): - subscriber gate unchanged — byte-for-byte the existing clsIsApp; - source loosens only to a CURATED resolver allowlist (exactly PaletteHelper.GetThemeManager for now; grown one confirmed sibling at a time, like the #223 weak-event list), resolved semantically through the receiver's local binding (var-initializer or is-pattern designation, casts/! peeled) — any unprovable step keeps today's honest warning; - handler must be a METHOD GROUP of the App class itself — a lambda is rejected outright, closing the captures-a-local hole that sank clsIsStatic. Pinned by AppScopedSourceSample.cs: two silent positives (is-pattern local + direct-invocation receiver) and three flagged controls (non-App subscriber, lambda handler, non-curated resolver), each edge of the gate, wired into the wpf-extractor CI job. The rejected-approaches note gains a "what DID ship next to it" section distinguishing this narrowing. Verified locally with the real extractor (.NET 8): the sample yields exactly the three control warnings; a full-sample-set diff of old vs new extractor output is byte-identical (zero regression). Gates: run_tests 276/276, ruff, mypy, yaml all green. Closes #228 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
![github-advanced-security[bot]](https://cdn.github.com/in/57789?s=60&v=4){kind=small}
| { | ||
| var helper = new PaletteHelper(); | ||
| if (helper.GetThemeManager() is { } themeManager) | ||
| themeManager.ThemeChanged += OnTheme; // OWN001 warning |
| { | ||
| var counter = new int[1]; | ||
| if (new PaletteHelper().GetThemeManager() is { } themeManager) | ||
| themeManager.ThemeChanged += (s, e) => counter[0]++; // OWN001 warning |
|
Warning Review limit reached
Next review available in: 22 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 (4)
✨ 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 |
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f403bdb656
ℹ️ 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".
… (Codex review) Both Codex P2s verified real, both fixed: - Handler target must be `this`: a method group qualified with ANOTHER instance of the same App-derived type (`_twin!.OnTheme`) has the right ContainingType but pins that other instance, not the process-lived singleton. IsOwnMethodGroupHandler now accepts only a bare identifier (implicit this) or an explicit `this.X` receiver. - Stale resolver binding: `var tm = helper.GetThemeManager(); tm = injectedBus; tm.ThemeChanged += H` passed on the declaration-site initializer alone. IsNeverReassigned kills the proof on ANY assignment to the local or ref/out escape anywhere in the enclosing member — conservative (order-insensitive), worst case keeps today's warning. Two new flagged controls in AppScopedSourceSample.cs (TwinApp, ReassignApp) + CI assertions. Verified with the real extractor: all five controls warn, both positives silent; full-sample-set output still byte-identical to main. Refs #228 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
| public void OnStartup() | ||
| { | ||
| if (new OtherHelper().GetOtherService() is { } service) | ||
| service.ThemeChanged += OnTheme; // OWN001 warning |
| public void OnStartup() | ||
| { | ||
| if (new PaletteHelper().GetThemeManager() is { } themeManager) | ||
| themeManager.ThemeChanged += _twin!.OnTheme; // OWN001 warning |
| { | ||
| var tm = new PaletteHelper().GetThemeManager()!; | ||
| tm = _injectedBus; | ||
| tm.ThemeChanged += OnTheme; // OWN001 warning |
Same-tail collision as before: main's #225 empty-Dispose block (sample + assertions) and this branch's #228 app-scoped block appended to the same wpf-extractor job tail. Kept both, single OK line with both suffixes. Program.cs auto-merged (additive helpers in different regions). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
3 participants
Что и зачем
Issue #228 (под-задача (b) родительского #221, самая деликатная по soundness):
App : Application, подписывающийся наPaletteHelper().GetThemeManager()— app-scoped, процессно-живой инстанс, доставаемый через resolver-вызов, а не литеральныйstatic-член, — тонул в общийsource == "injected"и давал OWN001-warning (FP MaterialDesignMahMaterialDragablzMashUp/App.xaml.csиз sweep'а #201). Подписка ничего не промоутит: подписчик сам процессно-живой, источник привязан к состоянию самого приложения. Narrowing инвертирует каждое свойство, утопившее отвергнутыйclsIsStatic(см. «Rejected approaches»): гейт подписчика — байт-в-байт прежнийclsIsApp; источник — только курируемый allowlist resolver'ов (ровноPaletteHelper.GetThemeManager, расширение — по одному подтверждённому родственнику, как в #223), резолвится семантически через local-binding ресивера (var-инициализатор /is-pattern, касты и!очищаются), любой недоказуемый шаг = сегодняшний честный warning; хендлер — только method group самого App-класса, лямбда отвергается сразу (дыра «захватил локал — пришпилил навсегда» закрыта by construction).Тип изменения
Как проверено
python tests/run_tests.py— 276/276ruff check .иmypyNotAnApp,LambdaApp,CuratedOnlyApp), оба позитива (App,DirectApp) молчат; полный дифф старого/нового экстрактора по всем существующим сэмплам — байт-в-байт идентичен (0 регрессий); точные CI-ассерты прогнаны против живого выводаСвязанные issue
Closes #228. Родитель: #221 (части (a) #227 и (c) #229 — отдельными PR). Найдено sweep'ом #201.
Чеклист
AppScopedSourceSample.cs: 2 позитива (pattern-local и direct-receiver формы) + 3 негативных контроля на каждую грань гейта, CI-ассерты в обе стороныdocs/notes/oracle-known-fps.md: секция «What DID ship next to it (issue Self-owned source (b): Application-derived subscriber with a curated app-scoped source (careful clsIsApp loosening) #228)» рядом с отвергнутымclsIsStatic, почему это не он🤖 Generated with Claude Code
https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
Generated by Claude Code