level-2: add --severity flag + PowerShell own-check.ps1 #1 --severity {error,warning} (default error): threaded through the core renderer (Finding.render/_github/_msbuild + render_finding), the ownir CLI (--severity, validated; only valid for ownir), own-check.sh, and the composite Action's new 'severity' input. A presentation choice — the finding is still the core's verdict — so a build can show leaks as advisory warnings instead of failing. Resolves P-013 open question #1. #2 scripts/own-check.ps1: a PowerShell twin of own-check.sh so Windows/VS users without bash get the same one-command pipeline (same flags/output). #11
Merged
Conversation
…, CI) Covers the three usage surfaces with copy-paste steps: terminal via own-check.sh; Visual Studio via an External Tool (on-demand, clickable Output) or an MSBuild Exec target (findings in the Error List, no analyzer/ VSIX); and the GitHub Action for CI. Includes a bash-free PowerShell form for Windows, the error-vs-warning severity note (sed downgrade + the tracked --severity open question), and the heuristic/false-positive caveats. Links it from the extractor README. https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
#1 --severity {error,warning} (default error): threaded through the core renderer (Finding.render/_github/_msbuild + render_finding), the ownir CLI (--severity, validated; only valid for ownir), own-check.sh, and the composite Action's new 'severity' input. A presentation choice — the finding is still the core's verdict — so a build can show leaks as advisory warnings instead of failing. Resolves P-013 open question #1. #2 scripts/own-check.ps1: a PowerShell twin of own-check.sh so Windows/VS users without bash get the same one-command pipeline (same flags/output). Docs: how-to §2B now uses --severity warning (drops the sed hack) and §4 points at own-check.ps1; P-013 scope/open-questions updated. CI: new step asserts --severity warning emits warning-level (not error) MSBuild lines. Verified locally: ownir 28/28, ruff + mypy --strict clean, severity honored in github/msbuild/human, bad/misapplied severity returns 2. https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughA new DI001 captive-dependency checker is implemented with a service graph model and OwnIR integration, detecting when a singleton captures scoped dependencies directly or via transient chains. Concurrently, a ChangesDI001 captive-dependency checker
Severity flag end-to-end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
A new core analysis (ownlang/di.py): over a DI registration graph, a singleton
that reaches a scoped service — directly or through a transient — captures it
for the app lifetime (the classic 'cannot consume scoped from singleton' bug).
Deterministic and static-friendly; it is the registration-graph property, not
the acquire/release model, so it lives in its own analyzer that the OwnIR
bridge feeds an optional, additive 'services' fact block to. One checker,
several analyses — the frontend still only produces facts.
- ownlang/di.py: Service/CaptiveDependency model + find_captive_dependencies
(DFS: singleton->scoped and singleton->transient->scoped are captive;
singleton->singleton->scoped is the inner singleton's bug; cycles guarded).
- ownir.py: validate the optional 'services' array at load; emit DI001 findings
at each registration site after the lifetime findings.
- tests: di.facts.json fixture + unit (graph) and bridge checks (ownir 34/34);
an invalid lifetime fails loudly.
- docs: bridge docstring documents the services block; P-006 -> in progress;
ROADMAP marks DI001 core-built.
Built like P-001 started: core + bridge + hand-written facts first, all locally
tested. NEXT (deferred, CI-only like the rest of the extractor): the C#
registration-graph extractor (services.Add{Singleton,Scoped,Transient} +
constructor injection).
Verified: ownir 34/34, ruff + mypy --strict clean.
https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
![coderabbitai[bot]](https://cdn.github.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 `@ownlang/__main__.py`:
- Around line 233-249: The validation logic for ownir-only flags
(check/cfg/emit/report) currently only rejects non-default values, allowing
default values like --format human or --severity error to pass silently. Modify
the validation code that checks these flags to reject them by their presence in
the opts dictionary, regardless of what value was assigned to them. Instead of
checking if a flag's value differs from its default, explicitly check if the
flag exists in opts after parsing and raise an error if any ownir-only flag was
provided, ensuring the "ownir-only" flag contract is enforced for all
occurrences of these flags.
🪄 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: 0298f829-b31e-4939-81cc-a75c6e1b9c5a
📒 Files selected for processing (10)
.github/workflows/ci.ymlaction.ymldocs/howto-visual-studio.mddocs/proposals/P-013-distribution-surface.mdfrontend/roslyn/README.mdownlang/__main__.pyownlang/ownir.pyscripts/own-check.ps1scripts/own-check.shtests/test_ownir.py
--format/--severity are ownir-only, but the guard only blocked non-default values, so `check x --format human` passed silently. Track whether any value-flag was supplied (seen_value_flags) and reject it on non-ownir commands regardless of value, closing the contract leak. Verified: ownir 34/34, ruff + mypy --strict clean; `check --format human` and `--severity=error` now return 2, ownir still accepts both. https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_ownir.py (1)
285-287: ⚡ Quick winAdd DI schema regression cases for
nameandlinevalidation.The DI negative-path tests currently guard
lifetimeonly. Add cases for empty/missingnameand non-integerlineso the loader contract is pinned and the malformed-input crash path doesn’t regress.Suggested test additions
@@ if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], "services": [{"name": "X", "lifetime": "perpetual"}]}): fails.append("an invalid service lifetime did not raise OwnIRError") + checks += 1 + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"lifetime": "singleton"}]}): + fails.append("a missing/empty service name did not raise OwnIRError") + checks += 1 + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"name": "X", "lifetime": "singleton", + "line": "NaN"}]}): + fails.append("a non-integer service line did not raise OwnIRError")🤖 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 285 - 287, Add additional regression test cases in the test file following the same pattern as the existing lifetime validation test around line 285-287. Create similar negative-path tests using the _load_raises function to verify that the loader properly validates and raises OwnIRError when services have an empty or missing name field, and when services have a non-integer line field. Each test case should append an appropriate descriptive failure message to the fails list if the expected exception is not raised, mirroring the existing "an invalid service lifetime did not raise OwnIRError" pattern.
🤖 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 `@ownlang/ownir.py`:
- Around line 230-240: In the service validation block (around line 236 in the
diff), tighten the validation for the service name to ensure it is not only a
string but also non-empty, since the current check allows empty strings to pass;
reject any service with an empty or missing name by adding an additional check
after the isinstance check. Additionally, in the _di_findings() function around
line 389, wrap the raw int() cast for the line parameter in a try-except block
to catch ValueError exceptions and re-raise them as OwnIRError with a clear
error message, ensuring all input validation errors are consistently raised as
OwnIRError rather than allowing built-in exceptions to propagate.
---
Nitpick comments:
In `@tests/test_ownir.py`:
- Around line 285-287: Add additional regression test cases in the test file
following the same pattern as the existing lifetime validation test around line
285-287. Create similar negative-path tests using the _load_raises function to
verify that the loader properly validates and raises OwnIRError when services
have an empty or missing name field, and when services have a non-integer line
field. Each test case should append an appropriate descriptive failure message
to the fails list if the expected exception is not raised, mirroring the
existing "an invalid service lifetime did not raise OwnIRError" pattern.
🪄 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: d757fcac-8f01-4ee6-add9-90885adee8e2
📒 Files selected for processing (6)
docs/ROADMAP.mddocs/proposals/P-006-di-lifetimes.mdownlang/di.pyownlang/ownir.pytests/fixtures/ownir/di.facts.jsontests/test_ownir.py
✅ Files skipped from review due to trivial changes (3)
- docs/proposals/P-006-di-lifetimes.md
- tests/fixtures/ownir/di.facts.json
- docs/ROADMAP.md
Comment on lines
+230
to
+240
| for s in svcs: | ||
| lt = s.get("lifetime") | ||
| if lt not in DI_LIFETIMES: | ||
| raise OwnIRError( | ||
| f"service 'lifetime' must be one of {sorted(DI_LIFETIMES)}, " | ||
| f"got {lt!r}") | ||
| if not isinstance(s.get("name", ""), str): | ||
| raise OwnIRError("service 'name' must be a string") | ||
| deps = s.get("deps", []) | ||
| if not isinstance(deps, list) or not all(isinstance(d, str) for d in deps): | ||
| raise OwnIRError("service 'deps' must be an array of strings") |
There was a problem hiding this comment.
Tighten DI services shape validation to avoid malformed-input crashes and ambiguous nodes.
load() currently accepts missing/empty name (Line 236 path), and _di_findings() does a raw int(...) cast for line (Line 389), which can raise non-OwnIRError exceptions on bad external input. This breaks the module’s “clear input error” contract and can also admit invalid anonymous service nodes.
Suggested fix
@@
- for s in svcs:
+ seen_names: set[str] = set()
+ for s in svcs:
lt = s.get("lifetime")
if lt not in DI_LIFETIMES:
raise OwnIRError(
f"service 'lifetime' must be one of {sorted(DI_LIFETIMES)}, "
f"got {lt!r}")
- if not isinstance(s.get("name", ""), str):
- raise OwnIRError("service 'name' must be a string")
+ name = s.get("name")
+ if not isinstance(name, str) or not name:
+ raise OwnIRError("service 'name' must be a non-empty string")
+ if name in seen_names:
+ raise OwnIRError(f"duplicate service name: {name!r}")
+ seen_names.add(name)
deps = s.get("deps", [])
if not isinstance(deps, list) or not all(isinstance(d, str) for d in deps):
raise OwnIRError("service 'deps' must be an array of strings")
+ fpath = s.get("file", "?")
+ if not isinstance(fpath, str):
+ raise OwnIRError("service 'file' must be a string")
+ ln = s.get("line", 0)
+ if not isinstance(ln, int) or isinstance(ln, bool):
+ raise OwnIRError("service 'line' must be an integer")Also applies to: 383-390
🤖 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 230 - 240, In the service validation block
(around line 236 in the diff), tighten the validation for the service name to
ensure it is not only a string but also non-empty, since the current check
allows empty strings to pass; reject any service with an empty or missing name
by adding an additional check after the isinstance check. Additionally, in the
_di_findings() function around line 389, wrap the raw int() cast for the line
parameter in a try-except block to catch ValueError exceptions and re-raise them
as OwnIRError with a clear error message, ensuring all input validation errors
are consistently raised as OwnIRError rather than allowing built-in exceptions
to propagate.
- load(): require a non-empty string 'name', and validate 'file' is a string and 'line' an int (not bool) — malformed DI facts now fail with a clear OwnIRError instead of being admitted as anonymous nodes; - _di_findings(): coerce 'line' via a non-throwing _as_int (check_facts can be called directly on un-validated facts), so a bad line degrades to 0 rather than raising a bare ValueError; - tests: add negative cases for missing/empty name and non-integer line (ownir 36/36). Deliberately did NOT reject duplicate service names — registering one interface multiple times is legal in .NET DI, so that would be a false error. Verified: ownir 36/36, ruff + mypy --strict clean. https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
This was referenced Jun 20, 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.
Summary by CodeRabbit
Release Notes
New Features
--severityto run findings in error or warning (advisory) mode, across the leak-check pipeline and OwnIR CLI.own-check.ps1) for running the leak check.Documentation
Tests
severity="warning"rendering coverage across formats.