diff --git a/docs/proposals/P-026-csharp-strictness-retrofit.md b/docs/proposals/P-026-csharp-strictness-retrofit.md new file mode 100644 index 00000000..ff55c1ee --- /dev/null +++ b/docs/proposals/P-026-csharp-strictness-retrofit.md @@ -0,0 +1,194 @@ +# P-026 — C# strictness retrofit profile (`own audit strictness`) + +- **Status:** draft — a *framing/packaging* direction, not a new engine. + Sourced from an external strategic read (a developer's "The Missing + Programming Language" essay + a follow-up analysis of how Own.NET could speak + to that audience). Implementation not started. +- **Depends on:** the audit orchestrator ([`Plan.md`](../../Plan.md), + [`audit/README.md`](../../audit/README.md)) — SARIF normalization, cross-tool + agreement scoring, the health report; the existing lifetime/resource core + (`OWN001–015`), the typestate / obligation work + ([P-010](P-010-type-disciplines.md), [P-025](P-025-obligation-protocols.md)), + and the C# fact seam ([P-001](P-001-csharp-extractor.md), + [P-014](P-014-semantic-resolution.md)). Relates to + [P-015](P-015-configuration-surface.md) (check selection / severity) and + [P-023](P-023-architecture-guard.md) (drift). +- **Where it lives:** an **audit profile + a report renderer**, consumed through + CLI + SARIF only, zero coupling to `ownlang/`. It follows the audit code + (currently `OwnAudit`, per `audit/README.md`), like [P-024](P-024-security-audit-profile.md). + +## Decision (read this first) + +**`own audit strictness` is a witness, not an engine.** It is a profile and a +report that *front the findings the fleet already produces* under one narrative — +"where C# lets your types lie, your domain rules live in comments, and your +resources leak because the language cannot express ownership" — plus a single +headline number (a **strictness score**). It adds **no new detector heuristic**; +every number it prints traces to a real finding from a real tool, or is labelled +`NO-TOOL`. This is the same charter as the rest of the audit fleet +("оркестратор, не анализатор"; "берём готовое"; honest coverage). + +The motivating analysis proposed five angles. Two are **accepted as framing**, +two are **rejected as scope**, one is **already our identity**: + +| # | Angle from the analysis | Verdict | +|---|-------------------------|---------| +| 1 | Strictness audit — "make C# stop pretending to be grown-up" | **Accept as framing** — this profile *is* that packaging over existing findings | +| 2 | Domain-invariant miner (implicit state machines hidden in flags/dates) | **Already in flight** — [P-010](P-010-type-disciplines.md) typestate + [P-025](P-025-obligation-protocols.md); this profile *surfaces* their output, it does not re-implement | +| 3 | Result/Option migration assistant (`return null` → `Option`, `throw` → `Result`) | **Reject as core scope** — see Non-goals | +| 4 | Exhaustiveness guard over OneOf/LanguageExt/… | **Defer to P-010** — internal exhaustiveness is P-010's territory; not a new pack here | +| 5 | Ownership / lifetime as the unique card | **Already the identity** (ROADMAP: "lifetime/resource bugs C# cannot express") — this profile leads with it, does not dilute it | + +The load-bearing decision is the **rejection of #3/#4 as their own product.** +"Find `return null`, suggest `Option`; find `throw`, suggest `Result`" is, +in the analysis's own words, something "any caffeinated student with Roslyn can +write." It is a nullable-annotation / control-flow linter — exactly the broad +"we find smells" arena where CodeQL/Sonar/Semgrep win on sales budget, not +merit, and which the ROADMAP explicitly refuses. We take the *vocabulary* +("the domain model lies", "make invalid states visible → then unrepresentable") +without becoming that linter. Ownership/lifetime stays the moat. + +## Motivation — meeting an audience that already articulated the pain + +The essay ranks C# at 4.5/5: big ecosystem, good perf, good DevX, but +"expressive types: mixed" — nullable is opt-in and warning-only, `!` launders +nulls, unions are perennially "coming", no local immutability. The reader wants +**C# ecosystem + F#/Rust-like safety** and is not going to switch languages to +get it. That is precisely Own.NET's long-term identity restated by an outsider: + +> an external static-contract layer for C#/.NET that adds ownership, typestate, +> effects, capabilities, and domain-specific types **without rewriting the +> codebase.** — `docs/ROADMAP.md` + +So the value here is **not new capability** — it is a *doorway* sized for this +reader. Today the capabilities are addressed by their mechanism (`OWN008`, +`DI003`, `OBL002`, "captive dependency"), which lands with a lifetime nerd, not +with someone whose complaint is "C# lets my types lie". `own audit strictness` +re-presents the same evidence as *"here are the places C# let your model lie"*, +with one score to argue about. + +## Scope + +- A CLI surface `own audit strictness ` that runs the relevant slice of + the existing fleet and renders a **strictness report** (markdown + json), + reusing the audit orchestrator's normalize/score/report pipeline. +- A **strictness score** derived *only* from findings that already exist, + bucketed into named strictness dimensions (below), each dimension carrying an + honest coverage state (`covered` / `NO-TOOL` / `partial`). +- A markdown narrative that groups findings as *invariant lies*, not as tool + codes — e.g. an implicit state machine surfaced by P-010/P-025 is rendered as + "`Order.Status` + `PaidAt` + `CancelledAt` form an implicit state machine; + these constraints are not encoded in the type system", with the underlying + finding IDs as evidence. + +### The dimensions (all fronting existing or proposed detectors) + +| Strictness dimension | Backed by | +|----------------------|-----------| +| Ownership / resource lifetime not expressed | `OWN001–015`, WPF/`IDisposable`/DI/pool profiles ([P-004](P-004-wpf-lifetime-profile.md)–[P-007](P-007-arraypool-span.md)) | +| Implicit state machines / typestate in flags & dates | [P-010](P-010-type-disciplines.md), [P-025](P-025-obligation-protocols.md) | +| Non-exhaustive handling hiding future cases | [P-010](P-010-type-disciplines.md) | +| Source-of-truth drift (config vs DI, XAML vs VM, EF vs migrations) | [P-015](P-015-configuration-surface.md), [P-023](P-023-architecture-guard.md) | +| Null-safety not enforced (`!`, opt-in NRT) | external analyzers via SARIF adapter; `NO-TOOL` until wired | +| Local immutability not expressed (mutable setters on domain state) | `NO-TOOL` until wired (candidate: setter census via the P-001/P-014 extractor) | + +**One bucket per finding.** Every finding counts against exactly one *primary* +dimension, chosen by the table order above (first matching row wins — e.g. an +ambiguous `IDisposable` transfer on a field that is also an implicit-state flag +scores under ownership/lifetime, not typestate). Reclassifying a finding moves +it between buckets without changing its total contribution, so the headline +score never double-counts one defect across dimensions and stays stable under +re-bucketing. + +## Non-goals (the most important section) + +- **No new heuristic detector.** No regex over `return null` / `throw`. If a + dimension has no reliable tool, it is `NO-TOOL`, not faked. (Same rule that + killed the security scanner engine in [P-024](P-024-security-audit-profile.md).) +- **Not an Option/Result migration linter.** We do not ship "rewrite `Customer?` + to `Option`" as a product. Suggesting union/`Result` remodels *as + prose in the report* is fine (it is the narrative); shipping an autofix/analyzer + pack for it is the SAST fight we refuse. +- **Not a generic nullable-annotation nag.** Roslyn's own analyzers + Sonar + already own "you used `!`". We surface null-safety posture only as a *dimension + of the score*, via existing tools, not as our own rule. +- **The score is not a vanity metric.** Every point of the 0–100 must be + reconstructable from listed findings + coverage. A dimension that did not run + lowers *confidence/coverage*, it does not silently read as "clean" (the audit + charter's honest-coverage rule). +- **No new language, no autofix arm here.** This is a lens over the audit; the + fix-arm lives where the audit's `fix/` layer lives. + +## The honest gap (do not paper over it) + +The analysis's demo — `own audit strictness MySolution.sln → Strictness score: +61/100` over an *arbitrary* solution — assumes a general C# semantic frontend. +That is not shipped: real-C# ingestion is the Roslyn extractor +([P-001](P-001-csharp-extractor.md)) + semantic resolution +([P-014](P-014-semantic-resolution.md)), both "in progress", scoped to the +lifetime/leak profiles, not to whole-solution strictness. So the first cut is +either: + +- **(a)** run over the existing audit target (the legacy WPF app the fleet + already ingests) and score *that*, or +- **(b)** ship the score frame with most dimensions honestly `NO-TOOL` and fill + them as extractor coverage lands. + +Both are charter-honest. What we must **not** do is print a green `61/100` whose +missing dimensions were silently treated as passing — that is the exact +dishonesty `audit/README.md` forbids. + +## Sketch + +```text +own audit strictness [--profile wpf|generic] [--baseline ] + → run fleet slice → SARIF (existing adapters) + → normalize → categorized findings (audit/aggregate/normalize.py) + → map to strictness dimensions + coverage state (new: thin mapping table) + → score → 0..100 + per-dimension breakdown, confidence from coverage + → render → strictness.md + strictness.json (new renderer over report.py) +``` + +Report head (illustrative): + +```markdown +# C# Strictness Audit — @ + +Strictness score: 61/100 (coverage: 4/6 dimensions; 2 NO-TOOL) + +## Where the type system is not carrying the invariant +1. 9 implicit state machines (Status + *At flags) [P-010/P-025 evidence] +2. 8 ambiguous IDisposable ownership transfers [OWN / D-series] +3. 31 mutable setters on domain records [NO-TOOL: partial] +... +``` + +Score = weighted sum over dimensions; each dimension's contribution is a function +of its findings and severity; `NO-TOOL` dimensions do not contribute points but +*cap displayed confidence*. Deterministic over a fixed commit (diffable, like +every audit run), so `--baseline` yields new/old/suppressed exactly as the +orchestrator already does. + +## Open questions + +- **Score model.** Absolute (findings → deductions) is honest but noisy across + codebases; a percentile against a corpus is friendlier but needs a corpus and + can lie about small repos. Start absolute + per-dimension, defer any headline + cross-repo comparison. +- **First target.** (a) the audit's WPF target (real, narrow) vs (b) frame-first + with `NO-TOOL` dimensions. Leaning (a): it produces a *real* score to show, + not a mostly-empty frame. +- **Where the mapping table lives.** It is audit-side config (dimension ← rule + ids), so it should sit with the orchestrator's category map, not in `ownlang/`. +- **Naming.** `own audit strictness` vs `own strictness`; whether "strictness" + reads as scolding — the report's job is to read as *diagnosis of hidden + invariants*, not style-nagging. + +## What we deliberately keep from the analysis, in one line + +The *framing* ("the domain model lies; the lifetime is unexpressed; the error is +hidden in an exception; the invariant survives on the honor system") and the +*doorway* (one score, one narrative for the F#/Rust-refugee reader) — laid over +the evidence the fleet already produces. Not a new analyzer; a lens that makes +the existing verdicts legible to someone who wants an S-tier language and hasn't +noticed Own.NET is quietly building the missing layer for the one they already use. diff --git a/docs/proposals/P-027-resource-state-machine.md b/docs/proposals/P-027-resource-state-machine.md new file mode 100644 index 00000000..000bec3e --- /dev/null +++ b/docs/proposals/P-027-resource-state-machine.md @@ -0,0 +1,269 @@ +# P-027 — Resource state machines & stale-async-write detection (extends `Own.Async`) + +- **Status:** draft — proposal for discussion, not a committed design. Recorded on + the record so the idea isn't lost, not "we are building this next." +- **Depends on:** [P-021](P-021-async-audit-pack.md) (`Own.Async` — the sibling + catalog this slots into), [P-020](P-020-ownts-react-effects.md) (`Own.React` — + the OwnTS mirror of the same lifecycle shape), [P-010](P-010-type-disciplines.md) + (typestate/protocols — the heavier-weight cousin this deliberately does not + become), [P-004](P-004-wpf-lifetime-profile.md)/[P-005](P-005-idisposable-ownership.md) + (subscription/`IDisposable` cleanup — already-covered ground this reuses rather + than reinvents), [P-025](P-025-obligation-protocols.md) (obligation protocols — + a plausible home for the in-flight-guard check). + +## Origin and honest framing + +This proposal started from a critique of a React `useEffect` blog post. The post's +headline advice — "your code should always work without the dependency array" — +is wrong on its own terms: the dependency list is part of `useEffect`'s +synchronization contract with React (it's exactly what `react-hooks/exhaustive-deps` +checks), not an optional optimization to delete. But the critique's *real* kernel +is right, and it has nothing to do with React specifically: + +> A resource's lifecycle (empty / loading / available / failed) should be modeled +> as one explicit state, not reconstructed at read time from a pile of booleans +> and a nullable field that also means "not loaded yet." + +That kernel generalizes cleanly to any .NET code that loads a resource +asynchronously and drives state off ad hoc flags — WPF ViewModels, Presenters, +Blazor components, service classes: + +```csharp +private bool _isLoading; +private bool _isLoaded; +private Customer _customer; // null: not loaded? loading? load failed? + +public async Task LoadAsync() +{ + if (!_isLoaded && !_isLoading) + { + _isLoading = true; + _customer = await _repository.GetCustomerAsync(_customerId); + _isLoaded = true; + _isLoading = false; + } +} +``` + +This is the same animal P-020 already names for the OwnTS/React side (effect +re-entry, missing cleanup) and P-021 already partly covers for .NET async +(blocking waits, `async void`, task escaping a `using`/`finally`). What neither +proposal covers yet is the *state-shape* problem itself, and its most common +correctness consequence: a **stale write** when the resource identity changes +while the `await` is in flight. + +**What this proposal is not**: a rewrite of the source critique's tone or its +"always fail without the second `useEffect` argument" claim. Both are wrong and +out of scope. The only thing worth taking from that text is the two diagnostic +ideas below — everything else in it (dependency-array rhetoric, a proposed +runtime `ResourceState` helper as gospel) is either already someone else's job +in this project or not this project's job at all. + +## What's already covered elsewhere (do not reinvent) + +Following the "honest split" discipline P-020 already established, most of the +patterns in the source material map onto existing Own.NET ground: + +| Pattern in the source critique | Already covered by | +|---|---| +| Missing `-=`/`Dispose()`/`Unsubscribe()` for an event/timer/subscription | `OWN001` (P-004/P-005), and its OwnTS mirror `EFF003`–`EFF005` (P-020) | +| `Task` escaping a `using`/`try`-`finally` scope | `ASYNC001`/`ASYNC002` (P-021) | +| Blocking `.Result`/`.Wait()` on the UI thread | `ASYNC010` (P-021) | +| `async void` outside an event handler | `ASYNC020` (P-021) | +| Fire-and-forget without observation | `ASYNC030` (P-021) | +| Proving valid state *transitions* with consume-self semantics (a `Connection`-style protocol) | typestate/`protocol` blocks (P-010) — deliberately heavier machinery than this proposal wants | + +This proposal only adds the **two genuinely new** dimensions: state-shape +normalization, and stale-write protection. If it turns out either is just a +restatement of something above, that's a reason to fold it in, not to ship a +duplicate checker — the project's standing rule is one core, no parallel +verdict engines. + +## Scope — two new diagnostics + +### 1. Boolean/nullable state soup + +A resource's lifecycle is a small state space (`Empty`, `Loading`, `Available`, +`Failed` — sometimes `Cancelled`/`Refreshing`). Representing it as N independent +boolean or nullable fields lets the compiler accept states that shouldn't exist +(`IsLoading == true && IsLoaded == true`), and pushes every reader ("has this +loaded yet?") into reconstructing the state from field combinations instead of +reading one value. + +Detect: a cluster of ≥3 boolean/nullable-reference fields that are all assigned +inside the same async-loading method (heuristically: a method that both sets one +of the fields and `await`s something), read together in `if`/`&&` conditions +elsewhere in the same type. + +```csharp +// flagged: 4 fields modeling one lifecycle +private bool _isLoading; +private bool _isLoaded; +private Customer _customer; // doubles as "has value" via null +private Exception _loadError; +``` + +```text +ASYNC050: fields 'IsLoading', 'IsLoaded', 'Customer' (null-as-status), 'LoadError' +appear to model a single resource lifecycle as independent flags. Combinations +like IsLoading == true && IsLoaded == true are representable but meaningless. +Consider one explicit status (e.g. an enum: Empty/Loading/Available/Failed). +``` + +Suggested replacement is explanation-only in v0 (see Non-goals) — a comment +pointing at the shape, not an autofix: + +```csharp +private enum LoadStatus { Empty, Loading, Available, Failed } +private LoadStatus _status; +private Customer _customer; +private Exception _loadError; +``` + +### 2. Stale async write / missing in-flight guard + +The sharper bug: a value read *before* an `await` gates what gets written +*after* it, but nothing proves the world hasn't moved on while the method was +suspended. + +```csharp +public async Task LoadCustomerAsync() +{ + var customer = await _repository.GetCustomerAsync(_customerId); // _customerId read before await + CurrentCustomer = customer; // written after await +} +``` + +If `_customerId` changes (user picks a different row) while the first call is +still in flight, the stale response can overwrite the fresher one — or a second +concurrent call starts because nothing checked "am I already loading?" first. +This is the same failure class as the Cloudflare-dashboard effect-storm P-020 +already cites, just on the read/write-race axis instead of the re-trigger axis. + +Detect two sub-shapes: + +- **No in-flight guard (`ASYNC051`)**: an async loading method with no + early-return/guard against re-entry while a status field is already + `Loading`, and no synchronization (`SemaphoreSlim`, `lock` around a flag, + `CompareExchange`) preventing two concurrent calls from both proceeding. +- **No staleness check (`ASYNC052`)**: a field read before an `await` is used + (directly, or via a captured local) to decide what a field write after the + `await` commits, with no `CancellationToken`, no version/request-id + comparison, and no re-read/compare of the gating field after the `await`. + +```text +ASYNC051: 'LoadCustomerAsync' starts an async operation with no guard against +re-entry. If called again before the first call completes, both calls will +proceed concurrently. Guard on a status field (e.g. 'return if Status != Empty') +or a synchronization primitive before starting the operation. + +ASYNC052: 'LoadCustomerAsync' reads '_customerId' before the await and writes +'CurrentCustomer' after it, with no CancellationToken, version check, or +re-read of '_customerId' in between. If '_customerId' changes while this call +is in flight, a stale response can overwrite fresher state. +``` + +Both codes are heuristic/evidence-tiered, in the same spirit as P-021's +detectability matrix — high confidence when the guard/version-check is provably +absent syntactically, lower confidence (or silent) when a helper method or +`SemaphoreSlim` field exists nearby and the extractor can't yet prove it's used +correctly. + +## Non-goals + +- **Not a general data-race detector.** No happens-before model, no full + shared-mutable-state analysis. Scope stays "resource load method reads before + an `await`, writes after it, with no visible guard" — a syntactic pattern, not + a soundness proof. +- **No shipped runtime type.** Own.NET does not ship a `ResourceState` + NuGet package as the mandated fix, matching P-010's existing rule that + brands/refinements/protocols lower to plain structs the *team* owns. The + diagnostic explains the pattern; teams write their own status type (or adopt + whichever shape fits their codebase). +- **Not typestate.** Proving that `Loading → Available` is the *only* valid + transition, with consume-self semantics, is P-010's `protocol` block. This + proposal only flags "these fields look like an unmodeled lifecycle" — a lint, + not a proof. +- **No duplicate of `OWN001`/`ASYNC001`/`ASYNC002`/`ASYNC010`/`ASYNC020`/`ASYNC030`.** + Cleanup, blocking waits, `async void`, task-escape, and fire-and-forget stay + exactly where P-004/P-005/P-020/P-021 already put them. +- **No claim of catching every stale-write race.** Silent by default whenever a + synchronization primitive or cancellation token is present, even if its use + can't yet be proven correct — false negatives over false positives, same + posture as the rest of the async pack. + +## Sketch + +Same seam as the rest of the project — the Roslyn extractor emits facts, the +Python core emits verdicts: + +```text +*.cs --[Roslyn extractor]--> facts.ownir.json --[Python core]--> ASYNC050..052 +``` + +`ASYNC051`/`ASYNC052` are per-method and most likely land as two more hazard +kinds in the `async_methods` fact family P-021 plans (its proposed +`ownlang/async_rules.py` module). `ASYNC050` is type-scoped — a cluster of fields +assigned together across the whole class, not one method — so it needs its own +type-level fact rather than living inside a method entry: + +```json +{ + "types": [ + { + "id": "CustomerViewModel", + "status_field_clusters": [["_isLoading", "_isLoaded", "_loadError"]] + } + ], + "async_methods": [ + { + "id": "CustomerViewModel.LoadCustomerAsync", + "await_count": 1, + "reads_before_await": ["_customerId"], + "writes_after_await": ["CurrentCustomer"], + "has_cancellation_token": false, + "has_version_check": false, + "guarded_fields": [], + "hazards": [ + { "kind": "no_inflight_guard", "line": 40 }, + { "kind": "stale_write_no_guard", "line": 41, "read": "_customerId", "written": "CurrentCustomer" } + ] + } + ] +} +``` + +`types[].status_field_clusters` feeds `ASYNC050` independently of the +per-method `hazards`, keeping the type-level and method-level extractor +contracts separate. + +## Open questions + +1. **Home for the codes.** Extend P-021's `ASYNC0xx` family (leaning: yes — same + audience, same fact shape, same team owns both), or a separate prefix so + "state soup" doesn't read as an async-specific bug even when the loading path + happens to be synchronous? Leaning toward `ASYNC05x` since the motivating + cases are overwhelmingly async load methods. +2. **Threshold for state soup.** Two co-varying fields (`HasValue`/`Value`) are + completely ordinary and shouldn't fire. Where's the real line — 3 fields? A + specific combination like `bool + bool + nullable`? This needs corpus + evidence (P-012) rather than a guessed constant, in the same evidence-first + spirit as P-021. +3. **Staleness-check feasibility.** Proving "the gating field was re-read or a + version was compared between the pre-await read and the post-await write" + needs more than single-method syntax — it's adjacent to P-016's deep fact + extraction (CFG across the `await` boundary). Does `ASYNC052` start as a + narrower syntactic pattern (a specific field/property re-read immediately + before the write) and grow into real flow evidence later, same staging P-016 + already uses elsewhere? +4. **Does the in-flight guard belong to P-025 instead?** "Must not start a new + load while `Status == Loading`" is exactly a barrier-sensitive temporal + obligation — P-025's `OBL` machinery already models "call A before call B, + guarded by a barrier." `ASYNC051` may be P-025's first non-toy consumer + rather than a bespoke check; worth prototyping against `OBL` before writing a + parallel guard-detector. +5. **Suggested-fix surface.** Stay explanation-only (a diagnostic message + pointing at the shape) in v0, or eventually offer a code-fix snippet for the + `enum`-based replacement? Leaning: explanation-only until the pattern proves + out on real code, matching P-021's own "no mass autofix" stance for + `ASYNC040`. diff --git a/docs/proposals/P-028-unneeded-dependency-profile.md b/docs/proposals/P-028-unneeded-dependency-profile.md new file mode 100644 index 00000000..4ebf3167 --- /dev/null +++ b/docs/proposals/P-028-unneeded-dependency-profile.md @@ -0,0 +1,155 @@ +# P-028 — Unneeded-dependency profile (`Own.Lean`) + +- **Status:** draft — not started. +- **Depends on:** [P-001](P-001-csharp-extractor.md) (the Roslyn extractor + seam), [P-006](P-006-di-lifetimes.md) (the DI `services[]` registration + graph — YDN002 extends its facts with the closed generic arguments the + existing graph collapses away, see Sketch), [P-015](P-015-configuration-surface.md) + (severity/opt-in surface for the phase-2 heuristics). Bounded explicitly + against [P-021](P-021-async-audit-pack.md) (`ASYNC040` already owns the + "trivial async passthrough" case — not duplicated here) and + [P-023](P-023-architecture-guard.md) (Own.Arch gates *forbidden* structure; + this profile flags *provably redundant* structure — different verdict shape, + never a build gate). + +## Motivation + +The `you-dont-need/You-Dont-Need` meta-list (a curated collection of +"You Might Not Need Lodash/Moment/Redux/…" write-ups) makes one real point +under all the individual takes: teams often reach for a popular dependency +because it is popular, not because the problem in front of them needs it. The +honest version of that point is not "dependencies are bad" — it is that every +dependency has to clear a bar: + +```text +dependency_value > dependency_cost +``` + +where cost is never just install size — it is maintenance, transitive CVEs, +build complexity, onboarding, and the debugging friction of an indirection +layer nobody on the team wrote. .NET has its own instances of the same +pattern: an `AutoMapper` profile that copies five identically-named properties +and nothing else, a `MediatR` handler with exactly one implementation and no +pipeline behaviours standing in for a direct method call. The libraries are +not the problem — using them where they buy nothing is. + +The trap is that "you don't need X" is trivially easy to turn into an +opinionated hot-take generator (see the source list's own "You Might Not Need +TypeScript" entry) that flags a library's mere presence. That is exactly the +kind of noisy, ungrounded quality gate this project's other proposals +deliberately reject (see P-023's "no SOLID detector" stance). So the scope +here is narrower and stricter than the inspiration: + +> **Own.Lean never passes judgment on a library. It flags one call site at a +> time, only when the code at that site proves the abstraction added nothing +> — and the moment any real customization is visible, it stays silent.** + +## Scope + +### MVP — deterministic, evidence-only + +| Code | Finding | Evidence required | Suggestion | +|------|---------|--------------------|------------| +| `YDN001` | `AutoMapper` `CreateMap()` (or `Profile`-declared map) that is a pure 1:1 copy | every public writable member of `TDest` has an exact-name, assignable-type public readable counterpart on `TSrc`; **no** `.ForMember`/`.Ignore`/`.ConvertUsing`/custom value resolver/`.ReverseMap`; member count within a configurable bound (default 10) | replace with an explicit object initializer or a mapping constructor | +| `YDN002` | `MediatR` `IRequestHandler` resolved via `ISender`/`IMediator` with exactly one registered implementation and zero registered `IPipelineBehavior<,>` (open or closed) anywhere in the DI graph | needs the DI registration graph's generic arguments preserved per `IRequestHandler` registration (see Sketch — today's graph collapses these) | inject the handler directly instead of dispatching through the mediator | + +`YDN001` is structurally the same shape already used for `DI001` (P-006): read +a graph the extractor already builds, compare cardinalities and declared +customization, emit a verdict only when the customization set is empty. +`YDN002` needs the same shape but over a graph the extractor does not yet +build in the needed resolution — see Sketch. Neither rule inspects call-site +*style* — only the declared shape of the mapping/registration. + +### Phase 2 — heuristic, report-only, opt-in via P-015 + +| Code | Finding | Confidence | +|------|---------|------------| +| `YDN010` | A DI-registered service with exactly one registration across the whole solution, not exposed as a public extension point, and never re-registered in a test project | heuristic — a real single-impl service and a "this interface is pure ceremony" service look identical without knowing intent; ships as report-only or not at all | + +This tier stays report-only, never a build gate, and is the honest limit of +what this profile should attempt — see Non-goals for the parts of the "You +Don't Need" list that were deliberately left out rather than downgraded to +Phase 2. + +## Non-goals + +- **No library blocklist.** "Don't use Lodash/Axios/Moment" has no .NET + analogue that would be evidence rather than opinion, and even in spirit, + Own.Lean does not ship a list of disfavoured packages. Every finding names a + specific call site and the specific evidence at it. +- **No "replace the ORM with hand-written SQL" suggestion.** Whether a + hand-rolled query beats an ORM call depends on performance requirements this + tool cannot observe statically. Not evidence-based; not built. +- **No reflection → source-generator suggestion.** There is no oracle for + "this reflection could have been codegen'd" short of writing the generator — + guesswork, not a finding. +- **No JSON → MessagePack/binary-format suggestion.** A wire-format choice + depends on external constraints (interop, human-readability requirements) + invisible to static analysis. +- **No reimplementation of existing Roslyn/FxCop LINQ micro-optimizations** + (`.Where(p).Count()` → `.Count(p)` and siblings — already `CA1826`/`CA1827`/ + `CA1828`/`CA1829`). Own.NET's differentiator is checks nobody else runs, not + a third copy of a rule two analyzers already ship. +- **No build-blocking severity, ever, for this family.** Every `YDN###` is + info/warning and never wired into the P-023 architecture-guard ratchet. A + false positive here costs a reviewer one comment, not a red PR. +- **No bundle-size / transitive-CVE / dependency-count scoring.** That is a + supply-chain audit tool (NuGet advisory scanning, dependency-graph size), + a different project; if ever pursued it is its own proposal, not folded in + here. +- **No hostility to AutoMapper or MediatR as libraries.** Both are legitimate + the moment they are used for what they are for — custom resolvers, cross- + cutting pipeline behaviours, polymorphic dispatch over many handlers. + `YDN001`/`YDN002` are silent the instant any of that evidence appears. + +## Sketch + +```text +C# source --[Roslyn extractor]--> mapping-profile facts (YDN001) + \-> services[] graph, extended with closed generic args (YDN002) + | + [core: same Python seam] + | + YDN### verdicts --> SARIF + markdown +``` + +`YDN001` needs one new extractor fact family: for each `CreateMap` +call (or `Profile`-declared map), emit the two member lists plus whichever +customization calls (`.ForMember`, `.Ignore`, `.ConvertUsing`, `.ReverseMap`) +appear in the same fluent chain. + +`YDN002` is **not** a free ride on the existing P-006 `services[]` graph, and +the MVP scope above was wrong to claim otherwise (caught in review): the +extractor's `DiTypeName` helper +(`frontend/roslyn/OwnSharp.Extractor/Program.cs`) deliberately reduces a +generic registration to its rightmost identifier — `IRequestHandler` +and `IRequestHandler` both become the bare `IRequestHandler` — because +P-006's captive-lifetime checks never needed to distinguish closed generic +arguments. `YDN002` does need that distinction: counting "implementations of +*this* `TReq[,TResp]`" from an identifier-only graph would silently count +every unrelated handler in the solution as the same bucket the moment a +project has more than one MediatR request. The fix is a small, additive +extractor change — preserve the closed type-argument pair (and the +`IPipelineBehavior<,>` type arguments, open or closed) alongside the existing +service/impl identifiers when the generic is one of the MediatR marker +interfaces — not a reinterpretation of the current collapsed facts. + +## Open questions + +1. Where does the "still trivial" member-count bound for `YDN001` live — + hardcoded default, or a P-015 per-project knob? Leaning: a default with a + P-015 override, consistent with how severity is already configured + elsewhere. +2. Does `YDN002` also need to inspect the handler body for inline cross- + cutting code (logging/validation) that a pipeline behaviour would normally + own, or is DI-graph evidence (impl count + behaviour count) sufficient on + its own? Needs a trial against a real MediatR-using corpus sample. +3. Naming/positioning: a standalone `Own.Lean` family, or a phase-4 "ceremony" + tier under `Own.Arch` (P-023)? Leaning: standalone — P-023 gates *forbidden* + structure (a graph-edge violation); this profile flags *provably redundant* + structure (an indirection with zero customization). The verdict shapes + differ (a gate vs. a suggestion), which argues for keeping them separate + families sharing only the extractor seam. +4. Prefix: following the `ASYNC`/`ARCH`/`OBL` precedent of a family-specific + code rather than overloading `OWN###` — `YDN###` as proposed above, unless + a shorter/clearer prefix surfaces during naming review. diff --git a/docs/proposals/P-029-agent-memory-layer.md b/docs/proposals/P-029-agent-memory-layer.md new file mode 100644 index 00000000..ad8a7197 --- /dev/null +++ b/docs/proposals/P-029-agent-memory-layer.md @@ -0,0 +1,204 @@ +# P-029 — Agent memory & policy layer (`.agents/`) + +- **Status:** draft — design only, no code/directory changes shipped by this + proposal. +- **Depends on:** nothing structurally; it formalizes conventions already used + elsewhere in this repo (see Sketch). Consumed by, but does not depend on, the + agent-memory-layer design in the sibling private repo `PhysShell/007` + (`docs/agent-memory-layer.md` there) — that memory layer is one possible *source* of + promotions into the layer this proposal defines; a human editing `AGENTS.md` + by hand is another, equally valid, source. + +## Motivation + +`AGENTS.md` today is one flat 18-line file covering core commands, the lint +gate, the pipeline shape, and a handful of hard rules (unknown-call handling, +`assert_never` dispatch sites, codegen modes, OwnIR versioning). That is +exactly the right size for what it covers today. The risk is what happens as +it grows: this repo already carries 30+ proposals and 30+ design notes under +`docs/`, plus a second, much longer file +(`AGENTS.execution-surfaces.md`, 14KB) for a single ADR. Two failure modes are +already visible in miniature: + +1. **One file bloats past readability.** An agent-guidance file that grows by + accretion (one more bullet per lesson learned) turns into the "corporate + Confluence after three reorgs" problem — long enough that neither a human + nor an agent reliably reads all of it before acting. +2. **Guidance scatters undiscoverably.** `docs/notes/` already holds + agent-relevant lessons (e.g. `docs/notes/field-notes-patterns.md`, + `docs/notes/agent-capability-layer.md`) that a coding agent has no reason to + load unless it happens to grep for them. + +Separately, an external tool +([`claude-reflect`](https://github.com/BayramAnnakov/claude-reflect)) and the +sibling private harness `007` (which drives `claude`/`codex` over this repo +from the outside — see `007`'s own `README.md`) both converge on the same +idea: corrections and repeat-failure patterns from real agent runs should +become reviewed, persistent project memory rather than being re-learned every +session. `007` is explicitly the place that *mines* run records for candidate +learnings (`docs/agent-memory-layer.md` in that repo) — it is private, and its own +`README.md` is explicit that the harness must stay private (its auth/agent- +routing internals must not land in a public tree). What this repo needs, +independent of whether 007 ever ships that engine, is: **a defined, reviewed, structured place those promotions — or a +human's own manual corrections — land in.** That is this proposal's entire +scope: the destination shape, not a detector. + +## Scope + +### The directory + +```text +AGENTS.md # short index — points into .agents/*.md, nothing else +.agents/ + commands.md # how to run check/emit/cfg/report, tests, lint — today's AGENTS.md body + invariants.md # hard rules: unknown-call handling, assert_never sites, OwnIR versioning + gates.md # prose description of what CI/agents must run and why + codegen.md # the two codegen modes; "do not add runtime released? flags" + frontend-roslyn.md # Roslyn extractor rules: bin/ refs, OWN050 advisory, facts-only boundary +.007/ + gate.toml # machine-readable mirror of gates.md — see below +``` + +`AGENTS.md` shrinks to an index in the same shape this repo already uses for +`docs/proposals/README.md` (a table of what exists and one line of status) and +`docs/ROADMAP.md` (a hub linking to satellite documents). This is not a new +idiom for the repo — it is applying the hub-and-satellite pattern the repo +already relies on to agent guidance specifically, instead of one growing file. + +Each `.agents/*.md` file gets a soft line budget (~150 lines, the same +heuristic `claude-reflect` uses to warn on oversized memory files). Once a +file would cross that budget, it splits — same discipline that already +produced 30+ separate proposals instead of one `PROPOSALS.md`. + +### `.007/gate.toml` + +This repo does not yet have a `.007/` directory. `007`'s `o7 run` already +looks for `/.007/gate.toml` by convention and ships a worked example at +`007/examples/gate.own.net.toml` (three steps: `ruff check .`, `mypy ownlang`, +`python tests/run_tests.py` — a direct read of the current `AGENTS.md` lint +and regression rules). Adopting it here means: + +```toml +schema = 1 + +[[gate]] +name = "ruff" +cmd = "ruff check ." +required = true + +[[gate]] +name = "mypy-ownlang" +cmd = "mypy ownlang" +required = true + +[[gate]] +name = "regression" +cmd = "python tests/run_tests.py" +required = true +``` + +`gates.md` is then prose *about* this file (why each gate exists, what to run +when only touching a subset), not a competing source of truth — see the +open question on generation below. + +### The promotion contract + +Whatever proposes a change to `.agents/*.md` — a human noticing a repeat +correction, or a human-confirmed memory entry promoted out of `007`'s (separate, +private) memory layer — must arrive as an ordinary reviewed PR carrying: + +- **the rule itself**, scoped to one `.agents/*.md` file and section; +- **provenance**: what motivated it (a run, a PR comment, a postmortem) — + free text is enough, this is not a machine-checked field; +- **no bypass of normal review**. This repo does not gain a direct-write or + auto-merge path for agent memory. A promotion patch is a diff like any + other; it goes through the same PR process as this proposal itself. + +## Non-goals + +- **No detector/queue/regex-capture pipeline lives here.** Mining agent runs + for candidate learnings is explicitly `007`'s concern (private, separate + repo) or a human's own judgment — never a component added to this repo. +- **No live prompt-capture hook** (claude-reflect's `UserPromptSubmit` + mechanism). This repo has no chat surface to hook, and does not gain one for + this purpose. +- **No unreviewed auto-write.** Every change to `.agents/*.md` or + `.007/gate.toml` is a normal, human-reviewed commit — the same bar as any + other source change in this repo. +- **No cross-project memory.** `.agents/` describes this repo only; it is not + a place to accumulate generic "how agents should behave" advice that belongs + in a user's own global config. +- **No new rule DSL.** `.agents/*.md` is prose for humans and agents to read, + same register as the existing `AGENTS.md`; `.007/gate.toml` is the one + machine-readable artifact, and it already has an owner (`007`'s + `GateManifest` parser) — this proposal does not invent a second one. +- **Not a replacement for `.cursor/rules` or `.roo/rules-*`.** Those already + exist for tool-specific surfaces; this proposal does not touch them (see + open question below on whether they should later generate *from* + `.agents/`, not the reverse). + +## Sketch + +Today's `AGENTS.md` maps onto the split almost line-for-line, which is a good +sign the split is carving at a real joint rather than inventing one: + +| Current `AGENTS.md` line | Destination | +| --- | --- | +| `python -m ownlang check\|emit\|cfg\|report` usage | `.agents/commands.md` | +| `tests/run_tests.py` / `test_codegen_props.py` invocation | `.agents/commands.md` | +| `ruff check .` + `mypy` gate | `.agents/gates.md` (+ `.007/gate.toml`) | +| Ruff SIM omission rationale | `.agents/gates.md` | +| Pipeline shape (parser → CFG → analyses → diagnostics) | `.agents/invariants.md` | +| Unknown-call hard-error rule | `.agents/invariants.md` | +| `assert_never` dispatch-site rule | `.agents/invariants.md` | +| Codegen's two modes / no runtime flags | `.agents/codegen.md` | +| OwnIR schema versioning rule | `.agents/invariants.md` | +| `own-check.sh`/`.ps1`, `--flow-locals` default | `.agents/frontend-roslyn.md` | +| Roslyn `bin/` refs / OWN050 | `.agents/frontend-roslyn.md` | +| `audit/` decoupling note | `.agents/invariants.md` (one line, pointer to `audit/README.md`) | +| CodeGraph MCP preference | `.agents/commands.md` | + +`AGENTS.md` becomes: + +```markdown +# AGENTS.md + +Guidance for agents working in this repo. Start here, then follow a link: + +| File | Covers | +|---|---| +| `.agents/commands.md` | How to run check/emit/cfg/report, tests, lint | +| `.agents/gates.md` | What must pass before a change lands, and why | +| `.agents/invariants.md` | Hard rules that must not be violated | +| `.agents/codegen.md` | Codegen modes and constraints | +| `.agents/frontend-roslyn.md` | Roslyn extractor rules and boundaries | +``` + +## Open questions + +1. **Should `.agents/gates.md` be generated from `.007/gate.toml`, or hand + written?** P-023 (`Own.Arch`) already rejects the "two parrots" pattern for + C4 diagrams vs. `rules.yaml` — the same logic applies here: if `gates.md` + drifts from what `gate.toml` actually runs, agents get told one thing and + CI does another. Leaning: `gate.toml` is the source of truth; `gates.md` + carries only the *why*, with a generated table of the *what* (name + cmd) + checked in CI so drift fails loudly rather than rotting silently. +2. **Does the promotion contract need its own proposal**, or does it stay a + section of this one? Leaning: stays here until there's a second consumer + besides `007`'s memory-layer design — one contract, one place, until proven + otherwise. +3. **Should `.cursor/rules/` and `.roo/rules-*/` eventually generate from + `.agents/`** instead of maintaining separate tool-specific prose? Not in + scope for this proposal's MVP, but the same "single source of truth, many + renderers" instinct that shaped the `.007/gate.toml` question applies. Left + for a later proposal once `.agents/` exists and the duplication is real + (not hypothetical). +4. **Line-budget enforcement:** soft convention (reviewers watch for it) or a + CI check (`wc -l` gate on `.agents/*.md`, matching the "warn past ~150 + lines" heuristic)? Leaning: start as a soft PR-review convention; only add + a mechanical gate if bloat actually recurs — no gate for a problem that + hasn't happened yet. +5. **Timing relative to `007`'s memory layer:** this proposal's directory + layout is useful on its own (splitting an already-growing `AGENTS.md`) + regardless of whether `007`'s mining engine ever ships. It should not block + on that design landing first. diff --git a/docs/proposals/P-030-naughty-strings-testing.md b/docs/proposals/P-030-naughty-strings-testing.md new file mode 100644 index 00000000..e27d9d97 --- /dev/null +++ b/docs/proposals/P-030-naughty-strings-testing.md @@ -0,0 +1,203 @@ +# P-030 — Naughty-strings robustness pack (BLNS-driven crash testing) + +- **Status:** draft +- **Depends on / relates to:** + - [P-001](P-001-csharp-extractor.md) — the C# → OwnIR extractor: the thing that + has to survive arbitrary third-party source text in the first place. + - [P-012](P-012-bug-corpus-mining.md) — same "curated corpus, gated in CI" + shape, but keyed by **string content**, not by bug pattern; orthogonal, not a + replacement. + - [P-015](P-015-configuration-surface.md) — the future `own.toml`/`.ownrc` + config surface; config discovery has to survive naughty paths/globs too. + - [P-024](P-024-security-audit-profile.md) — same "берём готовое, не + изобретаем свою эвристику" instinct (adopt an existing corpus instead of + hand-rolling a dozen Unicode edge cases), but explicitly **not** a security + profile — see Non-goals. + +## Motivation + +Own.NET's whole value proposition is running against real, uncontrolled legacy +C#/WPF/DevExpress code: arbitrary identifiers, string literals, resource +strings, file and project paths chosen by other people over twenty years. The +project's own honest-skip philosophy (`docs/ROADMAP.md`) already treats "the +checker doesn't know" as an acceptable, first-class outcome — but a **crash** +is not "doesn't know," it's the tool falling over on a customer's codebase, +which is strictly worse than a missed diagnostic. + +Today robustness is exercised only by *valid* fixtures — `corpus/wpf/`, +`corpus/real-world/`, `tests/fixtures/` — plus whatever `ParseError`/`LexError` +paths (`ownlang/lexer.py`, `ownlang/parser.py`) happen to be hit incidentally. +Nothing in the suite deliberately throws adversarial *text* at the extractor, +the JSON/SARIF emitters (`ownlang/cfg_json.py`, `ownlang/diag_sarif.py`, +`ownlang/diagnostics.py`, `ownlang/report.py`), or the CLI (`ownlang/__main__.py`) +— zero-width joiners, RTL/LTR override characters, unpaired surrogates, SQL/ +XSS-shaped strings sitting inert inside a C# string literal, absurdly long +lines, mixed line endings, strings that are themselves valid-looking JSON or +XML, "your kernel just crashed"-style command injection payloads. And this +class of bug is not hypothetical here: `OwnAudit/Run-Audit.ps1` already carries +a scar from exactly this — `PYTHONUTF8=1` is set specifically to dodge a +**cp1251 console crash on a Russian-locale Windows target**. That is one +instance of the bug class BLNS exists to catch *systematically*, found the hard +way instead of by test. + +[Big List of Naughty Strings](https://github.com/minimaxir/big-list-of-naughty-strings) +(BLNS) is a maintained, MIT-licensed corpus built for exactly this: ~500 strings +(Unicode edge cases, escaping/injection-shaped strings, whitespace and +line-ending oddities, format-breakers for JSON/XML/CSV/SQL/shell), shipped as a +plain `blns.json` array plus a `.NET` port (`NaughtyStrings` NuGet package) for +the C#-side pieces (`audit/`'s eventual C# on lift-out, per +[`OwnAudit/README.md`](https://github.com/PhysShell/OwnAudit/blob/main/README.md)). BLNS itself is +explicit that it is not a substitute for real security testing (see +Non-goals) — its contract here is narrower and cheaper: **the tool must not +crash, hang, or corrupt output on any string in the corpus.** + +## Scope + +1. **Vendor the corpus.** A pinned, static copy of `blns.json` (upstream tag/ + commit recorded in a comment) as a fixture, e.g. + `tests/fixtures/blns.json` — no network fetch at test time, no submodule + (matches the project's existing "no external runtime deps beyond stdlib" + posture in the Python core). + +2. **Layer 1 — lexer/parser/extractor.** Parametrize over every BLNS entry, + embedding it as: (a) `.own` string-literal content, (b) a C# string literal + fed through the P-001 extractor, (c) a file/module name passed on the CLI. + Assert only: no unhandled exception escapes `ownlang/lexer.py` / + `ownlang/parser.py` / the extractor; the *only* acceptable failure shapes + are `LexError`/`ParseError` (or the extractor's own diagnostic-and-skip + path) — never a raw traceback, never a hang past a fixed timeout. + +3. **Layer 2 — serialization.** Pipe BLNS content through + `ownlang/diagnostics.py` → `ownlang/diag_sarif.py` / `ownlang/cfg_json.py` / + `ownlang/report.py` (as a synthesized finding message / file path / symbol + name) and assert the emitted JSON/SARIF/Markdown is well-formed + (round-trips through a JSON/SARIF parser) with no crash — this is the + layer `test_cfg_json.py` / `test_diag_sarif.py` already exercise for valid + input; this proposal is the adversarial-input twin. + +4. **Layer 3 — CLI & future config.** `ownlang/__main__.py` argument/path + handling, and (when [P-015](P-015-configuration-surface.md) lands) `own.toml` + discovery, given BLNS-flavored file names, directory names, and glob + patterns. Same contract as Layer 1, spelled out for I/O: the *only* + acceptable rejections are `FileNotFoundError` / `IsADirectoryError` / + `PermissionError` / `UnicodeDecodeError` / `OSError` surfaced as a clean CLI + error — the shape `cmd_explain`'s `--json` path already uses + (`except (OSError, json.JSONDecodeError)`) — and, once P-015 lands, a + documented config-parse error; never a raw traceback, never a hang past a + fixed timeout. This pack is expected to *find*, not assume, that contract: + today `_read()` — the path opener behind `check`/`emit`/`cfg`/`report` — + catches nothing, so a BLNS-flavored path (a null byte, an unpaired + surrogate, a name that turns out to be a directory) is a live candidate for + turning this layer red on day one, not a hypothetical. + +5. **Land as one hermetic, parametrized module** — + `tests/test_naughty_strings.py` — wired into `tests/run_tests.py` and CI the + same way `tests/test_corpus.py` is: fast, offline, property-style + ("must not crash," not "must produce code X"). + +6. **Follow-on, not in v0:** an equivalent pass over `OwnAudit`'s SARIF + ingestion / `artifacts/health-report.*` rendering, since that's the other + place free text from arbitrary source flows into output — deferred because + it crosses the repo boundary and OwnAudit already treats SARIF as its + external contract. + +## Non-goals + +- **Not a security test / pentest substitute.** BLNS's own README says the + same. This proposal claims only "does not crash / does not corrupt state on + adversarial text" — nothing about exploitability, authorization, or network + surface. That territory is [P-024](P-024-security-audit-profile.md)'s, and + this proposal does not overlap it: no scanning, no CVE claims, no new + security-flavored diagnostic codes. +- **Not a new checker or diagnostic.** No new `OWN0NN` code, no severity + change, no touch to ownership/lifetime semantics. Purely a regression/ + robustness harness around existing entry points. +- **Not coverage-guided fuzzing.** That is `007`'s `fuzz/` (cargo-fuzz) + territory on the eventual Rust core (P-022) — an open-ended search for novel + crashes. This is a fixed, curated, deterministic corpus, cheap enough to run + on every commit, not a campaign. +- **Not "every naughty string gets a pretty diagnostic."** The honest-skip / + `ParseError` contract is sufficient; the property under test is "no crash, + no hang, no corrupted output," not "graceful handling with a nice message" + for all ~500 entries. +- **Does not change the `corpus/` layout** (`before.cs`/`after.cs`/`case.own`) + from P-012 — BLNS fixtures are a separate, orthogonal corpus keyed by string + content, not by bug pattern, and live under `tests/fixtures/`, not `corpus/`. + +## Sketch + +```text +tests/fixtures/blns.json # vendored, pinned copy (upstream commit noted) +tests/test_naughty_strings.py # parametrized over every entry, 3 layers above +``` + +```python +import json, os +from ownlang.lexer import LexError +from ownlang.parser import ParseError, parse + +with open(os.path.join(os.path.dirname(__file__), "fixtures", "blns.json"), + encoding="utf-8") as f: + BLNS = json.load(f) + + +def run() -> int: + """One case per BLNS entry, in the suite's zero-dependency style: no + pytest — tests/run_tests.py auto-discovers every test_*.py that exposes + a run() -> int, same as tests/test_corpus.py.""" + fails: list[str] = [] + for naughty in BLNS: + # Embed the entry in a grammatically legal STRING position: per + # ownlang/parser.py's grammar, string literals appear only in + # resource emit_*/kind members (a `let` rhs never takes a string), + # so a benign entry parses cleanly and only the naughty content is + # under test — the wrapper follows the run_tests.py PRELUDE shape. + # Escape for Own string syntax first (the lexer decodes \\ and \") + # so entries containing quotes/backslashes reach the parser as + # themselves instead of terminating the literal early or being + # decoded into different text — otherwise those exact corpus rows + # would be silently skipped as "honest rejections". + payload = naughty.replace("\\", "\\\\").replace('"', '\\"') + src = f'module M\nresource R {{ acquire a release r emit_type "{payload}" }}\n' + try: + parse(src) + except (ParseError, LexError): + pass # an honest rejection is fine; anything else is a bug + except Exception as e: + fails.append(f"{naughty!r}: {type(e).__name__}: {e}") + for f in fails: + print(f"NAUGHTY FAIL: {f}") + return 1 if fails else 0 +``` + +Two complementary paths per entry: the *escaped* embedding above proves the +parser/analyzer path survives the payload as data; Layer 1 additionally feeds +the **raw, unescaped** entry straight to `ownlang.lexer` (no wrapper), where +`LexError` is a legal outcome — that path exercises exactly the quote/escape/ +control-character entries the escaped wrapper neutralizes. + +Serialization side follows the same shape against `diag_sarif.py` / +`cfg_json.py`, asserting `json.loads(...)` / a SARIF-shape check succeeds. + +## Open questions + +1. **Generation vs. fixture files.** Synthesize `.own`/`.cs` source around each + BLNS entry on the fly (parametrized, no repo bloat — the sketch above) vs. + materializing ~500 tiny fixture files. Leaning generation; only fall back to + files if a specific entry needs a shape the generator can't express. +2. **Timeout bound.** Several BLNS entries are specifically shaped to blow up + naive parsers (repetition/expansion strings). What per-case wall-clock + cutoff counts as "hung" in CI? +3. **Encoding boundary.** Is UTF-8 the only contract for this pack, or does + OwnAudit's cp1251-console incident warrant its own explicit BLNS pass over + the PowerShell/console path, given it already burned once? +4. **Vendoring mechanics.** Pinned static copy of `blns.json` (simple, matches + current no-submodule posture) vs. a `scripts/` updater that re-fetches on + demand — leaning static copy with the upstream commit noted in a header + comment. +5. **Rust-core inheritance.** When/if P-022's Rust core lands with a + differential oracle (Python = golden), does this pack become a shared input + fed to both sides rather than a Python-only test? +6. **OwnAudit follow-on timing.** Scope item 6 defers the OwnAudit-side pass — + confirm that's the right call now vs. folding it in immediately given the + cp1251 precedent already lives there. diff --git a/docs/proposals/P-031-resource-model-files.md b/docs/proposals/P-031-resource-model-files.md new file mode 100644 index 00000000..eddea1c5 --- /dev/null +++ b/docs/proposals/P-031-resource-model-files.md @@ -0,0 +1,242 @@ +# P-031 — Project resource model files (declarative acquire/release/capture) + +- **Status:** draft +- **Depends on:** + - [P-001](P-001-csharp-extractor.md) — the Roslyn extractor → OwnIR seam this + proposal feeds; it adds a new *fact producer*, not a new fact shape. + - [P-014](P-014-semantic-resolution.md) — the project-local `SemanticModel` + resolution this proposal reuses verbatim: a model entry is bound to a real + symbol the same way a bare `+=` is bound to a real `IEventSymbol` today, with + the same honest-skip discipline (unresolved → **OWN050**, never a guess). + - `ownlang/ownir.py` `_prelude_resources()` (the built-in `Subscription` / + `Timer` / `Disposable` / `PooledBuffer` table, lines ~801-821) — the existing, + but **compiled-in**, form of exactly this idea. This proposal externalizes the + *extractor-side* half of that table (which real C# call sites feed a kind), + not the core lowering, which is untouched. + - `ownlang/ownir.py` `_KNOWN_RESOURCE_KINDS` / `_RESOURCES` and + `spec/ownir.schema.json`'s `resourceKind` — the **closed** discriminator + vocabulary (`subscription` / `subscribe` / `timer` / `disposable` / + `local-disposable` / `pool`, plus `capture` / `unresolved-subscription`) with + a **fixed** `resource → [resource: tag]` mapping (spec/OwnIR.md §4); + `load()` rejects an unknown discriminator (fail-loud). v0 of this proposal + binds to that vocabulary rather than extending it — see *Scope* and the + *Non-goal* on minting new kinds/tags. + - [P-015](P-015-configuration-surface.md) — the sibling config surface + (check on/off, severity). Deliberately **not the same file**: see + *Relationship to P-015* below. + - [P-024](P-024-security-audit-profile.md) — the precedent this proposal must + not repeat. See *Relationship to P-024* below. +- **Strategy hub:** [`docs/ROADMAP.md`](../ROADMAP.md). + +## Motivation + +Every resource kind Own.NET knows about today — `Subscription` (`event +=`/`-=`), +`Timer` (`DispatcherTimer.Start`/`Stop`), `Disposable` (`new` / `Dispose`), +`PooledBuffer` (`ArrayPool.Rent`/`Return`) — is recognised by a **hardcoded C# +syntax classifier** inside `frontend/roslyn/OwnSharp.Extractor/Program.cs` (a +single ~4,400-line file; see the split deferred in +[`consolidation-and-positioning.md`](../notes/consolidation-and-positioning.md)). +The mapping from "this call is an acquire of that kind" to an OwnIR fact lives in +C# `if`/`switch` logic, not in data. + +That is fine for patterns common enough to justify a core classifier (an `event`, +`ArrayPool`). It breaks down for the pattern every real codebase has at least +one of: an **in-house** acquire/release pair that is semantically identical to +`Subscribe`/`Dispose` but syntactically invisible to the extractor — +`ConnectionFactory.Open()` / `Connection.Close()`, `Registry.RegisterCallback()` / +the returned token's `.Unregister()`, a legacy `EventBus.Subscribe(...)` that +predates `IObservable`. Today the only way to catch a leak of *that* resource +is to add a new hardcoded classifier to the extractor and cut a new Own.NET +release — the exact "teach the analyzer your project's `OpenDbConnection()` +convention" capability Coverity ships as **custom models**, which Own.NET has no +equivalent of. + +This is a real gap, not a nice-to-have: the fleet's own real-world corpus already +has cases whose bug class (a project-local `Close`/`Dispose` convention Own.NET +doesn't know) would be a model-file entry rather than a new core classifier, if +this surface existed. + +## Scope + +A discovered, versioned, project-local file — working name `own.models.yaml` +(location/format TBD, see *Open questions*) — that declares **additional** +acquire/release/capture *call sites* for the **existing, closed** OwnIR +`resource` discriminators, in terms of real bound symbols, not source-text +patterns. v0 does **not** mint new discriminators or new `[resource: …]` tags +(both are closed vocabulary — see *Non-goals*); a model entry picks which +already-known analysis path (and its already-fixed tag) a project-specific call +pair feeds: + +```yaml +resources: + - name: DbConnection # documentation label only — never emitted + resource: disposable # existing discriminator -> fixed tag "disposable field" + acquire: + - method: "MyApp.Data.ConnectionFactory.Open" + release: + - method: "MyApp.Data.Connection.Close" + - method: "System.IDisposable.Dispose" # already-known BCL release still allowed + + - name: LegacyBusToken + resource: subscribe # existing discriminator -> fixed tag "subscription token" + acquire: + - method: "MyApp.Messaging.EventBus.Subscribe" + release: + - method: "MyApp.Messaging.SubscriptionToken.Unregister" +``` + +A finding on `DbConnection` above reads `[resource: disposable field]`, not +`[resource: db connection]` — the same tag an ordinary hardcoded `disposable` +hit would carry. That precision loss (the message names the *bucket*, not the +project's own vocabulary) is the price of zero core/schema change in v0; see +Open Question 6 for the natural, separately-scoped follow-up. + +Two call shapes only, each binding onto a fact the core already understands (no +new `ownlang` code path — this is purely a new fact *producer* at the extractor +edge, choosing an existing `resource:` value, exactly like routing one more +call site into an existing entry of `_prelude_resources()`'s table instead of +compiling it in): + +1. **Paired method acquire/release** — `method:` names a call by its canonical + Roslyn symbol display string. The extractor resolves it through the same + project-local `CSharpCompilation`/`SemanticModel` P-014 already builds (plus + `--ref-dir` for third-party assemblies); a call site binding to that exact + symbol emits an `acquire`/`release` OwnIR record under the declared + `resource:` discriminator, indistinguishable downstream from a hardcoded hit + on that same discriminator. +2. **Event-shaped add/remove pair** — for project APIs that mimic `+=`/`-=` + semantically (a custom `Subscribe`/`Unsubscribe` pair) without being a real + C# `event`, reusing the existing `subscribe`/`subscription` fact shape. + +## Non-goals + +- **Not a detection language.** Every entry is a **symbol reference resolved by + the compiler**, never a regex or glob over source text or identifier names. An + unresolved entry (typo, renamed method, unreferenced assembly) is skipped with + the same **OWN050**-style advisory P-014 already uses for an unresolved event — + never a silent guess, never a string-match fallback. This is the line that + keeps this proposal apart from the *Own.SecurityChecks* idea P-024 permanently + rejected — see below. +- **Not a new severity/config surface.** A project resource kind reuses the + existing generic families (**OWN001** leak, **OWN002** use-after-release, + **OWN003** double-release, **OWN014** region escape) with its own + `[resource: ]` tag, exactly as WPF/DI/Pool do today. No per-kind custom + diagnostic codes, no new severity tier — that axis is P-015's. +- **Not a replacement for the built-in kinds.** `Subscription`/`Timer`/ + `Disposable`/`PooledBuffer` and the DI registration graph stay first-class, + shipped-by-default core classifiers. Model files are strictly **additive**, + for the project-specific long tail the fleet cannot pre-populate. +- **Not a registry or marketplace.** One file, local to the repo, under version + control — the `.editorconfig` model, not an npm-style package ecosystem. +- **Not general aliasing or a new lifetime-ordering DSL.** No way to declare a + custom region ladder (that stays the built-in `OWN014` ordering); the only + expressiveness added is *which symbols count as acquire/release/subscribe* for + a kind, not new semantics for what "leak" or "escape" means. +- **Not a way to mint new `resource` discriminators or new `[resource: …]` + display tags in v0.** Both are closed vocabulary today — + `ownlang/ownir.py::_KNOWN_RESOURCE_KINDS`/`_RESOURCES`, pinned by + `spec/ownir.schema.json`'s `resourceKind` — and `load()` fails loud on an + unknown value; adding a genuinely new one is a core vocabulary change that + must bump `OWNIR_VERSION` (spec/OwnIR.md §2), which this proposal does not + attempt. A model entry **binds** a project's call sites to one of the + existing discriminators (and inherits its existing fixed tag); it does not + extend the enum. See Open Question 6 for the scoped-out follow-up that would. + +## Relationship to P-015 + +P-015 is the sibling axis, not the same file: it turns existing findings +on/off and reweights severity per **category** (`.ownrc`/`own.toml`, format TBD) +and explicitly disclaims being "a query/policy language — it is a settings +file." This proposal is squarely that disclaimed territory, deliberately kept in +its **own** file: `own.models.yaml` declares *what counts as a resource fact* (an +extractor-input concern), while P-015 decides *what to do with a fact once +emitted* (a core-output concern). They compose — a project-declared kind is just +another category name in P-015's vocabulary — but conflating the two files would +turn the settings file into exactly the policy language P-015 rejects. If P-015 +ships first, unifying discovery (one nearest-config walk-up) is worth revisiting; +until then they are independent, optional files. + +## Relationship to P-024 (why this is not the rejected DSL) + +P-024 permanently rejected *Own.SecurityChecks* — a custom YAML DSL of +request/response matchers over banners/config, because its checks were +"regex-over-config" and "regex-over-banner": free-text pattern matching with no +semantic grounding, which is an intrinsic false-positive/false-negative +generator (a version regex misses `1.1.0`, over-fires on backports, etc.) and a +second, competing decision-maker outside the one core. + +This proposal's model entries are the opposite shape: they name a symbol, which +the Roslyn `SemanticModel` either **resolves to one real, unambiguous method / +event** or does not — there is no partial match, no pattern language, no text +scanning. Precision is inherited from the same binder P-014 already trusts for +the built-in kinds; a project model entry is exactly as sound (or as silently +skipped) as a hardcoded classifier would be for the same symbol. The verdict +authority stays exactly where P-013's "one checker" discipline puts it — the +Python core, deciding leak/escape over OwnIR facts — this proposal only widens +which C# call sites *produce* those facts. Nothing about severity, wording, or +the leak/escape verdict itself is decided in the model file. + +## Sketch + +```text +own.models.yaml (project root) + │ discovered + parsed by the extractor at startup + ▼ +ModelLoader: for each declared `method:` / event pair, + resolve via the SAME project-local SemanticModel P-014 builds + (framework refs + --ref-dir) — unresolved entry -> OWN050-style warning, skip + │ resolved entries route into the declared EXISTING `resource:` bucket + ▼ +existing acquire/release/capture OwnIR fact emission (unchanged) + │ + ▼ +ownlang core (unchanged): OWN001/002/003/014 over the widened fact set, + the bucket's already-fixed `[resource: …]` tag on the finding +``` + +No change to `ownlang/`, `spec/OwnIR.md`, or the JSON schema in v0 — a +project-bound call site is indistinguishable, downstream of fact emission, from +a hardcoded hit on the *same* discriminator (it is not a new discriminator; see +the Non-goal above). The only new component is a small loader in the Roslyn +extractor that turns YAML entries into the same internal classifier shape +`Program.cs`'s hardcoded ones already use (a natural companion to the extractor +split noted as deferred in `consolidation-and-positioning.md` — this is one +more reason that split earns its keep once this lands). + +## Open questions + +1. **File location & format.** Repo-root `own.models.yaml` discovered + independently (v0), or folded into P-015's future `.ownrc`/`own.toml` once + that ships? Leaning: ship independently now (P-015 is still a draft stub), + revisit unification later. +2. **Overload granularity.** Does `method: "T.M"` mean the whole method group or + one exact overload? Real conventions (`Open()` vs `Open(string)`) may need a + parameter-list suffix; start with "whole group," add signature narrowing if a + real corpus case needs it. +3. **Cross-type acquire/release.** `ConnectionFactory.Open()` returns a + `Connection`, but `release` is a method *on the returned type*, not on the + acquiring type — same shape the built-in `Disposable` kind already handles + (`new` on one type, `Dispose` on the value). Confirm the schema states + acquire/release independently (as sketched above) rather than requiring a + `release_on:` cross-reference; the built-in table's precedent says + independent declarations are enough. +4. **Scope of the WPF component heuristic.** Do project-declared kinds + participate in the `ViewModel`/`View`-shaped component detection P-004 uses, + or are they type-agnostic (fire anywhere a resolved acquire/release pair is + unbalanced, regardless of enclosing class shape)? Leaning: type-agnostic — + the WPF heuristic is specific to the *subscription-outlives-source* escape + judgment (OWN014), not to the ordinary leak judgment (OWN001) a custom + resource kind mostly needs. +5. **Silent staleness.** A renamed method quietly drops its model entry to + "always unresolved." Should the extractor emit one summary warning per run + listing every model entry it could not bind (so CI catches drift), the same + way an unresolved `--ref-dir` DLL is reported today? +6. **A free-text display tag, later.** v0 accepts the precision loss of + reusing an existing discriminator's fixed tag (`DbConnection` reads + `[resource: disposable field]`). A follow-up **could** add a small, + optional, additive `resourceRecord` field (e.g. `display_kind: string`) that + overrides only the rendered tag, without touching the closed `resource` + discriminator or its analysis-path routing — that is a genuinely additive + OwnIR schema change (§2), unlike minting a new discriminator, and would be + its own small PR against `spec/ownir.schema.json` + `ownlang/ownir.py`, not + a prerequisite for this proposal's v0. diff --git a/docs/proposals/P-032-own-arch-facts.md b/docs/proposals/P-032-own-arch-facts.md new file mode 100644 index 00000000..3561be02 --- /dev/null +++ b/docs/proposals/P-032-own-arch-facts.md @@ -0,0 +1,381 @@ +# P-032 — Own.Arch facts & intent model + +- **Status:** draft. Imported from a design discussion and normalized into the + proposal series (the pasted original self-titled itself P-027; renumbered here). +- **Extends:** [P-023 — Architecture guard](P-023-architecture-guard.md) — this + proposal deepens P-023's intent-model/drift design into a fact-extraction core. + +## Summary + +Introduce `Own.Arch` as the Own.NET-side architecture analysis core: a deterministic extractor and evaluator for architecture facts, intent models, and rule packs. + +This proposal is intentionally scoped to Own.NET. Own.NET should not become the full audit dashboard, PR gate, or agent runner. Its job is to produce reliable architecture facts and deterministic findings that other projects can consume. + +Own.NET owns: + +- project graph extraction; +- package/reference extraction; +- later Roslyn/type-level dependency extraction; +- architecture intent model schema; +- deterministic rule evaluation; +- stable finding fingerprints; +- generated architecture artifacts such as graph JSON and optional diagrams. + +OwnAudit consumes these outputs for reporting, baseline, drift, SARIF, and dashboards. + +007 consumes these outputs indirectly through typed refactoring tasks and gates. + +## Existing groundwork + +This builds directly on the existing "P-023 — Architecture guard (Own.Arch)" proposal. P-023 already defines the key architecture: hand-written intent model, extracted actual dependency graph, and drift as "actual - allowed"; it also explicitly says PRs should fail only on new architectural dependency violations, while existing debt is baselined and ratcheted. + +P-023 already scopes the MVP to project-level dependency checks over `.sln`, `.csproj`, `ProjectReference`, `PackageReference`, and `packages.config`, with rules such as forbidden project edges, forbidden packages per layer, unmapped projects, and unused allowed edges. + +P-023 also already sketches Phase 2 as type-level facts using IL/Roslyn to catch type dependencies, forbidden APIs, and namespace-level cycles. + +Related proposals: + +- [Agentic coding discipline proposal](../agentic-coding-discipline-proposal.md) + introduces disciplined agentic coding ideas for Own.NET/OwnAudit/007, including task contracts, diff policy gates, agent-readable invariants, and analyzer rule catalogs. + +- [P-031 — Project resource model files](P-031-resource-model-files.md) + proposes declarative per-project resource model files (`own.models.yaml`) for acquire/release/capture conventions. It is not an architecture model, but it is useful prior art for project-local declarative models resolved through the semantic layer. + +## Problem + +Own.NET already has strong ambitions around ownership, resources, WPF lifetime diagnostics, Roslyn extraction, and analyzer rules. The missing architecture piece is not “draw a diagram”. The missing piece is a stable fact model that can answer: + +- which projects belong to which architectural layer; +- which references cross forbidden boundaries; +- which packages are forbidden in a layer; +- which namespaces/types depend on presentation, persistence, SQL, DevExpress, or other sensitive APIs; +- which intended dependencies are unused and probably stale; +- which violations are new versus old debt. + +Without a deterministic architecture fact layer, any high-level architecture review becomes subjective prose. That is useless as a gate. A gate needs facts, fingerprints, and evidence. + +## Non-goals + +Own.NET should not own: + +- PR dashboards; +- long-term trend history; +- SARIF publishing to GitHub; +- AI-generated refactoring patches; +- 007 run records; +- runtime heap correlation; +- hand-maintained C4 diagrams as source of truth. + +Own.NET may generate diagrams from the intent model and graph, but diagrams must be artifacts, not the canonical architecture model. + +## Proposed design + +Introduce a small `Own.Arch` subsystem: + +```text +.sln/.csproj/packages.config + ↓ +project/package extractor + ↓ +arch-facts.json + +architecture.intent.json + ↓ +rule evaluator + ↓ +arch-findings.json + ↓ +optional generated artifacts: + - arch-report.md + - architecture.mmd + - structurizr.dsl +``` + +Phase 2 adds: + +```text +compiled solution / Roslyn workspace / IL + ↓ +type dependency extractor + ↓ +type-level arch-facts.json +``` + +## Architecture intent model + +Use JSON for the MVP, because OwnAudit already uses stdlib-only Python and existing `arch/rules.json` is JSON. + +Contract note (reconciling with P-023): P-023 names hand-written +`architecture.rules.yaml` as the intent-model source of truth. This proposal +keeps a single canonical *machine* contract — `architecture.intent.json`, the +form the evaluator reads — and treats P-023's YAML as the human authoring +layer that renders to it (`yaml → json`, in CI or locally). Until that +authoring layer exists, the JSON file is hand-written and reviewed directly; +at no point are there two independently edited sources of truth. + +Example: + +```json +{ + "schema": "own.arch.intent/v1", + "architecture": { + "name": "STS Broker", + "style": ["layered", "modular-monolith", "hexagonal-boundaries"] + }, + "layers": [ + { + "name": "Presentation", + "matches": ["*.UI.*", "*.Wpf.*", "*.ViewModels.*"], + "mayDependOn": ["Application", "DomainAbstractions"] + }, + { + "name": "Application", + "matches": ["*.Application.*", "*.Services.*"], + "mayDependOn": ["Domain", "DomainAbstractions"] + }, + { + "name": "Domain", + "matches": ["*.Domain.*", "*.Core.*"], + "mayDependOn": ["DomainAbstractions"], + "forbiddenApis": ["System.Windows.*", "System.Data.*", "DevExpress.*"] + }, + { + "name": "Infrastructure", + "matches": ["*.Infrastructure.*", "*.DataAccess.*"], + "mayDependOn": ["Domain", "DomainAbstractions"] + } + ], + "forbiddenPackages": { + "Domain": [ + "System.Data.SqlClient", + "Microsoft.Data.SqlClient", + "DevExpress*", + "PresentationFramework" + ], + "Application": [ + "DevExpress*", + "PresentationFramework" + ] + } +} +``` + +## Finding codes + +MVP deterministic rules: + +```text +ARCH001: ProjectReference crosses a forbidden layer boundary. +ARCH002: Forbidden package is referenced from a layer. +ARCH003: Project matches zero or multiple layers. +ARCH004: Forbidden direct framework/API reference at project level. +ARCH030: Allowed dependency is declared but unused. +``` + +Phase 2 rules: + +```text +ARCH010: Type-level dependency crosses a forbidden layer boundary. +ARCH011: Forbidden API is used in a layer. +ARCH012: Namespace-level dependency cycle. +ARCH013: Type-level dependency on forbidden framework namespace. +``` + +Phase 3 heuristic/report-only rules: + +```text +ARCH020: Type has excessive fan-out. +ARCH021: Type mixes APIs from unrelated layers. +ARCH022: Interface has too many members. +ARCH023: Component behaves as unstable hub. +``` + +Important: deterministic findings may gate. Heuristic findings are report-only until proven reliable on the project corpus. + +## Fingerprint policy + +Every deterministic architecture finding must have a stable fingerprint: + +```text +sha256(rule_id | normalized_from_symbol | normalized_to_symbol | normalized_target_kind) +``` + +Do not include file path or line number in the primary fingerprint. File paths and line numbers are evidence, not identity. + +This prevents baseline churn during refactors while still catching newly introduced architectural edges. + +## CLI sketch + +```bash +own-arch extract-projects \ + --solution Broker.sln \ + --out arch-facts.project.json + +own-arch evaluate \ + --facts arch-facts.project.json \ + --intent architecture.intent.json \ + --out arch-findings.json \ + --report arch-report.md + +own-arch render \ + --intent architecture.intent.json \ + --facts arch-facts.project.json \ + --format mermaid \ + --out architecture.mmd +``` + +Phase 2: + +```bash +own-arch extract-types \ + --solution Broker.sln \ + --configuration Release \ + --out arch-facts.types.json +``` + +## Output contracts + +"arch-facts.project.json": + +```json +{ + "schema": "own.arch.facts.project/v1", + "projects": [ + { + "name": "Broker.Presentation", + "path": "src/Broker.Presentation/Broker.Presentation.csproj", + "targetFrameworks": ["net472"], + "projectReferences": ["Broker.Application"], + "packageReferences": ["DevExpress.Xpf"], + "layer": "Presentation" + } + ], + "edges": [ + { + "from": "Broker.Presentation", + "to": "Broker.Application", + "kind": "ProjectReference" + } + ] +} +``` + +"arch-findings.json": + +```json +{ + "schema": "own.findings/v1", + "tool": "own-arch", + "findings": [ + { + "rule": "ARCH001", + "severity": "error", + "category": "architecture", + "resource": "Broker.Presentation", + "message": "Presentation depends on Infrastructure directly", + "fingerprint": "sha256:...", + "evidence": [ + { + "kind": "ProjectReference", + "from": "Broker.Presentation", + "to": "Broker.Infrastructure", + "path": "src/Broker.Presentation/Broker.Presentation.csproj" + } + ] + } + ] +} +``` + +## Integration with OwnAudit + +Own.NET produces: + +- "arch-facts.project.json"; +- "arch-facts.types.json"; +- "arch-findings.json"; +- optional generated diagrams/reports. + +OwnAudit consumes these artifacts for: + +- SARIF export; +- baseline/diff; +- PR drift report; +- dashboards; +- health scoring. + +Own.NET should not duplicate OwnAudit’s baseline/diff/reporting layer. + +## Integration with 007 + +007 should consume Own.Arch through task specs, not through direct architecture logic. + +Example 007 task target: + +```yaml +task_id: ownarch.fix.arch001.ui-sql +target_repo: Own.NET +input: + findings: artifacts/arch-findings.json + selector: + rule: ARCH001 + rank: 1 +constraints: + max_files_changed: 5 + require_tests: true + require_reaudit: true + forbid_baseline_update_without_reason: true +gates: + required: + - own-arch-evaluate + - no-new-arch-findings +``` + +## Acceptance criteria + +MVP is accepted when: + +1. "own-arch extract-projects" can read a solution/project set and emit "arch-facts.project.json". +2. "own-arch evaluate" can detect forbidden project edges, forbidden packages, and unmapped projects. +3. Findings use stable fingerprints. +4. Existing violations can be consumed by OwnAudit baseline/diff without schema translation hacks. +5. Generated markdown report includes evidence for every finding. +6. A simple Mermaid or Structurizr artifact can be generated from the intent model, but is not source of truth. +7. Tests cover: + - valid layered graph; + - forbidden Presentation → Infrastructure edge; + - Domain package pollution; + - unmapped project; + - multi-mapped project; + - unused allowed dependency. + +## Risks + +### Risk: Own.Arch becomes a second NDepend clone + +Mitigation: keep scope narrow. Own.Arch detects architecture dependency facts, not every possible code smell. + +### Risk: false confidence from inferred architecture style + +Mitigation: style inference must be report-only. Gates rely on deterministic facts. + +### Risk: duplicate rule languages + +Mitigation: one intent schema. Generated ArchUnitNET, NetArchTest, C4, Mermaid, or Structurizr outputs are optional render targets, not alternative sources of truth. + +## First implementation slice + +Implement only: + +```text +architecture.intent.json +project graph extraction +ARCH001 +ARCH002 +ARCH003 +arch-findings.json +arch-report.md +tests +``` + +Everything else waits. The first slice should be boring, deterministic, and hard to misinterpret. \ No newline at end of file diff --git a/docs/proposals/P-033-probabilistic-data-structures.md b/docs/proposals/P-033-probabilistic-data-structures.md new file mode 100644 index 00000000..df24f31c --- /dev/null +++ b/docs/proposals/P-033-probabilistic-data-structures.md @@ -0,0 +1,273 @@ +# P-033 — In-process sketches and bitmap indexes for legacy .NET diagnostics + +- **Status:** draft. Imported from a design discussion and normalized into the + proposal series (the pasted original suggested the then-taken number P-028). + Note on scope: the subject is **not** the OwnLang analyzer itself but the + legacy .NET Framework / WPF desktop application the audit targets (see + [`audit/README.md`](../../audit/README.md)). The proposed module would ship + as instrumentation guidance for audited legacy apps — e.g. alongside the + runtime harnesses in [`audit/runtime/`](../../audit/runtime/README.md) — not + as part of the analyzer. + +## Summary + +The legacy .NET application under audit (the WPF/.NET Framework desktop app targeted by `audit/README.md`) should gain a small, dependency-light module for compact runtime diagnostics and fast set operations using classic probabilistic and compressed data structures: + +- bitsets / roaring-style bitmap indexes; +- Top-K / heavy-hitter counters; +- Count-Min Sketch for approximate frequencies; +- t-digest or DDSketch-style latency summaries; +- optional Bloom/Cuckoo filters for import and lookup pre-checks; +- optional SimHash for grouping similar errors. + +The goal is not to turn a legacy desktop .NET application into a fake distributed analytics platform. That would be architecture cosplay, and nobody needs that circus. The goal is narrower: improve local diagnostics, filtering, dirty tracking, and performance visibility without requiring Redis, Valkey, Kafka, or some other infrastructure animal. + +## Problem + +The legacy .NET application under audit has several known pain points: + +- large legacy WPF/.NET Framework surface; +- heavy dictionaries and reference data; +- expensive recalculation paths; +- memory-sensitive UI workflows; +- difficult-to-debug performance spikes; +- repeated validation and import scenarios; +- need for better local evidence before changing architecture. + +Current code can observe some issues, but it likely lacks compact, queryable runtime summaries: + +- which operations are actually slow at p95/p99; +- which validations fail most often; +- which dictionary/reference entries are hot; +- which rows/documents are affected by a recalculation; +- which errors are effectively the same root cause; +- which imports contain duplicates or obviously invalid references. + +Without compact summaries, developers either over-log, under-measure, or guess. Guessing is not engineering. It is astrology with stack traces. + +## Proposed solution + +Add an internal module to the audited application, tentatively named +`Own.Diagnostics.Sketches`. + +The module should expose simple interfaces, not leak implementation details into business logic. + +Example conceptual interfaces: + +```csharp +public interface ILatencySketch +{ + void Record(long elapsedMilliseconds); + LatencySnapshot Snapshot(); +} + +public interface IHeavyHitters +{ + void Add(T item, long weight = 1); + IReadOnlyList> Top(int count); +} + +public interface IApproxFrequency +{ + void Add(T item, long count = 1); + long Estimate(T item); +} + +public interface IBitmapIndex +{ + void Add(int id); + void Remove(int id); + bool Contains(int id); + // Non-mutating: each set operation returns a new index; the receiver + // and `other` are never modified (no aliasing surprises for callers). + IBitmapIndex And(IBitmapIndex other); + IBitmapIndex Or(IBitmapIndex other); + IBitmapIndex Except(IBitmapIndex other); +} +``` + +The first implementation may be deliberately boring: + +- `BitArray` / custom packed bitset for dense ids; +- `HashSet` fallback for sparse ids; +- simple Space-Saving Top-K; +- simple Count-Min Sketch; +- latency sketch adapter with an initially simple histogram implementation. + +The point is to introduce the model safely before chasing cleverness. Cleverness without containment is how a “small optimization” becomes a haunted subsystem. + +## Candidate use cases + +### 1. Dirty tracking and affected-row calculation + +Use bitmap indexes to represent sets such as: + +- rows with validation errors; +- rows affected by changed customs rate; +- rows requiring recalculation; +- rows visible after current filter; +- rows already processed; +- rows excluded by user action. + +Instead of scanning large collections repeatedly, compute set operations: + +```text +RowsToRecalculate = + AffectedByRateChange + AND CurrentDeclarationRows + AND NOT AlreadyRecalculated +``` + +This is especially suitable when ids are stable integer indexes within a document/import/session. + +### 2. Validation and import diagnostics + +Use Top-K and Count-Min Sketch to track: + +- most frequent validation errors; +- most frequent invalid TNVED codes; +- most frequent import normalization problems; +- most frequently missing reference data; +- most common user correction patterns. + +This helps answer: + +Which 20 validation problems actually hurt users most? + +Not “which validation problems look important in a meeting”, because apparently humans needed a database to learn humility. + +### 3. Performance telemetry + +Use latency sketches to record p50/p90/p95/p99 for operations such as: + +- opening large WPF forms; +- loading reference dictionaries; +- graph 47 recalculation; +- report generation; +- import parsing; +- SQL query wrappers; +- UI filtering. + +The output should be local and cheap: + +```text +Operation: LoadTnvedTree +Count: 143 +p50: 120 ms +p95: 2.4 s +p99: 8.1 s +Max: 9.6 s +``` + +Average latency alone should be treated as suspicious. Averages hide pain like a rug hides broken glass. + +### 4. Error grouping + +Use SimHash-like fingerprints to group similar: + +- exception messages; +- stack traces; +- validation failure clusters; +- SQL error patterns. + +This can later connect to the existing idea of error ids, hidden stack traces, and build-aware deobfuscation. + +## Scope + +### MVP + +The MVP should include: + +1. `ILatencySketch` +2. `IHeavyHitters` +3. `IBitmapIndex` +4. one local diagnostic sink: + - JSON file; + - text report; + - or debug window export. +5. instrumentation examples for 2–3 real operations. + +Suggested first targets: + +- dictionary/reference loading; +- graph 47 recalculation; +- validation/import flow. + +### Phase 2 + +Add: + +- Count-Min Sketch; +- Bloom filter for import pre-checks; +- SimHash grouping; +- optional compact binary export; +- analyzer/test coverage for misuse. + +### Phase 3 + +Integrate with OwnAudit or 007 by exporting normalized evidence +(OwnAudit's `docs/sketch-based-evidence.md` already anticipates ingesting +these runtime diagnostic exports; the 007-side run evidence is specified in +007's `docs/sketch-aware-evidence.md`): + +```json +{ + "schema": "own.sketches.v1", + "source": "own.diagnostics.sketches", + "operation": "LoadTnvedTree", + "latency": { + "p50_ms": 120, + "p95_ms": 2400, + "p99_ms": 8100 + }, + "top_errors": [], + "affected_sets": [] +} +``` + +## Non-goals + +This proposal explicitly does not include: + +- adding Redis/Valkey as a runtime dependency; +- replacing SQL Server; +- changing business rules; +- introducing approximate answers into critical legal/business decisions; +- using Bloom/HLL/Count-Min for authorization, licensing, billing, or correctness checks; +- rewriting existing WPF flows around sketches. + +Approximate structures may support diagnostics and optimization. They must not become the source of truth for business decisions. Works fine?! A cart with three wheels “works fine” too. + +## Safety rules + +1. Every approximate structure must expose its error model in docs. +2. Approximate values must be named as estimates. +3. Exact fallback must exist where correctness matters. +4. Sketches must be resettable and exportable. +5. No global mutable singleton dumping random metrics from everywhere. +6. No business logic may depend on false-positive behavior. + +## Acceptance criteria + +The proposal is successful when: + +- a developer can instrument an operation in fewer than 10 lines; +- bitmap indexes can represent affected row sets and combine them efficiently; +- p95/p99 latency is visible for selected operations; +- Top-K diagnostics identify frequent validation/import issues; +- exported evidence can be consumed later by OwnAudit; +- no new infrastructure is required; +- no correctness-sensitive path relies only on probabilistic results. + +## Expected benefit + +The audited legacy application gets a practical local observability and set-processing layer: + +- fewer full scans; +- better dirty tracking; +- better recalculation targeting; +- better import diagnostics; +- clearer performance evidence; +- less guessing before refactoring. + +This is not highload cosplay. It is a small internal toolbox for making the old codebase confess where it hurts. \ No newline at end of file diff --git a/docs/proposals/P-034-runtime-lifetime-guard.md b/docs/proposals/P-034-runtime-lifetime-guard.md new file mode 100644 index 00000000..2fd982fa --- /dev/null +++ b/docs/proposals/P-034-runtime-lifetime-guard.md @@ -0,0 +1,168 @@ +# P-034 — Runtime lifetime guard & disposal quarantine + +- **Status:** draft. +- **Depends on:** `spec/OwnCore.md` (OWN001–003), [P-005](P-005-idisposable-ownership.md) (`D1`–`D5` `IDisposable` ownership), [P-004](P-004-wpf-lifetime-profile.md) (WPF profile); complements OwnAudit's runtime correlation (`OwnAudit/docs/runtime-contract.md`, phase 5). +- **Related, not overlapping:** [P-025](P-025-obligation-protocols.md) (obligation protocols) and [P-027](P-027-resource-state-machine.md) (resource state machines) are both **static, analysis-time** checks over source — P-027 explicitly ships no runtime type ("Own.NET does not ship a `ResourceState` NuGet package as the mandated fix"). This proposal is the complementary **dynamic/runtime** half: something that actually executes and throws. No redundancy either way. + +## Motivation + +Trigger: a design discussion asked whether the "harden malloc/free with paranoid +enterprise checks" idea (common in C/C++ shops — wrap the allocator to catch +use-after-free / OOB / double-free) has a .NET analog. Conclusion, worth pinning +down before anyone re-derives it: + +- As a **malloc shim**, the idea is nearly void under the CLR: bounds-checked + arrays, a tracing GC, and the absence of manual `free()` already remove the + C-style failure modes (use-after-free, OOB write, double-free) the + enterprise-malloc pattern exists to catch. +- The *idea itself* — make lifetime misuse loud instead of silent — has a real + .NET target, and Own.NET already built most of it as a **static** discipline: + `OWN001` (leak), `OWN002` (use-after-dispose), `OWN003` (double-dispose), + `OWN014` (region-escape / event retention) are exactly "managed + use-after-free" and "managed double-free" for `IDisposable` and event + subscriptions, proven end-to-end by P-005 (D1–D4 built) and + P-004/WPF-region-escape. OwnAudit's phase 5 (`runtime-contract.md`) then + confirms the WPF-specific case (event retention) against a real heap. + +So the static half of "enterprise lifetime checking" is not a gap — it's shipped. +What's actually missing is the **dynamic/runtime half**: a lightweight guard +that fails loudly *at run time* when misuse actually happens — a double-dispose +or a post-dispose access, whether it originates from an ownership hand-off +own-check doesn't model (P-005 D5) or from a cross-thread race (both explicit +non-goals of P-005) — plus a cheap, ClrMD-independent test-time check that runs +in an ordinary unit test, no Windows stand required. Neither piece models *why* +an object ended up misused, the way OwnAudit's phase 5 or a static prover would; +they only make the misuse itself loud the moment it happens. + +## What already exists (do not re-derive) + +| Idea from the discussion | Already covered by | +|---|---| +| use-after-`Dispose` as managed use-after-free | `OWN002`, built (P-005 D4, flow-sensitive intraprocedural) | +| double `Dispose` as managed double-free | `OWN003`, built (P-005 D3) | +| `IDisposable` field/local never released | `OWN001`, built (P-005 D1/D2) | +| event `+=` without `-=` (leak) | `OWN001` subscription-leak category (P-004 WPF001–005) | +| static-event / long-lived source retaining a short-lived subscriber | `OWN014` region-escape (`docs/lifetimes.md` §4, slice #2/#3) | +| `DispatcherTimer`/`Timer` not stopped | `WPF002` | +| singleton captures scoped dependency (captive dependency) | `DI001`–`DI005` (P-006) | +| `ArrayPool.Rent` without `Return`, use-after-`Return` | `POOL001`–`003` (P-007) | +| "boolean/nullable soup" standing in for a resource's lifecycle state | `ASYNC050` (P-027), static | +| runtime confirmation of a static leak against a real heap (WPF specifically) | OwnAudit phase 5 — `runtime.json` / ClrMD heap walk, confirmed / static-only / runtime-only buckets (`OwnAudit/docs/runtime-contract.md`) | + +If a future task proposes any of the above as new work, point back here first. + +## Deliberately out of scope already — don't re-open without new evidence + +- **Raw `IntPtr` / `Marshal.Alloc*`/`Free*` balance proof.** Tagged *impossible + statically* in the corpus-mining detectability matrix (P-012 §Non-goals: + "unmanaged cyclic refs / `Marshal.AllocHGlobal` freed on all paths"). A + flow-sensitive proof over arbitrary P/Invoke code is exactly the swamp + OwnLang's `native` buffer policy avoids by only covering *code compiled + through OwnSharp* (where `Free`/`NativeMemory.Free` is enforced by + construction, `spec/BufferPolicies.md`), not arbitrary legacy C#. +- **`SafeHandle` internals / finalizer ceremony.** Explicit non-goal in P-005: + "we care about the leak, not the ceremony." Requiring `SafeHandle` over a raw + `IntPtr` is a one-line syntactic lint if anyone wants it (no dataflow needed + — flag a field/param typed `IntPtr` that crosses a P/Invoke boundary and is + never wrapped) but nobody has asked for it yet; noted here as a cheap future + D-rule under P-005 if a corpus case ever needs it, not a commitment. + +## Scope — the new piece: a runtime lifetime guard + disposal quarantine + +Two small, independent, opt-in runtime helpers (a "diagnostic mode", not a +shipped allocator): + +**1. `LifetimeGuard` base / wrapper — loud instead of silent.** +A `DEBUG`/`TEST`-only `IDisposable` base that turns silent misuse — a +double-dispose, or an access after `Dispose()` — into an immediate +`ObjectDisposedException`/`InvalidOperationException` instead of corrupting +state quietly, regardless of whether the root cause was an unmodeled +ownership hand-off (P-005 D5) or a cross-thread race: + +```csharp +public abstract class LifetimeGuard : IDisposable +{ + private int _disposed; // Interlocked — catches the cross-thread double-dispose race by construction + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) != 0) + throw new ObjectDisposedException(GetType().Name, "double Dispose"); + DisposeCore(); + GC.SuppressFinalize(this); + } + protected void ThrowIfDisposed() { if (Volatile.Read(ref _disposed) != 0) throw new ObjectDisposedException(GetType().Name); } + protected abstract void DisposeCore(); +} +``` + +This does not replace `OWN002`/`OWN003` — those catch the mistake at compile +time, for free, when the pattern is intraprocedural. `LifetimeGuard` catches +what's left: it surfaces misuse *at the point it happens*, even when the +underlying cause was ownership handed through a callee own-check doesn't model +(P-005 D5) or the cross-thread race P-005 explicitly declines to touch — it +does not itself model or detect the hand-off/race, only the resulting +double-dispose or post-dispose access. + +**2. Disposal quarantine for tests — a ClrMD-free complement to phase 5.** +An opt-in `ITrackedDisposable` + an ambient registry that records the +allocation-site stack trace on construction and asserts, at test teardown, +that nothing tracked is still undisposed: + +```csharp +public interface ITrackedDisposable : IDisposable +{ + bool IsDisposed { get; } +} +// test base: DisposalQuarantine.AssertClean() at [TearDown] — throws with the +// recorded allocation-site stack trace for anything still live. +``` + +This is *not* a substitute for OwnAudit's phase 5 (`runtime-contract.md`) — +that walks the real CLR heap of the real app and is the only thing that proves +an object is *actually rooted* (via `roots[]`, e.g. a `static-event` delegate) +after realistic UI scenarios, on Windows, against STS. The quarantine only +proves "this object's own `Dispose()` was/wasn't called during this test" — +recall bounded by test coverage, exactly like any other dynamic check, and +blind to *why* something is still reachable. Its value is that it needs no CLR +heap walk, no Windows stand, and no compiled STS: it runs in an ordinary +`dotnet test`, in CI, for any class the team chooses to opt in — closer to a +debug assertion than an auditor. + +## Non-goals + +- Not a general-purpose allocator shim; nothing here wraps `malloc`/GC + allocation. +- Not a production-safe pattern as-is: throwing from `Dispose()` is a real + behavior change (already-suppressed `Dispose` exceptions in `finally`/`using` + chains can mask the original exception) — `LifetimeGuard` must ship + `DEBUG`/`TEST`-gated: wrap the throw in `#if DEBUG` / `#if TEST`, or route it + through a small helper method decorated `[Conditional("DEBUG")]` (the + attribute only applies to methods, not to a bare `throw` statement), or gate + it behind a config flag — never silently opt production code into new + exceptions. +- Not a replacement for `OWN002`/`OWN003` (compile-time, zero runtime cost, + works before the code ever ships) or for OwnAudit phase 5 (ground-truth heap + retention) — it fills the gap between them: cases neither can see, at the + cost of only firing when a test actually exercises the path. +- Not a replacement for P-027's static state-machine lint (`ASYNC050`) — that + flags the *shape* of ad-hoc lifecycle state in source; this catches *misuse* + of an actual `Dispose()` contract at run time. Different signal, same theme. +- Not proposing to relitigate the `Marshal.Alloc*`/`SafeHandle` non-goals + above. + +## Open questions + +1. Home for this: a new tiny package (`Own.Diagnostics`?) versus living inside + OwnAudit's `runtime/` as a test-time collector alongside the ClrMD one? + Leans OwnAudit, since it's audit tooling for *consumers'* code, not part of + the OwnLang core checker. +2. Does the quarantine registry need to be thread-safe / async-local scoped + per test, or is a single ambient static acceptable given tests already run + isolated per fixture? +3. Should `LifetimeGuard`'s double-dispose check be `Interlocked`-based by + default (cross-thread-safe) or opt-in, given most `IDisposable` usage in the + STS corpus is single-threaded and the extra `Interlocked.Exchange` has a + (tiny) cost? +4. Worth a corpus entry (P-012) once a real cross-thread + `ObjectDisposedException` or D5-transfer bug is mined, to validate + `LifetimeGuard` actually would have caught it? diff --git a/docs/proposals/README.md b/docs/proposals/README.md index b382cd7f..41a983dc 100644 --- a/docs/proposals/README.md +++ b/docs/proposals/README.md @@ -45,6 +45,15 @@ proposal is marked `done` with a pointer. | [P-023](P-023-architecture-guard.md) | Architecture guard (`Own.Arch`): rules.yaml intent model + dependency-graph gate + baseline ratchet | draft | | [P-024](P-024-security-audit-profile.md) | Security audit profile (external tools + SARIF adapters; rejects own scanner engine) | draft | | [P-025](P-025-obligation-protocols.md) | Obligation protocols (`Own.Protocols`): barrier-sensitive project invariants (OBL001–005) | first slice built (core + bridge + fixtures; extractor pending) | +| [P-026](P-026-csharp-strictness-retrofit.md) | C# strictness retrofit profile (`own audit strictness`): a witness/score over existing findings, not a new engine | draft (framing) | +| [P-027](P-027-resource-state-machine.md) | Resource state machines & stale-async-write detection (extends `Own.Async`) | draft | +| [P-028](P-028-unneeded-dependency-profile.md) | Unneeded-dependency profile (`Own.Lean`): evidence-only "you don't need this abstraction" findings (YDN001–002) | draft | +| [P-029](P-029-agent-memory-layer.md) | Agent memory & policy layer (`.agents/`): reviewed destination for AGENTS.md, gates, and learned-rule promotions | draft | +| [P-030](P-030-naughty-strings-testing.md) | Naughty-strings robustness pack (BLNS-driven crash testing of lexer/parser/extractor/serializers/CLI/config) | draft | +| [P-031](P-031-resource-model-files.md) | Project resource model files (declarative acquire/release/capture, symbol-resolved) | draft | +| [P-032](P-032-own-arch-facts.md) | Own.Arch facts & intent model: deterministic architecture-fact extractor/evaluator core (deepens P-023) | draft | +| [P-033](P-033-probabilistic-data-structures.md) | In-process sketches & bitmap indexes for legacy .NET diagnostics (Top-K, CMS, t-digest, Bloom) | draft | +| [P-034](P-034-runtime-lifetime-guard.md) | Runtime lifetime guard & disposal quarantine — the "enterprise malloc" idea, correctly scoped for .NET | draft | > For priorities, milestones, the framing, and the design philosophy across all > of these, see the strategy hub: [`docs/ROADMAP.md`](../ROADMAP.md). P-004 … P-016