feat(#146): returned-fresh publisher provenance — prove the Create→Apply shape bounded#208
Conversation
…pply shape bounded The dual of ownership transfer, interprocedural: a += on a PARAMETER publisher is locally indistinguishable from a DI-singleton bus, so it honestly warns (OWN001, injected tier). The extractor now runs a lazy, memoized, compilation-wide provenance pass: when the subscribing method is private/internal and EVERY visible caller passes a freshly-constructed local that escapes only into the call / its own return (and the callee never lets the param escape), the subscription is stamped source_provenance: "returned_fresh" and the bridge drops it — bounded by the returned publisher's lifetime, same boundedness as a locally-constructed source. Instance-level provenance deliberately beats the type-level DI hop. Precision-first denials (all keep the warning, pinned by ReturnedPublisherSample.cs in CI + test_ownir.py): public candidate, method-group reference, named/ref/omitted argument, non-fresh/non-local argument, any other use of the caller's local (field store, other callee, lambda capture, reassignment), param->param forwarding, zero visible callers. Constructors are excluded by construction (MethodKind gate), so the ctor-injected DI shape can never be silenced. Clears the mined Newtonsoft JsonSerializer.Create over-report (field-notes #8); its oracle-fp-baseline entry is REMOVED so a regression resurfaces in triage instead of being swallowed by the allowlist. Spec'd in OwnIR.md §4 + the JSON schema (additive optional field, no version bump per IR rules). Closes #146 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
|
Warning Review limit reached
Next review available in: 52 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)
📝 WalkthroughWalkthroughAdds interprocedural detection for freshly constructed publishers returned by callers. Matching subscriptions receive ChangesReturned Publisher Provenance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ProvFactory
participant ApplyBounded
participant OwnSharpExtractor
participant OwnIR
Caller->>ProvFactory: construct publisher and call factory
ProvFactory->>ApplyBounded: pass publisher parameter
ApplyBounded->>OwnSharpExtractor: emit subscription candidate
OwnSharpExtractor->>Caller: verify fresh construction and return
OwnSharpExtractor->>OwnIR: emit returned_fresh provenance
OwnIR-->>Caller: suppress bounded subscription finding
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| public static void ApplyPublic(ProvPublisher pub, ProvSettings settings) | ||
| { | ||
| pub.Faulted += settings.Error; // stays the OWN001 warning |
|
|
||
| private static void ApplyMixed(ProvPublisher target, ProvSettings settings) | ||
| { | ||
| target.Mixed += settings.Error; // stays the OWN001 warning |
|
|
||
| private void Wire(ProvBus bus) | ||
| { | ||
| bus.Changed += OnChanged; // stays the OWN001 warning |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.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: 0ae0b5868b
ℹ️ 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".
| if (id.Ancestors().OfType<AnonymousFunctionExpressionSyntax>().Any()) | ||
| return false; |
There was a problem hiding this comment.
When the subscribing helper captures the publisher parameter in a local function, e.g. void Later() { publisher.Error += h; } and then stores Later, this check still accepts the parameter because it only rejects anonymous-function ancestors. The new provenance stamp can then be applied even though the publisher escaped through the local-function closure and may be invoked from a longer-lived root, causing the bridge to drop an OWN001 warning that should remain.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in c3cb656 — a stored local function is a closure exactly like a lambda, and the callee gate only rejected AnonymousFunctionExpressionSyntax. The ancestor walk now also matches LocalFunctionStatementSyntax, bounded at the method's own declaration node (TakeWhile(a => a != mnode)) so a candidate never self-denies. Regression pinned: ProvLocalFuncFactory in ReturnedPublisherSample.cs (deferred.Deferred must keep the OWN001 warning), asserted in the wpf-extractor CI job.
Generated by Claude Code
| if (use.Ancestors().TakeWhile(a => a != scope) | ||
| .OfType<AnonymousFunctionExpressionSyntax>().Any()) |
There was a problem hiding this comment.
If the fresh local is passed to the audited helper from a nested local function, e.g. void Later() => Apply(p, s);, and that local function is stored/delegated, this loop treats the use of p as an allowed target-argument use because it only rejects anonymous functions. That lets a local which escaped via a local-function closure satisfy returned_fresh, so later calls from the stored delegate are silenced as bounded even though p may now be process-lived.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in c3cb656 — same hole on the caller side: the use-walk's TakeWhile(a => a != scope) window now denies on LocalFunctionStatementSyntax as well as anonymous functions, so a target-argument use inside a stored void Wire() => Apply(p, s); no longer counts as bounded. Regression pinned: ProvCallerLocalFuncFactory in ReturnedPublisherSample.cs (later.Later must keep the OWN001 warning), asserted in the wpf-extractor CI job.
Generated by Claude Code
…tes (Codex P2 x2)
A stored local function is a closure exactly like a lambda: it can be invoked
from a longer-lived root after the audited call returns. Both gates rejected
only AnonymousFunctionExpressionSyntax, so a publisher parameter subscribed
inside a stored 'void Later() { p.Event += h; }' (callee side), or a fresh
local passed to the helper from a stored 'void Wire() => Apply(p, s);'
(caller side), could still earn the returned_fresh stamp and silence a real
warning. Both ancestor walks now also match LocalFunctionStatementSyntax,
bounded at the audited node so a method never self-denies.
Regression-pinned by two new denial cases in ReturnedPublisherSample.cs
(deferred.Deferred / later.Later) asserted in the wpf-extractor CI job.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
|
|
||
| private static void ApplyStored(ProvPublisher stored, ProvSettings settings) | ||
| { | ||
| stored.Stored += settings.Error; // stays the OWN001 warning |
|
|
||
| private static void ApplyDeferred(ProvPublisher deferred, ProvSettings settings) | ||
| { | ||
| void Later() { deferred.Deferred += settings.Error; } // captures the param |
|
|
||
| private static void ApplyLater(ProvPublisher later, ProvSettings settings) | ||
| { | ||
| later.Later += settings.Error; // stays the OWN001 warning |
|
@coderabbitai review Generated by Claude Code |
|
✅ Action performedReview finished.
|
|
@coderabbitai review Generated by Claude Code |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/notes/field-notes-patterns.md (1)
260-271: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDenial case list omits local-function closure captures.
The denial case enumeration at lines 265-266 lists "public candidate, method-group reference, mixed callers, field-stored local, param→param forwarding" but omits the two local-function closure capture cases (callee-side
ProvLocalFuncFactoryand caller-sideProvCallerLocalFuncFactory) that are present inReturnedPublisherSample.cs(lines 116-158) and explicitly checked in CI (lines 253-254:"deferred.Deferred","later.Later"). The commit messages confirm these were added as Codex P2 regression safeguards.🤖 Prompt for 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. In `@docs/notes/field-notes-patterns.md` around lines 260 - 271, Update the denial-case enumeration in the documented returned-fresh provenance description to include both local-function closure capture cases: callee-side ProvLocalFuncFactory and caller-side ProvCallerLocalFuncFactory. Mention that these cases deny the proof alongside the existing safeguards, matching ReturnedPublisherSample.cs and the corresponding CI checks.docs/notes/oracle-known-fps.md (1)
49-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDenial case list and summary counts need updating for
#146.Two issues:
The denial case list at lines 55-57 omits the two local-function closure capture cases (callee-side and caller-side) that are present in
ReturnedPublisherSample.cs(lines 116-158) and checked in CI (lines 253-254:"deferred.Deferred","later.Later").The disposition summary table (lines 26-27) is now stale. Moving
serializer.Errorfrom "Baselined FP" to "Fixed in the extractor" means "Fixed" should increase from 6 to 7 and "Baselined FP" should decrease from 5 to 4. The text at lines 36-39 ("5 baselined FPs...8 findings...7 rules") should correspondingly become "4 baselined FPs...7 findings...6 rules" to match the updatedcorpus/oracle-fp-baseline.txt.🤖 Prompt for 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. In `@docs/notes/oracle-known-fps.md` around lines 49 - 57, The denial-case list in docs/notes/oracle-known-fps.md must include the callee-side and caller-side local-function closure captures, identified by “deferred.Deferred” and “later.Later” in ReturnedPublisherSample.cs. Update the disposition counts to Fixed 7 and Baselined FP 4, and revise the summary text from 5 baselined FPs, 8 findings, and 7 rules to 4 baselined FPs, 7 findings, and 6 rules.
🤖 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.
Nitpick comments:
In `@docs/notes/field-notes-patterns.md`:
- Around line 260-271: Update the denial-case enumeration in the documented
returned-fresh provenance description to include both local-function closure
capture cases: callee-side ProvLocalFuncFactory and caller-side
ProvCallerLocalFuncFactory. Mention that these cases deny the proof alongside
the existing safeguards, matching ReturnedPublisherSample.cs and the
corresponding CI checks.
In `@docs/notes/oracle-known-fps.md`:
- Around line 49-57: The denial-case list in docs/notes/oracle-known-fps.md must
include the callee-side and caller-side local-function closure captures,
identified by “deferred.Deferred” and “later.Later” in
ReturnedPublisherSample.cs. Update the disposition counts to Fixed 7 and
Baselined FP 4, and revise the summary text from 5 baselined FPs, 8 findings,
and 7 rules to 4 baselined FPs, 7 findings, and 6 rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e39336d5-2263-435c-9f01-98a8d5544fd2
📒 Files selected for processing (10)
.github/workflows/ci.ymlcorpus/oracle-fp-baseline.txtdocs/notes/field-notes-patterns.mddocs/notes/oracle-known-fps.mdfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/samples/ReturnedPublisherSample.csownlang/ownir.pyspec/OwnIR.mdspec/ownir.schema.jsontests/test_ownir.py
…nction denial cases CodeRabbit nitpicks on #208: the returned-fresh denial-case enumerations in field-notes #8 and oracle-known-fps predate c3cb656, so they omitted the two local-function closure captures; and the disposition summary still counted serializer.Error as baselined (Fixed 6->7, Baselined 5->4, 8 findings/7 rules -> 7 findings/6 rules). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
3 participants
Что и зачем
Issue #146: подписка
+=на publisher-ПАРАМЕТР локально неотличима от DI-singleton шины, поэтому честно даёт OWN001-warning (injectedtier). Экстрактор теперь запускает ленивый мемоизированный общекомпиляционный provenance-проход — дуал ownership transfer (D5): если подписывающий метод private/internal и каждый видимый вызывающий передаёт свежесконструированный локал, который не покидает область иначе как в этот вызов и собственныйreturn(а callee не выпускает параметр дальше), подписка штампуетсяsource_provenance: "returned_fresh"и бридж её скипает — она ограничена временем жизни возвращённого publisher'а, та же bounded-семантика, что у локально сконструированного источника. Инстанс-факт сознательно бьёт type-level DI-хоп (даже если тип publisher'а зарегистрирован singleton'ом, ЭТОТ экземпляр создан вызывающим, а не разрешён из контейнера).Precision-first отказы — все сохраняют честный warning, запинены сэмплом
frontend/roslyn/samples/ReturnedPublisherSample.cs(CIwpf-extractor) и проверками вtests/test_ownir.py: public/protected кандидат, method-group ссылка, named/ref/omitted аргумент, не-свежий/не-локальный аргумент, любое иное использование локала (запись в поле, другой callee, лямбда-захват, переприсваивание), param→param форвардинг, ноль видимых вызовов. Конструкторы исключены самим MethodKind-гейтом, так что DI-форму через ctor заглушить невозможно по построению.Закрывает майненный ньютоновский over-report (
JsonSerializer.Create→ApplySerializerSettings, field-notes #8). Его запись вcorpus/oracle-fp-baseline.txtудалена сознательно: при регрессии provenance-прохода находка должна всплыть в триаж-очереди оракула, а не быть молча проглоченной allowlist'ом. Поле специфицировано вspec/OwnIR.md§4 иspec/ownir.schema.json(аддитивное опциональное — без бампаOWNIR_VERSIONпо IR-правилам эволюции).Тип изменения
Как проверено
python tests/run_tests.py— зелено после ребейза на main c README front door: 20s landing, 3 case studies, FP policy page, C# gallery pairs, CI SHA-pinning #204/feat(P-007): pin POOL005 full-length view stored into a field; defer object-level escape #206 (ownir 255/255, включая 7 новых provenance-проверок: bounded→silent, unknown-значение→warning, premise-guard DI-эскалации, instance-beats-DI, lowered sketch парсится, load() отвергает не-строку)ruff check .иmypy— чисто (mypy --strict по ownlang)wpf-extractor: bounded-кейс молчит со штампомreturned_freshв фактах, все четыре denial-кейса сохраняют OWN001-warning.Связанные issue
Closes #146. Refs field-notes-patterns.md #8,
docs/notes/oracle-known-fps.md(запись обновлена: FP → fixed).Чеклист
feat:)🤖 Generated with Claude Code
https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation