diff --git a/code-reviews/Audit/findings.md b/code-reviews/Audit/findings.md new file mode 100644 index 0000000..2734f51 --- /dev/null +++ b/code-reviews/Audit/findings.md @@ -0,0 +1,36 @@ +# Code Review — Audit + +| Field | Value | +|-------|-------| +| Library | `ZB.MOM.WW.Audit/` | +| Packages | `ZB.MOM.WW.Audit` | +| Component spec | `components/audit/spec/SPEC.md` | +| Shared contract | `components/audit/shared-contract/ZB.MOM.WW.Audit.md` | +| Status | Not yet reviewed | +| Last reviewed | — | +| Reviewer | — | +| Commit reviewed | — | +| Open findings | 0 | + +## Summary + +_Not yet reviewed._ + +## Checklist coverage + +| # | Category | Examined | Notes | +|---|----------|----------|-------| +| 1 | Correctness & logic bugs | ☐ | | +| 2 | Public API surface & compatibility | ☐ | | +| 3 | Concurrency & thread safety | ☐ | | +| 4 | Error handling & resilience | ☐ | | +| 5 | Security & secret handling | ☐ | | +| 6 | Performance & resource management | ☐ | | +| 7 | Spec & shared-contract adherence | ☐ | | +| 8 | Packaging, dependencies & project layout | ☐ | | +| 9 | Testing coverage | ☐ | | +| 10 | Documentation & XML docs | ☐ | | + +## Findings + +_No findings recorded yet._ diff --git a/code-reviews/Auth/findings.md b/code-reviews/Auth/findings.md new file mode 100644 index 0000000..e1016cd --- /dev/null +++ b/code-reviews/Auth/findings.md @@ -0,0 +1,36 @@ +# Code Review — Auth + +| Field | Value | +|-------|-------| +| Library | `ZB.MOM.WW.Auth/` | +| Packages | `ZB.MOM.WW.Auth.Abstractions`, `ZB.MOM.WW.Auth.Ldap`, `ZB.MOM.WW.Auth.ApiKeys`, `ZB.MOM.WW.Auth.AspNetCore` | +| Component spec | `components/auth/spec/SPEC.md` | +| Shared contract | `components/auth/shared-contract/ZB.MOM.WW.Auth.md` | +| Status | Not yet reviewed | +| Last reviewed | — | +| Reviewer | — | +| Commit reviewed | — | +| Open findings | 0 | + +## Summary + +_Not yet reviewed._ + +## Checklist coverage + +| # | Category | Examined | Notes | +|---|----------|----------|-------| +| 1 | Correctness & logic bugs | ☐ | | +| 2 | Public API surface & compatibility | ☐ | | +| 3 | Concurrency & thread safety | ☐ | | +| 4 | Error handling & resilience | ☐ | | +| 5 | Security & secret handling | ☐ | | +| 6 | Performance & resource management | ☐ | | +| 7 | Spec & shared-contract adherence | ☐ | | +| 8 | Packaging, dependencies & project layout | ☐ | | +| 9 | Testing coverage | ☐ | | +| 10 | Documentation & XML docs | ☐ | | + +## Findings + +_No findings recorded yet._ diff --git a/code-reviews/Configuration/findings.md b/code-reviews/Configuration/findings.md new file mode 100644 index 0000000..7d23733 --- /dev/null +++ b/code-reviews/Configuration/findings.md @@ -0,0 +1,36 @@ +# Code Review — Configuration + +| Field | Value | +|-------|-------| +| Library | `ZB.MOM.WW.Configuration/` | +| Packages | `ZB.MOM.WW.Configuration` | +| Component spec | `components/configuration/spec/SPEC.md` | +| Shared contract | `components/configuration/shared-contract/ZB.MOM.WW.Configuration.md` | +| Status | Not yet reviewed | +| Last reviewed | — | +| Reviewer | — | +| Commit reviewed | — | +| Open findings | 0 | + +## Summary + +_Not yet reviewed._ + +## Checklist coverage + +| # | Category | Examined | Notes | +|---|----------|----------|-------| +| 1 | Correctness & logic bugs | ☐ | | +| 2 | Public API surface & compatibility | ☐ | | +| 3 | Concurrency & thread safety | ☐ | | +| 4 | Error handling & resilience | ☐ | | +| 5 | Security & secret handling | ☐ | | +| 6 | Performance & resource management | ☐ | | +| 7 | Spec & shared-contract adherence | ☐ | | +| 8 | Packaging, dependencies & project layout | ☐ | | +| 9 | Testing coverage | ☐ | | +| 10 | Documentation & XML docs | ☐ | | + +## Findings + +_No findings recorded yet._ diff --git a/code-reviews/Health/findings.md b/code-reviews/Health/findings.md new file mode 100644 index 0000000..d016820 --- /dev/null +++ b/code-reviews/Health/findings.md @@ -0,0 +1,36 @@ +# Code Review — Health + +| Field | Value | +|-------|-------| +| Library | `ZB.MOM.WW.Health/` | +| Packages | `ZB.MOM.WW.Health`, `ZB.MOM.WW.Health.Akka`, `ZB.MOM.WW.Health.EntityFrameworkCore` | +| Component spec | `components/health/spec/SPEC.md` | +| Shared contract | `components/health/shared-contract/ZB.MOM.WW.Health.md` | +| Status | Not yet reviewed | +| Last reviewed | — | +| Reviewer | — | +| Commit reviewed | — | +| Open findings | 0 | + +## Summary + +_Not yet reviewed._ + +## Checklist coverage + +| # | Category | Examined | Notes | +|---|----------|----------|-------| +| 1 | Correctness & logic bugs | ☐ | | +| 2 | Public API surface & compatibility | ☐ | | +| 3 | Concurrency & thread safety | ☐ | | +| 4 | Error handling & resilience | ☐ | | +| 5 | Security & secret handling | ☐ | | +| 6 | Performance & resource management | ☐ | | +| 7 | Spec & shared-contract adherence | ☐ | | +| 8 | Packaging, dependencies & project layout | ☐ | | +| 9 | Testing coverage | ☐ | | +| 10 | Documentation & XML docs | ☐ | | + +## Findings + +_No findings recorded yet._ diff --git a/code-reviews/README.md b/code-reviews/README.md new file mode 100644 index 0000000..d1a4b3f --- /dev/null +++ b/code-reviews/README.md @@ -0,0 +1,80 @@ +# Code Reviews + +Comprehensive, per-library code reviews of the `ZB.MOM.WW.*` shared libraries hosted +in this repo. Each library (one self-contained `.slnx` at the repo root) has its own +folder containing a `findings.md`. This README is the aggregated index — the single +place to see all outstanding work. + +> Generated by `regen-readme.py` from the per-library `findings.md` files. Do not +> edit by hand — edit the findings files and re-run the script. + +## How it works + +- Reviews are performed one library at a time against a fixed checklist. +- Each library is reviewed against its normalized component spec under `components/`. +- Every finding is recorded in the library's `findings.md` with a severity and status. +- Findings are **never deleted** — they are closed by changing their status, keeping + a full audit trail. +- This README aggregates every **pending** finding (`Open` / `In Progress`) across all + libraries. + +See **[REVIEW-PROCESS.md](REVIEW-PROCESS.md)** for the full procedure: the review +checklist, severity definitions, finding format, the library → component-spec mapping, +and how to mark items resolved. + +## Layout + +``` +code-reviews/ +├── README.md # this file — process overview + pending findings +├── REVIEW-PROCESS.md # how to perform a review and track findings +├── regen-readme.py # regenerates this README from the findings files +├── _template/findings.md # copy-this template for a library review +└── /findings.md # one folder per ZB.MOM.WW.* shared library +``` + +## Summary + +0 of 6 libraries reviewed. 0 pending findings across all libraries. + +| Severity | Open findings | +|----------|---------------| +| Critical | 0 | +| High | 0 | +| Medium | 0 | +| Low | 0 | +| **Total** | **0** | + +## Library Status + +| Library | Last reviewed | Commit | Open (C/H/M/L) | Open | Total | +|---------|---------------|--------|----------------|------|-------| +| [Audit](Audit/findings.md) | — | — | 0/0/0/0 | 0 | 0 | +| [Auth](Auth/findings.md) | — | — | 0/0/0/0 | 0 | 0 | +| [Configuration](Configuration/findings.md) | — | — | 0/0/0/0 | 0 | 0 | +| [Health](Health/findings.md) | — | — | 0/0/0/0 | 0 | 0 | +| [Telemetry](Telemetry/findings.md) | — | — | 0/0/0/0 | 0 | 0 | +| [Theme](Theme/findings.md) | — | — | 0/0/0/0 | 0 | 0 | + +## Pending Findings + +Every `Open` / `In Progress` finding across all libraries, highest severity first. +Resolved findings drop off this list but remain recorded in their library's +`findings.md` (see [REVIEW-PROCESS.md](REVIEW-PROCESS.md) §4–§5). Full detail — +description, location, recommendation — lives in the library's `findings.md`. + +### Critical (0) + +_None open._ + +### High (0) + +_None open._ + +### Medium (0) + +_None open._ + +### Low (0) + +_None open._ diff --git a/code-reviews/REVIEW-PROCESS.md b/code-reviews/REVIEW-PROCESS.md new file mode 100644 index 0000000..625e78e --- /dev/null +++ b/code-reviews/REVIEW-PROCESS.md @@ -0,0 +1,152 @@ +# Code Review Process + +This document describes how to perform a comprehensive, per-library code review of the +shared libraries hosted in **scadaproj** and how to track findings to resolution. + +scadaproj is an umbrella workspace, but it hosts first-party source: the six +`ZB.MOM.WW.*` shared libraries (the realized output of the +[component normalizations](../components/README.md)). Those libraries are the code that +gets reviewed here. A **library** is one self-contained solution at the repo root (its own +`.slnx`, with `src/` and `tests/`) — e.g. `ZB.MOM.WW.Auth/`. Each library has its own +folder under `code-reviews/` containing a single `findings.md`. + +The review unit is the **library**, not the individual NuGet package. A library that ships +several packages (Auth → 4, Health → 3, Telemetry → 2) is reviewed as a whole; findings +point at the specific package/file in their **Location**. + +### The six libraries and their design context + +Each library is reviewed against its normalized component spec. The library folder name is +the `ZB.MOM.WW.` prefix stripped; note the spec directory is **not** always the same word. + +| Library folder | Library root | Packages | Component spec | +|----------------|--------------|----------|----------------| +| `Auth` | `ZB.MOM.WW.Auth/` | `Abstractions`, `Ldap`, `ApiKeys`, `AspNetCore` | [`components/auth/`](../components/auth/) | +| `Theme` | `ZB.MOM.WW.Theme/` | single RCL | [`components/ui-theme/`](../components/ui-theme/) | +| `Health` | `ZB.MOM.WW.Health/` | `Health`, `.Akka`, `.EntityFrameworkCore` | [`components/health/`](../components/health/) | +| `Telemetry` | `ZB.MOM.WW.Telemetry/` | `Telemetry`, `.Serilog` | [`components/observability/`](../components/observability/) | +| `Configuration` | `ZB.MOM.WW.Configuration/` | single | [`components/configuration/`](../components/configuration/) | +| `Audit` | `ZB.MOM.WW.Audit/` | single | [`components/audit/`](../components/audit/) | + +## 1. Before you start + +1. Pick the library to review. Its review folder is `code-reviews//` where + `` is the table above (the `ZB.MOM.WW.` prefix stripped). +2. Identify the design context for the library: + - Its component spec: `components//spec/SPEC.md`. + - Its proposed shared contract: `components//shared-contract/ZB.MOM.WW..md`. + - The library's own `README.md` and the relevant row in the **Component normalization** + table in the root [`CLAUDE.md`](../CLAUDE.md). + - The adoption backlog `components//GAPS.md` (what is deliberately deferred). +3. Record the exact commit being reviewed: `git rev-parse --short HEAD`. Every review is a + snapshot — a finding only means something relative to a known commit. +4. Build and test the library so findings are grounded in a green (or known-red) baseline: + `cd ZB.MOM.WW. && dotnet test`. +5. Open `code-reviews//findings.md` and fill in the header table (reviewer, date, + commit SHA). + +## 2. Review checklist + +Work through **every** category below for the library. A comprehensive review means the +checklist is completed even where it produces no findings — record "No issues found" for a +category rather than leaving it ambiguous. + +These libraries are reusable packages consumed by three separate apps (OtOpcUa, +MxAccessGateway, ScadaBridge), so the bar is **API stability and minimal blast radius**: +a defect here can break every consumer at once. + +1. **Correctness & logic bugs** — off-by-one, null handling, incorrect conditionals, + misuse of BCL/framework APIs, broken edge cases. +2. **Public API surface & compatibility** — the public surface is intentional and minimal; + naming follows .NET conventions; nullable reference annotations are correct; `sealed` / + `abstract` / `virtual` choices are deliberate; no internal types leak through public + signatures; evolution is additive-only with semver discipline (libraries are at `0.1.0`). +3. **Concurrency & thread safety** — these libraries run inside concurrent hosts; options + and value types are immutable, singletons/statics are thread-safe, no shared mutable + state, correct `async`/`await`, no sync-over-async. +4. **Error handling & resilience** — exceptions thrown to consumers are appropriate and + documented; argument validation / guard clauses (`ArgumentNullException` etc.); no + swallowed exceptions; fail-fast vs. graceful degradation is deliberate. +5. **Security & secret handling** — Auth peppered-HMAC API keys and LDAP-injection surface; + redaction seams (`ILogRedactor` in Telemetry, `IAuditRedactor` in Audit); input + validation; no secrets or PII in messages, logs, or exception text. +6. **Performance & resource management** — `IDisposable` correctness and disposal, + allocations on hot paths, buffering/back-pressure, no unnecessary work in DI + registration, async all the way down. +7. **Spec & shared-contract adherence** — does the code match the library's + `components//spec/SPEC.md` and `shared-contract/ZB.MOM.WW..md`? + Flag both code that drifts from the normalized contract **and** spec/contract docs that + are now stale relative to the code. +8. **Packaging, dependencies & project layout** — minimal dependency footprint (e.g. Audit's + only non-BCL dependency is `Microsoft.Extensions.DependencyInjection.Abstractions`); + correct target framework (.NET 10); package id/version metadata; `dotnet pack` produces + the expected nupkg set; central versions in `Directory.Packages.props`; no public + dependency leakage; Abstractions-vs-implementation split; Options pattern owned by the + library; `AddZb*` DI registration ergonomics. +9. **Testing coverage** — are the library's behaviours covered by tests in its `tests/` + project? Note untested critical paths, missing edge-case tests, and whether the public + contract (not just internals) is exercised. +10. **Documentation & XML docs** — XML doc accuracy and presence on the **public** API + (these are libraries — public docs matter), library `README.md` correctness, misleading + or stale comments, undocumented non-obvious behaviour. + +## 3. Recording findings + +Add one entry per finding to the `## Findings` section of the library's `findings.md`, +using the entry format in [`_template/findings.md`](_template/findings.md). + +- **Finding ID** — `-NNN`, numbered sequentially within the library and never + reused (e.g. `Auth-001`, `Telemetry-003`). IDs are permanent even after resolution. +- **Severity:** + - **Critical** — security breach, data loss, or a defect that crashes/corrupts or + deadlocks every consuming app (e.g. a broken public contract, secret leakage). + - **High** — incorrect behaviour with significant impact across consumers, or a + public-API / back-compat break, with no safe workaround. + - **Medium** — incorrect or risky behaviour with limited impact or a workaround. + - **Low** — minor issues, style, maintainability, documentation. +- **Category** — one of the 10 checklist categories above. +- **Location** — `file:line` (clickable), or a list of locations. +- **Description** — what is wrong and why it matters. +- **Recommendation** — concrete suggested fix. + +After recording findings, update the library header table (status, open-finding count) and +refresh the base README (step 5). + +## 4. Marking an item resolved + +Findings are **never deleted** — they are an audit trail. To close one, change its +**Status** and complete the **Resolution** field: + +- `Open` — newly recorded, not yet addressed. +- `In Progress` — a fix is actively being worked on. +- `Resolved` — fixed. The Resolution field must state the fixing commit SHA, the date, and + a one-line description of the fix. +- `Won't Fix` — intentionally not fixed. The Resolution field must justify why. +- `Deferred` — valid but postponed. The Resolution field must say what it is waiting on + (e.g. a tracked issue, a `GAPS.md` item, or a later milestone). + +`Resolved`, `Won't Fix`, and `Deferred` findings are all considered **closed** and drop off +the base README's pending list. `Open` and `In Progress` are **pending**. + +## 5. Updating the base README + +`code-reviews/README.md` holds the single cross-library view (process overview, the Pending +Findings tables, and the Library Status table). It is **generated** from the per-library +`findings.md` files — do not edit it by hand. + +After any review or status change, regenerate it: + +``` +python3 code-reviews/regen-readme.py +``` + +`regen-readme.py --check` exits non-zero if `README.md` is stale, for use in CI. + +The per-library `findings.md` files are the source of truth; `README.md` is the aggregated +index and must always agree with them — which the script guarantees. + +## 6. Re-reviewing a library + +Re-reviews append to the same `findings.md`. Update the header to the new commit and date, +continue the finding numbering from the last used ID, and leave prior findings (including +closed ones) in place as history. diff --git a/code-reviews/Telemetry/findings.md b/code-reviews/Telemetry/findings.md new file mode 100644 index 0000000..366dea4 --- /dev/null +++ b/code-reviews/Telemetry/findings.md @@ -0,0 +1,36 @@ +# Code Review — Telemetry + +| Field | Value | +|-------|-------| +| Library | `ZB.MOM.WW.Telemetry/` | +| Packages | `ZB.MOM.WW.Telemetry`, `ZB.MOM.WW.Telemetry.Serilog` | +| Component spec | `components/observability/spec/SPEC.md` | +| Shared contract | `components/observability/shared-contract/ZB.MOM.WW.Telemetry.md` | +| Status | Not yet reviewed | +| Last reviewed | — | +| Reviewer | — | +| Commit reviewed | — | +| Open findings | 0 | + +## Summary + +_Not yet reviewed._ + +## Checklist coverage + +| # | Category | Examined | Notes | +|---|----------|----------|-------| +| 1 | Correctness & logic bugs | ☐ | | +| 2 | Public API surface & compatibility | ☐ | | +| 3 | Concurrency & thread safety | ☐ | | +| 4 | Error handling & resilience | ☐ | | +| 5 | Security & secret handling | ☐ | | +| 6 | Performance & resource management | ☐ | | +| 7 | Spec & shared-contract adherence | ☐ | | +| 8 | Packaging, dependencies & project layout | ☐ | | +| 9 | Testing coverage | ☐ | | +| 10 | Documentation & XML docs | ☐ | | + +## Findings + +_No findings recorded yet._ diff --git a/code-reviews/Theme/findings.md b/code-reviews/Theme/findings.md new file mode 100644 index 0000000..7764e40 --- /dev/null +++ b/code-reviews/Theme/findings.md @@ -0,0 +1,36 @@ +# Code Review — Theme + +| Field | Value | +|-------|-------| +| Library | `ZB.MOM.WW.Theme/` | +| Packages | `ZB.MOM.WW.Theme` (Razor Class Library) | +| Component spec | `components/ui-theme/spec/SPEC.md` | +| Shared contract | `components/ui-theme/shared-contract/ZB.MOM.WW.Theme.md` | +| Status | Not yet reviewed | +| Last reviewed | — | +| Reviewer | — | +| Commit reviewed | — | +| Open findings | 0 | + +## Summary + +_Not yet reviewed._ + +## Checklist coverage + +| # | Category | Examined | Notes | +|---|----------|----------|-------| +| 1 | Correctness & logic bugs | ☐ | | +| 2 | Public API surface & compatibility | ☐ | | +| 3 | Concurrency & thread safety | ☐ | | +| 4 | Error handling & resilience | ☐ | | +| 5 | Security & secret handling | ☐ | | +| 6 | Performance & resource management | ☐ | | +| 7 | Spec & shared-contract adherence | ☐ | | +| 8 | Packaging, dependencies & project layout | ☐ | | +| 9 | Testing coverage | ☐ | | +| 10 | Documentation & XML docs | ☐ | | + +## Findings + +_No findings recorded yet._ diff --git a/code-reviews/_template/findings.md b/code-reviews/_template/findings.md new file mode 100644 index 0000000..b77a42d --- /dev/null +++ b/code-reviews/_template/findings.md @@ -0,0 +1,71 @@ +# Code Review — + + + +| Field | Value | +|-------|-------| +| Library | `ZB.MOM.WW./` | +| Packages | `>` | +| Component spec | `components//spec/SPEC.md` | +| Shared contract | `components//shared-contract/ZB.MOM.WW..md` | +| Status | Not yet reviewed \| In progress \| Reviewed | +| Last reviewed | YYYY-MM-DD | +| Reviewer | | +| Commit reviewed | `` | +| Open findings | 0 | + +## Summary + +One short paragraph: overall health of the library, themes across findings, and anything +notable that is not a finding (e.g. test count, dependency footprint, adoption status). + +## Checklist coverage + +Confirm every category was examined. Record "No issues found" where applicable. + +| # | Category | Examined | Notes | +|---|----------|----------|-------| +| 1 | Correctness & logic bugs | ☐ | | +| 2 | Public API surface & compatibility | ☐ | | +| 3 | Concurrency & thread safety | ☐ | | +| 4 | Error handling & resilience | ☐ | | +| 5 | Security & secret handling | ☐ | | +| 6 | Performance & resource management | ☐ | | +| 7 | Spec & shared-contract adherence | ☐ | | +| 8 | Packaging, dependencies & project layout | ☐ | | +| 9 | Testing coverage | ☐ | | +| 10 | Documentation & XML docs | ☐ | | + +## Findings + + + +### -001 — + +| | | +|--|--| +| Severity | Critical \| High \| Medium \| Low | +| Category | | +| Status | Open \| In Progress \| Resolved \| Won't Fix \| Deferred | +| Location | `ZB.MOM.WW./src//.cs:` | + +**Description** + +What is wrong and why it matters. + +**Recommendation** + +Concrete suggested fix. + +**Resolution** + +_Unresolved._ + diff --git a/code-reviews/regen-readme.py b/code-reviews/regen-readme.py new file mode 100755 index 0000000..ffc06be --- /dev/null +++ b/code-reviews/regen-readme.py @@ -0,0 +1,198 @@ +#!/usr/bin/env python3 +"""Regenerate code-reviews/README.md from the per-library findings.md files. + +The findings files are the source of truth; README.md is a generated index. +Run this after recording, resolving, or re-triaging a finding so the aggregated +tables stay in sync (see REVIEW-PROCESS.md section 5). + +Usage: + python3 regen-readme.py # rewrite README.md + python3 regen-readme.py --check # exit 1 if README.md is stale (for CI) + +Works from any directory — paths are resolved relative to this script. +""" + +import os +import re +import sys + +BASE = os.path.dirname(os.path.abspath(__file__)) +SEVERITY_ORDER = {"Critical": 0, "High": 1, "Medium": 2, "Low": 3} +PENDING_STATUSES = {"Open", "In Progress"} +NOT_REVIEWED = "—" # rendered for libraries with no recorded review date/commit yet + + +def discover_libraries(): + """Library folders are every subdirectory of code-reviews/ holding a findings.md, + excluding the _template folder. Returned sorted for a stable README order.""" + libraries = [] + for name in sorted(os.listdir(BASE)): + if name.startswith(("_", ".")): + continue + if os.path.isfile(os.path.join(BASE, name, "findings.md")): + libraries.append(name) + return libraries + + +def parse_header(library, text): + """Extract (last_reviewed, commit) from the library's header table. + Returns None for either field when it is absent or still templated, so the + README can render a 'not yet reviewed' dash instead of a fake baseline.""" + last = re.search(r"\|\s*Last reviewed\s*\|\s*([0-9]{4}-[0-9]{2}-[0-9]{2})", text) + commit = re.search(r"\|\s*Commit reviewed\s*\|\s*`([^`<]+)`", text) + return ( + last.group(1) if last else None, + commit.group(1) if commit else None, + ) + + +def parse_findings(library): + """Parse one library's findings.md into ((last_reviewed, commit), [(library, id, severity, title, status), ...]).""" + text = open(os.path.join(BASE, library, "findings.md")).read() + header = parse_header(library, text) + findings = [] + for block in re.split(r"^### ", text, flags=re.M)[1:]: + head = block.splitlines()[0].strip() + m = re.match(r"([A-Za-z][A-Za-z0-9]*-\d+)\b(.*)", head) + if not m: + raise SystemExit(f"{library}/findings.md: unparseable finding heading: {head!r}") + fid = m.group(1).strip() + title = m.group(2).strip().lstrip("—–-").strip().replace("|", "\\|") + sev = re.search(r"\|\s*Severity\s*\|\s*([A-Za-z]+)", block) + status = re.search(r"\|\s*Status\s*\|\s*([A-Za-z' ]+?)\s*\|", block) + if not sev or not status: + raise SystemExit(f"{library}/findings.md: {fid} is missing a Severity or Status field") + findings.append((library, fid, sev.group(1), title, status.group(1).strip())) + return header, findings + + +def finding_number(finding): + return int(re.search(r"-(\d+)$", finding[1]).group(1)) + + +def build_readme(libraries, per_library): + pending = sorted( + (f for fs in per_library.values() for f in fs[1] if f[4] in PENDING_STATUSES), + key=lambda f: (SEVERITY_ORDER.get(f[2], 9), f[0], finding_number(f)), + ) + reviewed = sum(1 for lib in libraries if per_library[lib][0][0] is not None) + + def severity_total(sev): + return sum(1 for f in pending if f[2] == sev) + + def open_count(library, sev): + return sum(1 for f in per_library[library][1] + if f[2] == sev and f[4] in PENDING_STATUSES) + + lines = [] + add = lines.append + + add("# Code Reviews") + add("") + add("Comprehensive, per-library code reviews of the `ZB.MOM.WW.*` shared libraries hosted") + add("in this repo. Each library (one self-contained `.slnx` at the repo root) has its own") + add("folder containing a `findings.md`. This README is the aggregated index — the single") + add("place to see all outstanding work.") + add("") + add("> Generated by `regen-readme.py` from the per-library `findings.md` files. Do not") + add("> edit by hand — edit the findings files and re-run the script.") + add("") + add("## How it works") + add("") + add("- Reviews are performed one library at a time against a fixed checklist.") + add("- Each library is reviewed against its normalized component spec under `components/`.") + add("- Every finding is recorded in the library's `findings.md` with a severity and status.") + add("- Findings are **never deleted** — they are closed by changing their status, keeping") + add(" a full audit trail.") + add("- This README aggregates every **pending** finding (`Open` / `In Progress`) across all") + add(" libraries.") + add("") + add("See **[REVIEW-PROCESS.md](REVIEW-PROCESS.md)** for the full procedure: the review") + add("checklist, severity definitions, finding format, the library → component-spec mapping,") + add("and how to mark items resolved.") + add("") + add("## Layout") + add("") + add("```") + add("code-reviews/") + add("├── README.md # this file — process overview + pending findings") + add("├── REVIEW-PROCESS.md # how to perform a review and track findings") + add("├── regen-readme.py # regenerates this README from the findings files") + add("├── _template/findings.md # copy-this template for a library review") + add("└── /findings.md # one folder per ZB.MOM.WW.* shared library") + add("```") + add("") + add("## Summary") + add("") + add(f"{reviewed} of {len(libraries)} libraries reviewed. " + f"{len(pending)} pending finding{'s' if len(pending) != 1 else ''} across all libraries.") + add("") + add("| Severity | Open findings |") + add("|----------|---------------|") + for sev in ("Critical", "High", "Medium", "Low"): + add(f"| {sev} | {severity_total(sev)} |") + add(f"| **Total** | **{len(pending)}** |") + add("") + add("## Library Status") + add("") + add("| Library | Last reviewed | Commit | Open (C/H/M/L) | Open | Total |") + add("|---------|---------------|--------|----------------|------|-------|") + for library in libraries: + counts = [open_count(library, s) for s in ("Critical", "High", "Medium", "Low")] + last_reviewed, commit = per_library[library][0] + last_cell = last_reviewed if last_reviewed else NOT_REVIEWED + commit_cell = f"`{commit}`" if commit else NOT_REVIEWED + add(f"| [{library}]({library}/findings.md) | {last_cell} | {commit_cell} " + f"| {counts[0]}/{counts[1]}/{counts[2]}/{counts[3]} " + f"| {sum(counts)} | {len(per_library[library][1])} |") + add("") + add("## Pending Findings") + add("") + add("Every `Open` / `In Progress` finding across all libraries, highest severity first.") + add("Resolved findings drop off this list but remain recorded in their library's") + add("`findings.md` (see [REVIEW-PROCESS.md](REVIEW-PROCESS.md) §4–§5). Full detail —") + add("description, location, recommendation — lives in the library's `findings.md`.") + add("") + for sev in ("Critical", "High", "Medium", "Low"): + rows = [f for f in pending if f[2] == sev] + add(f"### {sev} ({len(rows)})") + add("") + if not rows: + add("_None open._") + add("") + continue + add("| ID | Library | Title |") + add("|----|---------|-------|") + for library, fid, _, title, _ in rows: + add(f"| {fid} | [{library}]({library}/findings.md) | {title} |") + add("") + + return "\n".join(lines) + + +def main(): + check = "--check" in sys.argv[1:] + libraries = discover_libraries() + per_library = {m: parse_findings(m) for m in libraries} + content = build_readme(libraries, per_library) + + readme_path = os.path.join(BASE, "README.md") + pending = sum(1 for fs in per_library.values() + for f in fs[1] if f[4] in PENDING_STATUSES) + total = sum(len(fs[1]) for fs in per_library.values()) + + if check: + current = open(readme_path).read() if os.path.exists(readme_path) else "" + if current != content: + print("README.md is stale — run: python3 code-reviews/regen-readme.py") + sys.exit(1) + print(f"README.md is up to date ({pending} pending / {total} total).") + return + + open(readme_path, "w").write(content) + print(f"README.md regenerated — {pending} pending, {total} total findings " + f"across {len(libraries)} libraries.") + + +if __name__ == "__main__": + main()