Add Plan.md — Own.NET Audit orchestrator plan#99
Conversation
Plan for a decoupled audit orchestrator that runs a fleet of mature, ready-made tools (Roslyn analyzer packs, CodeQL, Infer#, own-check, runtime profilers) over the legacy .NET Framework 4.7.2 / WPF / DevExpress project and produces a categorized "health anamnesis" report ranked by where it hurts most. Key design choices, grounded in the existing repo culture: - Orchestrator, not a new analyzer — no regex detectors; categories with no reliable tool are marked NO-TOOL/skipped, never faked (honest-skip / minimal-FP, mirroring OWN050 and own-check --stats). - SARIF as the single normalized format; reuse parse_sarif and generalize oracle_compare's agreement buckets into a cross-tool confidence score. - Three layers: static fleet (build-free vs build-required tiers) → runtime (FlaUI + PerfView/procdump/ClrMD + SematixTrace, leak harness, duplicate-immutable detector, PropertyChanged-storm profiler) → AI reviewer strictly downstream and falsifiable. - Lives in an isolated audit/ subtree with zero coupling to ownlang/, so it lifts out as a standalone project; own-check is consumed only via its CLI. - Feeds confirmed findings back into corpus/ to train the analyzer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDPpNT9Uh8RoTrKPvgcoRE
Resolve the four open decisions per owner's answers: - Build env: no Windows CI with DevExpress — CI runs build-free tier (own-check + CodeQL) per commit; build-required tier (Roslyn packs, Infer#) is a manual run on a Windows dev machine whose SARIF is fed into aggregation. - Orchestrator language: Python for static/aggregation (reuse parse_sarif and oracle logic); C# only where forced (FlaUI, ClrMD). - Tooling: free stack only — no NDepend (architecture ranking degrades to Roslynator/Sonar/CodeQL metrics) and no dotMemory (duplicate detection and retention paths computed over ClrMD from a procdump full dump). - DevExpress noise: baseline-suppress the DevExpress.* namespace at normalization, with a suppressed-count surfaced in the coverage section. Section 11 turns from open questions into confirmed decisions; affected sections (1, 2, 3.2-3.4, 4, 7, 8, 9) updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDPpNT9Uh8RoTrKPvgcoRE
…tep-1 deliverable Two clarifications from the owner folded in: - Environment: the entire legacy audit (build-free, build-required, and runtime) runs on a single local Windows machine. There is no automated CI run of the target; Own.NET's Linux CI only gates the Python aggregation selftests. The build-free vs build-required tiers are now framed as a robustness distinction (cayrcass even on an unbuildable solution), not a CI-vs-local split. - Scope: the done-criterion for the first (static) step is the legacy health report only. Feeding findings back into the OwnLang corpus/oracle (section 6) is explicitly a later, separate track (phase 4), out of the first deliverable. Updated sections 1, 3.2, 6, 8, 9, 11 accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDPpNT9Uh8RoTrKPvgcoRE
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4edd2116d
ℹ️ 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".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesOwn.NET Audit RFC
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Plan.md (1)
122-127: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake worktree lifecycle failure-safe in the documented flow.
If build fails mid-run,
git worktree removemay be skipped; documenttry/finally(or shelltrap) cleanup so temporary trees don’t accumulate.🤖 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 `@Plan.md` around lines 122 - 127, The documented worktree flow is not failure-safe because a failed build can skip cleanup and leave temporary worktrees behind. Update the instructions around git worktree add and git worktree remove to explicitly use a try/finally-style cleanup or a shell trap so the temporary target-audit tree is always removed, and reference the documented build sequence in Plan.md so readers can apply the fix consistently.
🤖 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 `@Plan.md`:
- Line 387: The planned tree still includes ndepend.ps1, which conflicts with
the accepted no-NDepend direction. Update the plan entry that lists
roslyn_pack.ps1, codeql.sh, infersharp.sh, owncheck.py, and ndepend.ps1 by
removing ndepend.ps1 entirely or marking it clearly as deprecated so the plan
aligns with the free-stack-only decision.
- Line 507: The Plan.md content has a dangling fenced code block marker, leaving
the Markdown invalid. Remove the standalone triple-backtick at the end of the
section and ensure the fenced block is properly opened and closed around the
intended content so the Markdown renders and lints cleanly.
- Line 52: Several fenced code blocks in the plan docs are missing language
identifiers, which trips MD040. Update each affected fence in the plan document
to include the appropriate language tag (for example text, bash, or tree) based
on the block content, and make sure the same fix is applied to all reported
occurrences so the markdown stays lint-clean and unambiguous.
---
Nitpick comments:
In `@Plan.md`:
- Around line 122-127: The documented worktree flow is not failure-safe because
a failed build can skip cleanup and leave temporary worktrees behind. Update the
instructions around git worktree add and git worktree remove to explicitly use a
try/finally-style cleanup or a shell trap so the temporary target-audit tree is
always removed, and reference the documented build sequence in Plan.md so
readers can apply the fix consistently.
🪄 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: 19c39715-0703-44c2-b0a0-ee49eaf49335
📒 Files selected for processing (1)
Plan.md
…pend leftover, markdown lint Resolves the unresolved review threads on Plan.md: - §3.1 worktree flow: copy the injected props/targets under MSBuild's recognized names (Directory.Build.props/.targets) instead of OwnAudit.Directory.Build.* — MSBuild only auto-imports the canonical names, so the analyzer injection would have been silently skipped. Added a trap-based worktree cleanup so a failed build can't leave temporary trees behind, and a note on chaining a pre-existing Directory.Build.props. - categories.yml taxonomy: OWN001 is an umbrella leak code (IDisposable field/local, subscription token, timer), distinguished by its [resource: ...] tag. Split it by resource kind so subscription/timer leaks land in categories 2/3 instead of being collapsed into IDisposable (category 1) and undercounted. Relabelled OWN014 as region-escape (vm->App / SystemEvents), which is what it actually emits — not generic subscription-leak. - Removed the leftover ndepend.ps1 from the §7 tree; it contradicts the accepted free-stack-only / no-NDepend decision. - Markdown lint: added language tags to fenced blocks (MD040) and removed a dangling closing fence at end of file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDPpNT9Uh8RoTrKPvgcoRE
|
Адресовал все ревью-треды в
Всё это правки консистентности самого плана, поведение не меняется. Документ остаётся внутренне согласованным с §8/§11. Generated by Claude Code |
Plan for a decoupled audit orchestrator that runs a fleet of mature,
ready-made tools (Roslyn analyzer packs, CodeQL, Infer#, own-check,
runtime profilers) over the legacy .NET Framework 4.7.2 / WPF / DevExpress
project and produces a categorized "health anamnesis" report ranked by
where it hurts most.
Key design choices, grounded in the existing repo culture:
no reliable tool are marked NO-TOOL/skipped, never faked (honest-skip /
minimal-FP, mirroring OWN050 and own-check --stats).
oracle_compare's agreement buckets into a cross-tool confidence score.
runtime (FlaUI + PerfView/procdump/ClrMD + SematixTrace, leak harness,
duplicate-immutable detector, PropertyChanged-storm profiler) →
AI reviewer strictly downstream and falsifiable.
lifts out as a standalone project; own-check is consumed only via its CLI.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01QDPpNT9Uh8RoTrKPvgcoRE
Summary by CodeRabbit