P-013: ship the CI/CLI distribution surface (Action + dotnet tool)#10
Merged
Conversation
Make the P-001 pipeline (C# -> extractor -> facts -> core) something people
can actually run on their own code, in the form that fits "one checker":
a CI/CLI surface where Python already lives, not a native Roslyn analyzer
(which would force a second checker in C# or per-keystroke shelling to Python).
Core (ownlang):
- `ownir --format {human,github,msbuild}`: the finding renderer lives in the
core, so the wrappers stay thin. github = PR annotations, msbuild = VS Error
List, human = unchanged default. Machine formats keep stdout to annotations
only (summary to stderr); GitHub metacharacters escaped.
Extractor (Roslyn):
- walk directories recursively for *.cs (skip bin/obj/.git/node_modules/
packages + generated files), so it can point at a whole repo;
- report finding paths relative to cwd (forward slashes) so annotations land on
the right file when names collide;
- PackAsTool: `dotnet tool install --global OwnSharp.Extractor`.
Orchestration:
- scripts/own-check.sh chains both stages (--format, --fail-on-finding);
- action.yml composite Action wraps it; examples/ci/own-check.yml for consumers.
Tests/CI:
- test_ownir: render-format + GitHub-escaping unit checks (24/24);
- new own-check-surface CI job: repo scan in github + msbuild formats,
--fail-on-finding exit code, and the composite action end-to-end.
Docs: P-013 records the decision and the three-surface cost ladder; frontend
README gains a real-repo/CI usage section; ROADMAP index updated.
https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
The PackAsTool comment in the csproj contained '--global', and '--' is illegal inside an XML comment, so MSBuild refused to load the project (every extractor job failed at build). Reword the comment without '--'. https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
…test The own-check-surface self-test aborted with exit 134 (an unhandled .NET exception), and the cause was invisible because the step piped the extractor's stderr to /dev/null. Fix robustness + visibility: - extractor: skip an input that is not a readable file instead of letting File.ReadAllText throw; walk directories with EnumerationOptions (IgnoreInaccessible) so a locked subdir cannot abort the whole scan; - own-check.sh: drop the unproven 'dotnet run --nologo' flag; - CI: stop redirecting the script's stderr to /dev/null, so build chatter and any error land in the job log (stdout still carries only annotations). https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a CI/CLI distribution surface (P-013) for Own.NET: the Python ChangesP-013 Distribution Surface
Sequence Diagram(s)sequenceDiagram
actor Consumer as Consumer CI Job
participant action as action.yml (composite)
participant script as own-check.sh
participant extractor as dotnet OwnSharp.Extractor
participant checker as python -m ownlang ownir
Consumer->>action: uses: PhysShell/own.net@main<br/>path, format, fail-on-finding
action->>action: setup-python@v5, setup-dotnet@v4
action->>script: own-check.sh --root --format [--fail-on-finding] path
script->>extractor: dotnet run -o facts.tmp path(s)
extractor-->>script: facts.tmp (OwnIR JSON)
script->>checker: python -m ownlang ownir --format facts.tmp
checker-->>script: exit 0 (clean) / 1 (findings) / 2+ (error)
script-->>action: exit 0 or propagated finding exit code
action-->>Consumer: annotations in github/msbuild format
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@action.yml`:
- Around line 49-53: The shell command in the run block directly interpolates
action inputs without proper quoting, creating a shell injection vulnerability.
Fix this by adding an env section that passes all user inputs (inputs.format and
inputs.path) as environment variables, then modify the run block to reference
these environment variables with proper quoting instead of direct template
interpolation. Specifically, replace the unquoted ${{ inputs.path }} on line 53
and the ${{ inputs.format }} reference on line 50 with references to
safely-defined environment variables, ensuring all variable expansions in the
shell command are wrapped in quotes to prevent command execution.
In `@frontend/roslyn/OwnSharp.Extractor/Program.cs`:
- Around line 133-142: The defensive check only validates file existence at line
136, but the File.ReadAllText call at line 141 can still throw exceptions for
locked or unauthorized files, causing the entire scan to abort. Wrap the
File.ReadAllText invocation in a try-catch block to handle any exceptions that
may occur during file access (such as IOException or
UnauthorizedAccessException), logging an appropriate error message via
Console.Error.WriteLine and using continue to skip that file and proceed with
the next one, consistent with the defensive intent expressed in the comment
above the existence check.
In `@ownlang/__main__.py`:
- Around line 228-248: The argument parsing loop in the while block (iterating
through rest) allows bare `--format` flags without values to be appended as
positional arguments, and extra positional arguments beyond the first are
silently ignored, both hiding CLI usage errors. After the while loop ends and
before using positional[0], add validation to reject a bare `--format` flag if
it appears in the positional list (which indicates missing its value), and
reject if there are more than one positional arguments provided. In both error
cases, print a descriptive message to sys.stderr and return exit code 2,
consistent with the existing validation pattern for unknown formats and format
misapplication.
In `@scripts/own-check.sh`:
- Around line 30-34: The `--root` and `--format` option handlers in the case
statement read `$2` without validating that the argument value exists. When
these options are invoked without a following value and `set -u` is enabled, the
script crashes with an unhelpful shell error instead of providing clear
feedback. Add validation checks before accessing `$2` for both the `--root` and
`--format` cases to ensure the argument exists, and output a descriptive error
message with usage information before exiting if a required value is missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d6547ddb-f1f3-4d00-9212-4af8c0d0ce35
📒 Files selected for processing (12)
.github/workflows/ci.ymlaction.ymldocs/ROADMAP.mddocs/proposals/P-013-distribution-surface.mdexamples/ci/own-check.ymlfrontend/roslyn/OwnSharp.Extractor/OwnSharp.Extractor.csprojfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/README.mdownlang/__main__.pyownlang/ownir.pyscripts/own-check.shtests/test_ownir.py
- action.yml: pass user inputs (path/format/fail-on-finding) via env vars instead of template-interpolating them into the run block — closes a shell injection vector where a crafted path would be expanded as code (Major); - extractor: wrap File.ReadAllText in try/catch (IOException / UnauthorizedAccessException) so a locked/unreadable file is skipped, not an abort — the File.Exists gate alone did not cover it (Major); - ownlang CLI: reject a bare --format (missing value) and more than one positional arg with a clear error/usage instead of a misleading file error; - own-check.sh: validate --root/--format have a value before reading $2, so a malformed call under set -u returns a usage error, not a shell crash. Verified locally: ownir 24/24, ruff + mypy --strict clean, CLI guards return 2. https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
This was referenced Jun 16, 2026
This was referenced Jun 23, 2026
This was referenced Jul 11, 2026
PhysShell
pushed a commit
that referenced
this pull request
Jul 12, 2026
From independent review of PR #245 (Codex + CodeRabbit both caught the permissions gap independently; each caught one issue the other missed): - consumer-simulation's `uses: PhysShell/Own.NET@${{ github.sha }}` cannot work at all: GitHub Actions does not evaluate expressions in `jobs.<job_id>.steps.uses` (not among the expression-capable fields the context-availability docs list, unlike with/env/if/run) - the job would never have resolved an action. There is no way to parameterize `uses:` with "the commit under test"; a genuinely dynamic remote-ref proof isn't automatable pre-tag. Switched to `uses: ./` (the same mechanism ci.yml's own-check-codescan dog-food job already relies on, correctly) - full checkout instead of a sparse one, since `./` needs the whole action definition present. docs/notes/action-marketplace-readiness.md and the fixture's own README corrected to match; the honest-limitation framing now matches what's actually achievable pre-tag. - consumer-simulation's upload-sarif step needs security-events:write (required for every workflow that calls it, not just push events); the job only granted contents:read. Added the permission, and guarded the SARIF/upload steps with the same fork-PR check own-check-codescan already uses (a fork PR's GITHUB_TOKEN is read-only, so the upload would 403 through no fault of an external contributor) - the leak/clean checks still run for forks, only the SARIF steps are skipped there. - move-major-tag's shell steps interpolated `${{ inputs.move_major_tag_to }}` and step outputs directly into `run:` bodies - a template-injection risk (flagged by zizmor via CodeRabbit), the same class of bug action.yml's own CodeRabbit #10 fix already guards against elsewhere in this repo. Moved both through `env:` and added a SemVer-shape validation as a second, independent guard.
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make the P-001 pipeline (C# -> extractor -> facts -> core) something people
can actually run on their own code, in the form that fits "one checker":
a CI/CLI surface where Python already lives, not a native Roslyn analyzer
(which would force a second checker in C# or per-keystroke shelling to Python).
Core (ownlang):
ownir --format {human,github,msbuild}: the finding renderer lives in thecore, so the wrappers stay thin. github = PR annotations, msbuild = VS Error
List, human = unchanged default. Machine formats keep stdout to annotations
only (summary to stderr); GitHub metacharacters escaped.
Extractor (Roslyn):
packages + generated files), so it can point at a whole repo;
the right file when names collide;
dotnet tool install --global OwnSharp.Extractor.Orchestration:
Tests/CI:
--fail-on-finding exit code, and the composite action end-to-end.
Docs: P-013 records the decision and the three-surface cost ladder; frontend
README gains a real-repo/CI usage section; ROADMAP index updated.
https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
Summary by CodeRabbit
Release Notes
New Features
ownirrendering to support GitHub annotations and MSBuild/VS error output in addition to human-readable results.Documentation
Tests