feat(action): Marketplace-readiness pipeline for the composite action#245
Closed
PhysShell wants to merge 2 commits into
Closed
feat(action): Marketplace-readiness pipeline for the composite action#245PhysShell wants to merge 2 commits into
PhysShell wants to merge 2 commits into
Conversation
Prepares action.yml for eventual GitHub Marketplace publication without
publishing a listing. No analyzer logic changed - action.yml, a new
workflow, a fixture, and doc/README accuracy only.
- Pinned action.yml's two nested `uses:` steps (actions/setup-python,
actions/setup-dotnet) to commit SHAs, matching this repo's own
stated convention (every third-party `uses:` pinned) that these two
had drifted from. SHAs verified via `git ls-remote --tags`, not
copied on faith.
- Validated branding/metadata against GitHub's actual Marketplace
constraints: icon is a real Feather icon, color is in the fixed
8-color allowlist - both already compliant.
- New .github/workflows/action-marketplace-readiness.yml:
- consumer-simulation: references the action the way an external
repo actually would (`uses: PhysShell/Own.NET@<sha>`, resolved by
the Actions runner's action-fetch mechanism, not `uses: ./`)
against a sparse checkout containing only a small dedicated
fixture - no other Own.NET source on disk in that job. Verifies
the leak sample fails the step, the clean sample doesn't, and the
SARIF output is accepted by github/codeql-action/upload-sarif.
- Versioning policy: immutable `vX.Y.Z` release tags (distinct
namespace from the CLI's `ownsharp-cli-v*`), with the moving major
tag (v0/v1) only ever repointed through a separate, explicit
workflow_dispatch gated behind a protected GitHub Environment -
never as a side effect of a patch-tag push. Both event_name and
ref-prefix are checked on every tag-gated job from the start (the
same class of gap independent review caught on the CLI's publish
gate in PR #244).
- fixtures/marketplace-consumer-demo/: a minimal, clearly-separate
leak+clean pair standing in for "an external consumer's repo" -
distinct from frontend/roslyn/samples (the extractor's own
precision-test corpus). Documented honestly as an approximation. A
genuinely separate GitHub repo would be stronger proof but wasn't
created without explicit authorization (a visible, user-facing
action this session doesn't take unilaterally).
- README.md/README.ru.md: fixed `PhysShell/own.net` casing drift
(-> `PhysShell/Own.NET`), added a pointer to the versioning policy
for what to switch to once a release exists.
- docs/notes/action-marketplace-readiness.md: the audit findings, the
versioning policy in full, the consumer-fixture rationale and its
honest limitation, and the Marketplace publish checklist for
whoever runs an actual release.
ci.yml's existing own-check-codescan job (uses: ./ dog-food on every
push/PR) is untouched - this workflow is the release-readiness path
alongside it, not a replacement.
Boundaries: not published to Marketplace; no analyzer logic touched;
no continue-on-error without an immediate, documented assertion on the
outcome.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Marketplace readiness validation for the composite action, including consumer checks, SARIF upload, release-tag validation, gated major-tag movement, pinned dependencies, consumer fixtures, and updated release guidance. ChangesMarketplace readiness
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant OwnNETAction
participant ConsumerFixture
participant CodeScanning
GitHubActions->>ConsumerFixture: check out marketplace fixture
GitHubActions->>OwnNETAction: invoke local composite action
OwnNETAction->>ConsumerFixture: analyze Leaky.cs and Clean.cs
OwnNETAction-->>GitHubActions: return outcomes and SARIF file
GitHubActions->>CodeScanning: upload SARIF results
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d08063c414
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/action-marketplace-readiness.yml:
- Around line 51-52: Update the permissions block for the consumer-simulation
job to include security-events: write alongside contents: read, so its
upload-sarif step can publish code-scanning results while preserving the
existing permission settings.
- Around line 168-182: Update the “Resolve and validate the target release tag”
and “Force-move the major tag” steps to receive move_major_tag_to through step
environment variables instead of direct GitHub expression interpolation in shell
commands. Validate the target against the expected SemVer release-tag format
before using it, while preserving the existing tag existence check and major-tag
movement behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9eb4731d-7595-4c33-b7b8-c703eee4860c
📒 Files selected for processing (8)
.github/workflows/action-marketplace-readiness.ymlREADME.mdREADME.ru.mdaction.ymldocs/notes/action-marketplace-readiness.mdfixtures/marketplace-consumer-demo/Clean.csfixtures/marketplace-consumer-demo/Leaky.csfixtures/marketplace-consumer-demo/README.md
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.
Owner
Author
|
Three issues fixed in 5106bad — Codex and CodeRabbit each independently caught the
Thanks both — #1 in particular meant the job would never have run at all, which local YAML parsing obviously couldn't catch. Generated by Claude Code |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| { | ||
| public void Run() | ||
| { | ||
| var buffer = new MemoryStream(); |
8 tasks
Owner
Author
|
Pausing this PR per direction: a public-facade rebrand (product "Owen", Action display name "Owen lifetime/resource check") needs to land first. See #246. Once #246 merges, this PR will be rebuilt against it — retargeting Generated by Claude Code |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Merged
8 tasks
Owner
Author
|
Superseded by #248 — rebuilt against the merged Owen public facade (#246), and corrects a stale claim in this PR's own description (the Generated by Claude Code |
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
3 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.
Что и зачем
Phase 4 of a repository-state cleanup: prepares the composite GitHub Action (
action.yml) for eventual Marketplace publication without publishing a listing (per the explicit boundary).Metadata/branding audit —
name/description/author/brandingwere already present and Marketplace-compliant (icon: shieldis a real Feather icon;color: purpleis in the fixed 8-color allowlist). Fixed: the two nesteduses:steps (actions/setup-python@v5,actions/setup-dotnet@v4) were floating tags, inconsistent with this repo's own stated convention (ci.yml: "every third-partyuses:is pinned to a commit SHA"). Pinned to the exact SHAs already used elsewhere in this repo, and independently re-verified against upstream viagit ls-remote --tagsrather than trusted on faith.Versioning policy — bare
vX.Y.Zrelease tags (distinct namespace from the CLI'sownsharp-cli-v*established in #244, avoiding ambiguity about which artifact a tag names), immutable once pushed. The moving major tag (v0/v1) only ever repoints through a separate, explicitworkflow_dispatchgated behind a protectedaction-major-tag-moveGitHub Environment — never as a side effect of a patch tag push. Every tag-gated job checks bothgithub.event_name == 'push'and the ref prefix from the start — the same class of gap independent review caught on PR #244's publish gate (workflow_dispatch --ref <tag>can satisfy a barestartsWith(github.ref, ...)check).Consumer-simulation fixture —
fixtures/marketplace-consumer-demo/(a minimal leak+clean pair, deliberately separate fromfrontend/roslyn/samples, the extractor's own precision-test corpus). The new workflow'sconsumer-simulationjob references the action the way an external repo actually would —uses: PhysShell/Own.NET@${{ github.sha }}, resolved through the Actions runner's real action-fetch mechanism, notuses: ./— against a sparse checkout containing only the fixture, so the job's own workspace has no other Own.NET source on disk either. Verifies the leak sample fails the step, the clean sample doesn't, and the SARIF output is accepted bygithub/codeql-action/upload-sarif. Documented honestly: a genuinely separate GitHub repo would be stronger proof but wasn't created — that's a visible action outside this session's call to make unilaterally.README accuracy — fixed a
PhysShell/own.net→PhysShell/Own.NETcasing drift in bothREADME.mdandREADME.ru.md, and added a pointer to the new versioning policy for post-release usage.Full findings, the versioning policy, and a Marketplace publish checklist for whoever runs an actual release are in
docs/notes/action-marketplace-readiness.md.ci.yml's existingown-check-codescanjob (dog-fooding viauses: ./on every push/PR) is untouched — this workflow is the release-readiness path alongside it, not a replacement.Тип изменения
Как проверено
python tests/run_tests.py— 276/276ruff check ./mypy— cleanaction.yml,action-marketplace-readiness.yml) parsed withpython3 -c "import yaml; yaml.safe_load(...)"— caught and fixed one real syntax bug (an unquotedname:containing a literaluses:colon, which YAML misparsed as a nested mapping key)actions/setup-python@v5andactions/setup-dotnet@v4SHA pins verified againstgit ls-remote --tags(already used elsewhere in this repo, re-verified independently)scripts/own-check.shonLeaky.cs→ exactly oneOWN001at the expected line; onClean.cs→ zero findingsСвязанные issue
Refs P-013 gate B, the "Action distribution" open question in
docs/proposals/P-013-distribution-surface.md. No issues closed (readiness infra, not a shipped release).Чеклист
consumer-simulationjob is the test surface; the fixture was hand-verified locally firstREADME.md/README.ru.mdcasing + pointer fix;docs/notes/action-marketplace-readiness.mdis newfeat:)Boundaries honored
action.ymlstep pins, a new workflow, a fixture, and doc/README accuracy only.continue-on-error: the one use (consumer-simulation's leak-sample step) is immediately followed by an explicit assertion onsteps.leak.outcome, documented inline as intentional (observing a specific expected failure, not swallowing an unexpected one).Generated by Claude Code
Summary by CodeRabbit