-
Notifications
You must be signed in to change notification settings - Fork 0
P-013: ship the CI/CLI distribution surface (Action + dotnet tool) #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
cafc721
P-013: ship the CI/CLI distribution surface (Action + dotnet tool)
claude 10adf99
extractor: fix MSB4025 — XML comment cannot contain '--'
claude 36b8f5c
extractor/ci: harden the directory walk + stop muting errors in self-…
claude 4011549
address CodeRabbit review on PR #10 (4 robustness/security fixes)
claude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| name: "Own.NET resource-leak check" | ||
| description: >- | ||
| Scan C# for lifetime/resource leaks the compiler cannot express — event/timer | ||
| subscription leaks, undisposed IDisposable fields/locals, ignored Subscribe() | ||
| tokens, ArrayPool buffers rented-but-never-returned — and annotate the PR. | ||
| author: "Own.NET" | ||
| branding: | ||
| icon: "shield" | ||
| color: "purple" | ||
|
|
||
| inputs: | ||
| path: | ||
| description: "File(s) or directory to scan (directories are walked for *.cs)." | ||
| required: false | ||
| default: "." | ||
| format: | ||
| description: "Finding surface: github (PR annotations), msbuild, or human." | ||
| required: false | ||
| default: "github" | ||
| fail-on-finding: | ||
| description: "Fail the step when any leak is found." | ||
| required: false | ||
| default: "true" | ||
| python-version: | ||
| description: "Python version for the Own.NET core." | ||
| required: false | ||
| default: "3.13" | ||
| dotnet-version: | ||
| description: "The .NET SDK version for the Roslyn extractor." | ||
| required: false | ||
| default: "8.0.x" | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Set up Python (Own.NET core) | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ${{ inputs.python-version }} | ||
|
|
||
| - name: Set up .NET (Roslyn extractor) | ||
| uses: actions/setup-dotnet@v4 | ||
| with: | ||
| dotnet-version: ${{ inputs.dotnet-version }} | ||
|
|
||
| - name: Own.NET leak check | ||
| shell: bash | ||
| env: | ||
| # Pass user-controlled inputs through the environment (data), not by | ||
| # template interpolation into the script body (code) — otherwise a path | ||
| # like `.; rm -rf x` would be expanded into the shell before bash parses | ||
| # it. CodeRabbit #10. | ||
| OWN_PATH: ${{ inputs.path }} | ||
| OWN_FORMAT: ${{ inputs.format }} | ||
| OWN_FAIL_ON_FINDING: ${{ inputs.fail-on-finding }} | ||
| run: | | ||
| args=(--root "${{ github.action_path }}" --format "$OWN_FORMAT") | ||
| if [ "$OWN_FAIL_ON_FINDING" = "true" ]; then | ||
| args+=(--fail-on-finding) | ||
| fi | ||
| "${{ github.action_path }}/scripts/own-check.sh" "${args[@]}" -- "$OWN_PATH" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| # P-013 — Distribution surface: how people actually run Own.NET on C# | ||
|
|
||
| - **Status:** v0 built (CI/Action + dotnet tool) | ||
| - **Depends on:** P-001 (the C# → OwnIR extractor) and the core CLI | ||
| (`python -m ownlang ownir`). Sibling of **P-011** (editor tooling for the | ||
| `.own` DSL — a *different* direction; see "Not the same as P-011" below). | ||
| Feeds **P-012** (the mining pipeline reuses the same repo-scan). | ||
|
|
||
| ## Motivation | ||
|
|
||
| The pipeline `*.cs → extractor → facts.json → core → finding @ C# line` has | ||
| worked end-to-end in CI since P-001, but only against a hardcoded list of sample | ||
| files. Nobody outside this repo can *run* it. The question "how will people use | ||
| this?" has three candidate answers, and they cost wildly different amounts — so | ||
| the first job is to pick the one that fits the architecture, not the one that | ||
| sounds most impressive. | ||
|
|
||
| The load-bearing constraint is the ROADMAP's **"one checker"**: the Python core | ||
| is the single source of truth, and every frontend only *produces or consumes* | ||
| OwnIR facts. That rule decides the surface for us. | ||
|
|
||
| ## The three surfaces (cost order) | ||
|
|
||
| | Surface | What the user sees | Cost | Fits "one checker"? | | ||
| | --- | --- | --- | --- | | ||
| | **CI / CLI gate** | a red check + PR annotations | low (≈ done in CI) | ✅ ideal — Python already runs here | | ||
| | **MSBuild diagnostics → VS Error List** | findings in VS, no extension | low–medium | ✅ text in a parseable format | | ||
| | **Native Roslyn `DiagnosticAnalyzer`** | live squiggles in the IDE | high | ❌ conflicts (see below) | | ||
|
|
||
| A native analyzer runs **in-process** inside `dotnet build` / the IDE. It would | ||
| have to either (a) reimplement the analysis in C# — a *second checker* that | ||
| drifts, the project's own meta-irony — or (b) shell out to Python on every | ||
| keystroke, which is slow, fragile, and needs a Python runtime on every dev | ||
| machine. So the obstacle to the IDE-native path is **architectural, not effort**. | ||
| The CI/CLI surface, by contrast, is where Python already lives and where "one | ||
| checker" is free. | ||
|
|
||
| Decision: **ship the CI/CLI surface first**, expose the same findings in the | ||
| MSBuild format so they *also* light up the VS Error List without an analyzer, | ||
| and defer the native analyzer until (if ever) the core itself moves to .NET. | ||
|
|
||
| ## Scope (v0 — built) | ||
|
|
||
| - **`python -m ownlang ownir facts.json --format {human,github,msbuild}`.** The | ||
| finding renderer lives in the core (`ownlang/ownir.py`), so the wrappers stay | ||
| thin and there is exactly one place that decides what a finding says: | ||
| - `human` — the existing CLI line (unchanged; the default). | ||
| - `github` — a `::error file=…,line=…,title=OWN001::…` workflow command; | ||
| GitHub renders it inline on the PR diff. Metacharacters are escaped. | ||
| - `msbuild` — `file(line): error OWN001: …`, which `dotnet build` and the VS | ||
| Error List parse — in-IDE findings with no extension. | ||
| - **Repo-walk in the extractor.** `ownsharp-extract <dir>` now recurses for | ||
| `*.cs`, skipping `bin`/`obj`/`.git`/`node_modules`/`packages` and generated | ||
| files (`*.g.cs`, `*.Designer.cs`, `*.AssemblyInfo.cs`). Finding paths are | ||
| reported relative to the working directory (forward slashes) so annotations | ||
| point at the right file even when names collide. | ||
| - **`scripts/own-check.sh`.** One command that chains both stages (extractor → | ||
| core) with `--format` and `--fail-on-finding`. The body of the Action and a | ||
| standalone local command. | ||
| - **`action.yml`** — a composite GitHub Action (`uses: PhysShell/own.net@…`): | ||
| sets up Python + .NET, runs `own-check.sh` over the consumer's checkout, | ||
| annotates the PR. Example consumer workflow in `examples/ci/own-check.yml`. | ||
| - **`dotnet tool`** — the extractor csproj is `PackAsTool` | ||
| (`dotnet tool install --global OwnSharp.Extractor` → `ownsharp-extract`). | ||
| Honest caveat: the tool is only the C# *extractor*; the verdict is still the | ||
| Python core. The script/Action are the complete product. | ||
|
|
||
| ## Not the same as P-011 | ||
|
|
||
| P-011 makes the **`.own` DSL** a first-class editor language (coloring, | ||
| squiggles for `.own` diagnostics) — input *to* the checker. P-013 is the | ||
| opposite direction: feed **C#** in, get findings out. Easy to conflate; they do | ||
| not overlap. | ||
|
|
||
| ## Non-goals | ||
|
|
||
| - **A native Roslyn analyzer in v0** — deferred for the architectural reason | ||
| above, not the effort. Revisit only if the core moves to .NET. | ||
| - **Wiring a 100-repo scan as a blocking gate** — that is P-012's offline job, | ||
| not this per-repo check. | ||
| - **A second checker anywhere.** The wrappers never decide a verdict. | ||
| - Publishing to the GitHub Marketplace / NuGet.org, SHA-pinning, signed releases | ||
| — packaging hardening, deferred to a release pass. | ||
|
|
||
| ## Open questions | ||
|
|
||
| 1. MSBuild severity: emit findings as `error` (fails a parsing build) or | ||
| `warning` (advisory)? v0 uses `error` to match the core; the Action's | ||
| `fail-on-finding` already gates CI independently. | ||
| 2. Should `own-check` grow a `--baseline`/diff mode (only new findings on a PR) | ||
| so adopting it on a legacy repo isn't an immediate wall of red? | ||
| 3. Action distribution: a moving `@main`, or tagged releases (`@v0.1.0`) + | ||
| Marketplace listing? (Ties into the packaging-hardening pass.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # Example consumer workflow — copy into a .NET repo's .github/workflows/. | ||
| # | ||
| # Drops the Own.NET resource-leak check onto every pull request: the Roslyn | ||
| # extractor scans the repo's C#, the Python core produces the verdict, and any | ||
| # leak shows up as an inline annotation on the PR diff (and fails the check). | ||
| # | ||
| # Pin the action to a released tag (e.g. @v0.1.0) rather than @main for stability. | ||
|
|
||
| name: Own.NET leak check | ||
|
|
||
| on: | ||
| pull_request: | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: read # the check only reads the code | ||
|
|
||
| jobs: | ||
| own-check: | ||
| name: resource-leak check | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: PhysShell/own.net@main | ||
| with: | ||
| path: . | ||
| format: github | ||
| fail-on-finding: "true" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.