What is this pt.2#9
Merged
Merged
Conversation
Three structural fixes to the OwnIR bridge so adding patterns (timers, IDisposable fields, region/escape facts) is additive rather than a rewrite: - Map verdicts back to C# by Diagnostic.subject (a structured name#line identity), not by regex-scraping the human message. Generic `acquire` now stamps `sym.origin` like buffers already did, so OWN001 on a subscription carries its handle. Changing a message no longer silently drops a finding. - Fail loudly instead of silently dropping: any error the core reports on the lowered facts that the bridge cannot attribute to a known handle now raises OwnIRError with an actionable message — a swallowed leak is the worst outcome for a leak checker. - Version the fact contract (`ownir_version`/OWNIR_VERSION). The extractor stamps it and the core rejects a mismatch at load with a clear message, rather than mis-reading a drifted vocabulary. Malformed JSON and bad facts now surface as one-line CLI errors, not tracebacks. Also split __main__.check_module out of _collect as the AST-level entry to the one checker, so the next pattern can build a Module directly instead of re-serialising to .own text and re-parsing it.
A long design discussion covered far more ground than the current spec/ proposals reflect, and such threads evaporate. Systematise it so every idea raised is on the record for consideration (drafts, not commitments). - docs/ROADMAP.md — the strategy hub: the one-sentence pitch, design philosophy (one checker; bug-driven expansion; narrow intraprocedural C# frontend), P0–P3 priorities, the five milestones, the "what static analysis can/can't catch" reality matrix, why not a full Rust-style borrow checker, and an honesty note on the (proxy, not measured) error statistics. - P-004 WPF/UI lifetime leak profile (extends P-001: timers, IDisposable subscription fields, ignored Subscribe, escape→OWN014). - P-005 IDisposable ownership profile (local/field not disposed, double dispose, use-after-dispose, transfer). - P-006 DI lifetime / captive dependency (DI001–003; WeakReference is not the fix; IServiceScopeFactory remedy). - P-007 ArrayPool/Span borrow-view profile (POOL001–005; replay corpus). - P-008 Effects & resources (declared use !Db/Log/Clock, capabilities, layer policies; Wybe/Plasma/Mercury/Clean background). - P-009 No-GC / allocation-free regions (OwnNoGc policy; OWN-GC001–007). - P-010 Richer type disciplines (branded/refinement/units/typestate now; dependent/GADT/HKT/row/existential/modal deferred). - P-011 Editor tooling & syntax highlighting (TextMate → CLI JSON → LSP → semantic tokens → tree-sitter). - P-012 Real-world bug corpus & GitHub/Roslyn mining pipeline. - proposals/README.md index updated; all link back to ROADMAP. Docs only — no code or behaviour change.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesOwnIR Versioning and Diagnostic Attribution Pipeline
Test Samples, Fixtures, and CI Validation
Roadmap and Proposal Documentation
Sequence Diagram(s)sequenceDiagram
participant Roslyn as Roslyn<br/>Extractor
participant JSONFacts as facts.json<br/>(ownir_version=0)
participant Load as load()
participant ToOwn as to_own()
participant ParseCheck as parse &<br/>check_facts()
participant HandleOf as _handle_of()
participant Finding as Finding<br/>[resource: ...]
Roslyn->>JSONFacts: emit subscriptions<br/>+ resource kinds<br/>+ ownir_version=0
JSONFacts->>Load: read JSON
Load->>Load: validate ownir_version<br/>components/subscriptions
Load-->>ToOwn: validated facts dict
ToOwn->>ToOwn: map resource→<br/>synthetic type<br/>preserve metadata
ToOwn-->>ParseCheck: lowered OwnLang<br/>module + handle map
ParseCheck->>ParseCheck: parse module
ParseCheck->>ParseCheck: check with core
ParseCheck->>HandleOf: extract from<br/>Diagnostic.subject
HandleOf-->>ParseCheck: subscription handle
ParseCheck->>Finding: create with<br/>resource kind tag
Finding-->>ParseCheck: Finding rendered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/proposals/P-008-effects-and-resources.md (1)
84-84: 💤 Low valueMinor style improvements (LanguageTool hints).
Three small wordiness and punctuation notes for polish:
- Line 84:
"in search of a bug"is slightly wordy. Consider:"not a philosophical purity analyzer chasing a bug."or similar.- Line 146: The word
"exactly"is used twice in close proximity (also line 160). Consider removing one occurrence for variety.- Line 159: The section has 14 exclamation marks across 5962 characters, creating visual heaviness. This is partly due to the
!effect syntax (e.g.,use !Db), but consider easing one or two punctuation marks to improve readability—e.g., change the closing question (line 161) to a period.Also applies to: 146-146, 159-159
🤖 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/proposals/P-008-effects-and-resources.md` at line 84, In docs/proposals/P-008-effects-and-resources.md, apply three style refinements for better readability and variety: At line 84, replace the phrase "in search of a bug" with a more concise alternative such as "chasing a bug" to reduce wordiness. At line 146, remove one occurrence of the word "exactly" to avoid repetition with the same word appearing at line 160. At line 159, reduce the visual heaviness of exclamation marks by changing the closing question mark on line 161 to a period, which will improve overall readability while maintaining the natural effect syntax throughout the document.
🤖 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 `@docs/proposals/P-008-effects-and-resources.md`:
- Around line 55-70: The document lists five behaviors that should be caught
(including "Unpaired protocol" for Rent without Return and BeginTransaction
without Commit/Rollback) but only provides four diagnostic codes
(EFF001–EFF004). Add a new diagnostic code EFF005 to the diagnostics section to
explicitly cover the unpaired protocol violation case, with a description that
references the specific scenarios like Rent without Return and BeginTransaction
without Commit/Rollback.
In `@ownlang/ownir.py`:
- Around line 153-156: The function's return type annotation is `str | None`,
but the `getattr(diag, "subject", None)` call on line 153 returns an untyped
`Any` in mypy strict mode, causing a type violation when `.split()` is called.
Add an `isinstance(subject, str)` check in addition to the existing `if not
subject` check to narrow the type and satisfy mypy strict mode. The condition
should verify both that subject is truthy and that it is specifically a string
type before proceeding to the `.split()` call.
---
Nitpick comments:
In `@docs/proposals/P-008-effects-and-resources.md`:
- Line 84: In docs/proposals/P-008-effects-and-resources.md, apply three style
refinements for better readability and variety: At line 84, replace the phrase
"in search of a bug" with a more concise alternative such as "chasing a bug" to
reduce wordiness. At line 146, remove one occurrence of the word "exactly" to
avoid repetition with the same word appearing at line 160. At line 159, reduce
the visual heaviness of exclamation marks by changing the closing question mark
on line 161 to a period, which will improve overall readability while
maintaining the natural effect syntax throughout the document.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 524e9ac4-e342-4869-be8c-8d4fab94d9c0
📒 Files selected for processing (18)
docs/ROADMAP.mddocs/proposals/P-001-csharp-extractor.mddocs/proposals/P-004-wpf-lifetime-profile.mddocs/proposals/P-005-idisposable-ownership.mddocs/proposals/P-006-di-lifetimes.mddocs/proposals/P-007-arraypool-span.mddocs/proposals/P-008-effects-and-resources.mddocs/proposals/P-009-nogc-regions.mddocs/proposals/P-010-type-disciplines.mddocs/proposals/P-011-editor-tooling.mddocs/proposals/P-012-bug-corpus-mining.mddocs/proposals/README.mdfrontend/roslyn/OwnSharp.Extractor/Program.csownlang/__main__.pyownlang/cfg.pyownlang/ownir.pytests/fixtures/ownir/sample.facts.jsontests/test_ownir.py
A Tick/Elapsed handler on a started timer is a Timer resource: the running
timer strong-refs the handler's owner, so an undetached one leaks it (OWN001,
tagged [resource: timer]). Released by a matching `-=` OR a `Stop()` on the
same receiver (e.g. `_timer.Stop()` in Dispose).
- ownir.py: optional `resource` field per subscription ("subscription" default
| "timer"); a Timer resource in the prelude (kind "timer"); timer-specific
finding message + kind tag. The field is additive, so it does NOT bump
ownir_version — an older core just reads every entry as a subscription.
- Roslyn extractor: classify `Tick`/`Elapsed` handlers as timers; a timer is
released by `-=` or a `Stop()` on the same receiver; emit `resource`.
- Sample TimerViewModel.cs (leaking timer + Stop()-in-Dispose clean one) and a
hand-written timer.facts.json fixture; test_ownir pins the timer leak,
the [resource: timer] tag, and silence on the stopped timer.
- wpf-extractor CI job runs the timer sample and asserts the timer leak +
[resource: timer] tag, and that the stopped timer stays silent.
- Docs: P-004/P-001/ROADMAP mark WPF002 built.
Python bridge verified locally (11/11 ownir checks); the extractor is
CI-validated (dotnet is CI-only).
An IDisposable field the class constructs (`new`) but never disposes leaks it (OWN001, tagged [resource: disposable field]). "Owned, not injected" = the field is assigned a `new` in this class; "released" = a `<field>.Dispose()` call exists somewhere in the class. - ownir.py: new "disposable" resource kind (Disposable resource, kind "disposable field"); optional `type` names the field's declared type in the message. Additive — no ownir_version bump. Docstring now describes `subscriptions` as the (historically named) owned-resource list with a `resource` discriminator. - Roslyn extractor: detect disposable fields by a curated type heuristic (syntax-only — no semantic model), gated on the class `new`ing the field so injected/borrowed disposables are not flagged; released iff disposed. - Sample DisposableFieldViewModel.cs (leaking CTS + one disposed in Dispose) and a disposable.facts.json fixture; test_ownir pins the leak, the type in the message, the [resource: disposable field] tag, and silence on the disposed one. CI asserts the same on the real extractor output. - Docs: P-004/P-005/ROADMAP mark WPF003 built. Python bridge verified locally (14/14); extractor CI-validated.
A `X.Subscribe(...)` whose IDisposable result is ignored (the call stands as a bare statement, not assigned/returned/added) drops the subscription token, so it is never disposed and leaks (OWN001, tagged [resource: subscription token]). - ownir.py: new "subscribe" resource kind (maps to the Subscription resource / subscription-token tag, with its own "result ignored" message). Additive — no ownir_version bump. - Roslyn extractor: flag member-access `x.Subscribe(...)` calls whose parent is an ExpressionStatement (result discarded); member-access only, to avoid bare void `Subscribe(...)` helpers. A captured-and-disposed token is not flagged. - Sample MessengerViewModel.cs (ignored Subscribe + captured/disposed clean one) and a subscribe.facts.json fixture; test_ownir pins the leak, the "ignored" message, and the [resource: subscription token] tag. CI asserts the same on the real extractor output. - Docs: P-004 marks WPF004 built. Python bridge verified locally (16/16); extractor CI-validated.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ownlang/ownir.py (1)
197-200:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winmypy --strict still failing:
getattrreturnsAny, so the return type is violated.The pipeline failure on line 200 is the same issue flagged in the prior review. The suggested fix (adding
isinstance(subject, str)guard) has not been applied yet. This blocks CI.Suggested fix
def _handle_of(diag: object) -> str | None: subject = getattr(diag, "subject", None) - if not subject: + if not isinstance(subject, str) or not subject: return None return subject.split("#", 1)[0]🤖 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 `@ownlang/ownir.py` around lines 197 - 200, The getattr call on the `diag` object returns `Any` type, which causes mypy --strict to fail when calling the `.split()` method on `subject` without proper type narrowing. Add an `isinstance(subject, str)` guard after the existing falsy check to narrow the type from `Any` to `str` before calling `.split("#", 1)[0]` on it. This ensures the function satisfies strict type checking while maintaining the same logic.Source: Pipeline failures
🤖 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 `@tests/test_ownir.py`:
- Around line 153-154: The test assertion at line 153 in the test_ownir.py file
expects the disposable field finding to be at line 11, but the _cts field is
actually declared at line 10 in DisposableFieldViewModel.cs, which is what the
CI correctly reports. Update the expected line number in the assertion from 11
to 10 to match the actual field declaration location.
---
Duplicate comments:
In `@ownlang/ownir.py`:
- Around line 197-200: The getattr call on the `diag` object returns `Any` type,
which causes mypy --strict to fail when calling the `.split()` method on
`subject` without proper type narrowing. Add an `isinstance(subject, str)` guard
after the existing falsy check to narrow the type from `Any` to `str` before
calling `.split("#", 1)[0]` on it. This ensures the function satisfies strict
type checking while maintaining the same logic.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ebe7aa7a-459c-43ba-9c55-2f7b2b0d30c8
📒 Files selected for processing (9)
.github/workflows/ci.ymldocs/ROADMAP.mddocs/proposals/P-004-wpf-lifetime-profile.mddocs/proposals/P-005-idisposable-ownership.mdfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/samples/DisposableFieldViewModel.csownlang/ownir.pytests/fixtures/ownir/disposable.facts.jsontests/test_ownir.py
✅ Files skipped from review due to trivial changes (4)
- tests/fixtures/ownir/disposable.facts.json
- docs/ROADMAP.md
- docs/proposals/P-005-idisposable-ownership.md
- docs/proposals/P-004-wpf-lifetime-profile.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci.yml
- frontend/roslyn/OwnSharp.Extractor/Program.cs
An ArrayPool/MemoryPool buffer Rent'ed but never Return'ed leaks (pool exhaustion / GC pressure) — OWN001, tagged [resource: pooled buffer]. The first finding of the flagship borrow-view profile, in the same flat acquire/release model as the WPF resources. - ownir.py: new "pool" resource kind (PooledBuffer resource, kind "pooled buffer", Rent/Return verbs). Additive — no ownir_version bump. - Roslyn extractor: detect `<pool>.Rent(...)` assigned to a variable/field (receiver text contains "pool"); released iff a `Return(buf)` with that identifier exists. Matched PER MEMBER so a `buf` returned in one method does not mask a same-named leak in another. - Sample PooledBufferSample.cs (leaky Rent + rent/return-in-finally clean one) and a pool.facts.json fixture; test_ownir pins the leak, the rented/returned message, and the [resource: pooled buffer] tag. CI asserts the leaky buffer is reported and the returned one is silent. - Docs: P-007/ROADMAP mark POOL001 built. Python bridge verified locally (18/18); extractor CI-validated.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/test_ownir.py (1)
157-158:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix disposable-field expected line number (off by one).
At Line 157, the assertion still expects line
11, but the finding location is line10for_ctsinDisposableFieldViewModel.cs. This will fail the test even when behavior is correct.Proposed fix
- if (d0.file, d0.line, d0.code) != ("DisposableFieldViewModel.cs", 11, "OWN001"): + if (d0.file, d0.line, d0.code) != ("DisposableFieldViewModel.cs", 10, "OWN001"):🤖 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 `@tests/test_ownir.py` around lines 157 - 158, The test assertion in the disposable field check is expecting the wrong line number. The assertion that validates d0.line should expect line 10 instead of line 11, as the _cts field in DisposableFieldViewModel.cs is located at line 10, not line 11. Update the hardcoded line number in the condition that checks (d0.file, d0.line, d0.code) to use 10 instead of 11.Source: Pipeline failures
🤖 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 `@frontend/roslyn/OwnSharp.Extractor/Program.cs`:
- Around line 200-205: The Return detection in the foreach loop iterating over
InvocationExpressionSyntax is too restrictive compared to the Rent detection
logic. Currently it only matches bare IdentifierNameSyntax arguments in the
invocation, which means it won't recognize qualified member access patterns like
this._buf. Instead of checking if the argument expression is an
IdentifierNameSyntax and extracting its text directly, use the FieldName()
method (which already handles both simple and qualified identifier patterns) to
extract the argument name and add it to the returned collection, matching the
pattern used for Rent detection.
---
Duplicate comments:
In `@tests/test_ownir.py`:
- Around line 157-158: The test assertion in the disposable field check is
expecting the wrong line number. The assertion that validates d0.line should
expect line 10 instead of line 11, as the _cts field in
DisposableFieldViewModel.cs is located at line 10, not line 11. Update the
hardcoded line number in the condition that checks (d0.file, d0.line, d0.code)
to use 10 instead of 11.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8dda99f2-bc98-4485-beaa-76137dc76659
📒 Files selected for processing (11)
.github/workflows/ci.ymldocs/ROADMAP.mddocs/proposals/P-004-wpf-lifetime-profile.mddocs/proposals/P-007-arraypool-span.mdfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/samples/MessengerViewModel.csfrontend/roslyn/samples/PooledBufferSample.csownlang/ownir.pytests/fixtures/ownir/pool.facts.jsontests/fixtures/ownir/subscribe.facts.jsontests/test_ownir.py
✅ Files skipped from review due to trivial changes (5)
- tests/fixtures/ownir/subscribe.facts.json
- tests/fixtures/ownir/pool.facts.json
- docs/ROADMAP.md
- docs/proposals/P-007-arraypool-span.md
- docs/proposals/P-004-wpf-lifetime-profile.md
🚧 Files skipped from review as they are similar to previous changes (1)
- ownlang/ownir.py
A local the method `new`s of an IDisposable type, never disposed and not guarded by `using` (and not returned/passed/assigned out), leaks — OWN001, tagged [resource: disposable]. The local analog of WPF003's owned fields; the canonical CA2000 leak. - ownir.py: new "local-disposable" resource kind (Disposable resource, tag "disposable", with its own message). Additive — no ownir_version bump. - Roslyn extractor: per-member detection of `new`'d IDisposable locals; released iff disposed; conservatively excludes `using`-guarded locals and ones whose ownership may leave the scope (returned / passed as an argument / assigned out — transfer is ambiguous syntactically, P-005 D5). - Sample LocalDisposableSample.cs (leaky local + a `using` one + a returned/ transferred one) and a local_disposable.facts.json fixture; test_ownir pins the leak, the type in the message, and the [resource: disposable] tag. CI asserts the leaky local is reported and the using/returned ones are silent. - Docs: P-005 marks D1 built (and D2 via WPF003). Python bridge verified locally (20/20); extractor CI-validated.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 167-172: The local-disposable CI assertion block is incomplete
compared to other resource checks. It currently only verifies the leak text
"local IDisposable 'leaky'" and checks that certain strings are absent, but it
does not assert the resource-kind tag "resource: disposable" or the source file
"LocalDisposableSample.cs". To achieve parity with other resource checks and
prevent kind-mapping regressions from slipping through, add additional grep
assertions in this block to verify both the resource-kind tag and the source
file are present in the output, similar to what is done in corresponding checks
for other resource types.
In `@frontend/roslyn/OwnSharp.Extractor/Program.cs`:
- Around line 234-239: The disposal detection logic in the foreach loop
iterating through InvocationExpressionSyntax only checks for the "Dispose"
method name, but since IsDisposableType() includes IAsyncDisposable types, it
must also detect DisposeAsync calls to avoid false positives. Modify the
condition that checks m.Name.Identifier.Text == "Dispose" to also match
"DisposeAsync", either by using an OR condition to check for both method names
or by extending the check to accommodate async disposal patterns.
In `@frontend/roslyn/samples/LocalDisposableSample.cs`:
- Around line 8-21: The sample code shows a line-anchor drift where the Leaky
method's MemoryStream allocation and the Guarded method's MemoryStream
allocation have shifted to different line numbers than what the fixture contract
expects. To fix this, you must realign the contract by either updating the
fixture's asserted line locations to match the current positions of the
allocations (Line 12 for Leaky and Line 20 for Guarded), or by adding or
removing blank lines in the sample code to restore the allocations to their
originally expected line numbers (10 and 18). Choose whichever approach
maintains consistency with the rest of your test infrastructure and fixture
contracts.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 27b3f3b5-0f38-4f8a-befe-f2a20519a9c3
📒 Files selected for processing (7)
.github/workflows/ci.ymldocs/proposals/P-005-idisposable-ownership.mdfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/samples/LocalDisposableSample.csownlang/ownir.pytests/fixtures/ownir/local_disposable.facts.jsontests/test_ownir.py
✅ Files skipped from review due to trivial changes (2)
- tests/fixtures/ownir/local_disposable.facts.json
- docs/proposals/P-005-idisposable-ownership.md
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_ownir.py
- ownlang/ownir.py
Comment on lines
+8
to
+21
| // A MemoryStream created but never disposed and not in a `using`: leak. The | ||
| // core reports OWN001 [resource: disposable] at the declaration. | ||
| public static long Leaky() | ||
| { | ||
| var leaky = new MemoryStream(); | ||
| leaky.WriteByte(1); | ||
| return leaky.Length; // never disposed => leak | ||
| } | ||
|
|
||
| // `using` guarantees disposal — not flagged. | ||
| public static long Guarded() | ||
| { | ||
| using var guarded = new MemoryStream(); | ||
| guarded.WriteByte(1); |
There was a problem hiding this comment.
Line-anchor drift breaks the sample↔fixture contract for local-disposable checks.
The leaky allocation now occurs at Line 12 (and guarded allocation at Line 20), but the fixture contract is anchored to lines 10/18, and CI logs already show the leak reported at 12-12. This will cause brittle failures in checks that assert exact C# locations.
Please realign one side of the contract: either update the fixture/asserted locations to 12/20, or keep the sample’s anchored statements at the expected lines.
🧰 Tools
🪛 GitHub Actions: CI / 4_C# leak extractor (Roslyn) - OwnIR - core.txt
[error] 12-12: OWN001: local IDisposable 'leaky' (type 'MemoryStream') is created but never disposed (leak) [resource: disposable]
🪛 GitHub Actions: CI / C# leak extractor (Roslyn) - OwnIR - core
[error] 12-12: OWN001: local IDisposable 'leaky' (type 'MemoryStream') is created but never disposed (leak) [resource: disposable]
🤖 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 `@frontend/roslyn/samples/LocalDisposableSample.cs` around lines 8 - 21, The
sample code shows a line-anchor drift where the Leaky method's MemoryStream
allocation and the Guarded method's MemoryStream allocation have shifted to
different line numbers than what the fixture contract expects. To fix this, you
must realign the contract by either updating the fixture's asserted line
locations to match the current positions of the allocations (Line 12 for Leaky
and Line 20 for Guarded), or by adding or removing blank lines in the sample
code to restore the allocations to their originally expected line numbers (10
and 18). Choose whichever approach maintains consistency with the rest of your
test infrastructure and fixture contracts.
Source: Pipeline failures
…ge cases Verified each of the 7 review threads against the code; all valid, all minimal: - mypy --strict (the actual red CI): _handle_of returned Any from getattr; narrow with isinstance(subject, str) before .split(). Confirmed clean with `mypy` + `ruff check .`. - extractor: ArrayPool Return detection now uses FieldName() so `pool.Return(this._buf)` matches the rented `this._buf` (was a false-positive leak for qualified fields). - extractor: disposal detection (both owned-field and local) now matches `DisposeAsync()` as well as `Dispose()` — IsDisposableType already accepts IAsyncDisposable, so an awaited DisposeAsync was a false positive. - CI: local-disposable check now also asserts the source file and the exact `[resource: disposable]` tag (parity with the other resource checks; the trailing `]` avoids matching `disposable field`). - P-008: add EFF005 for the unpaired-protocol case the prose already lists. - fixtures/tests: realign the hand-written disposable/local fixture line anchors to the actual sample declarations (10/21 and 12) so the fixtures faithfully mirror the samples. Tests 20/20; mypy + ruff clean locally.
… CI) The C# leak-extractor CI job has been red since WPF003 landed: IsDisposableType matched `*Timer` (and `DispatcherTimer`/`Timer`), so a DispatcherTimer field was reported as a "disposable field" needing Dispose(). DispatcherTimer is not even IDisposable, and a timer's release is Stop()/detach (the WPF002 pattern) — so this double-reported every timer and false-positived CleanTimerViewModel (a correctly Stop()'d timer), breaking the "stopped timer not reported" assertion. Fix: exclude timer types from the disposable-field/local heuristic; timers are owned by the WPF002 timer pattern. Caught only in CI (dotnet is CI-only) — the local Python bridge suite cannot exercise the extractor heuristic.
This was referenced Jun 15, 2026
This was referenced Jun 24, 2026
PhysShell
pushed a commit
that referenced
this pull request
Jul 15, 2026
…port Real N2-D1b evidence (qodec-n2d1b-miner-pilot.yml CI runs #9-#11) showed Gradle's own daemon architecture always needs SOME loopback TCP port, chosen by the OS -- two independent argv/env-only workarounds to avoid this both failed identically with "java.net.BindException: Permission denied". Sandboy's own tcp_bind is a fixed port list (Landlock scopes ports, not addresses, per this repo's own README) with no way to express "any port, loopback only", so there is no small, correct port allowlist to add for this case. Add network_enforcement_mode: Option<String> to Policy, with exactly one accepted value, "outer-netns-loopback-only". When set (and only when tcp_connect/tcp_bind are both empty -- a fixed port list alongside this mode is contradictory and a hard error): Landlock's own TCP bind/connect mediation is skipped entirely for that ruleset, relying solely on the caller having already placed the process inside a network namespace with no route beyond loopback. Filesystem, seccomp, and env_clear mediation are completely unaffected -- only the .handle_access(AccessNet::...) call becomes conditional. Left unset (the default): byte-for-byte the same behavior as every prior policy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0146FG6sjNiVaQEbCUZkzSy6
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
2 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.
ideas ideas
Summary by CodeRabbit
ownir_version) with stricter fact-to-checker lowering..Stop()-released subscriptions and clearer resource-kind tagging.Subscribe(), pooled buffers, and localIDisposablehandling (including CI coverage).