feat(action): Marketplace-readiness pipeline, rebuilt against the Owen facade (P-013 gate B)#248
Conversation
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ 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 |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
| { | ||
| public void Run() | ||
| { | ||
| var buffer = new MemoryStream(); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e755a84b5
ℹ️ 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".
Two independent review findings, both closed: 1. owen-cli-release.yml only triggered on tag push / workflow_dispatch, so a green PR check never actually ran build-test-pack -> smoke-test on either OS -- only a real tag push did, and PRs never push tags. Added a pull_request trigger scoped to the paths that matter (the CLI/extractor projects, ownlang, the workflow file itself, the shared protection script). The publish job's existing tag-push condition is unchanged, so it stays skipped on pull_request exactly as it already was on workflow_dispatch. 2. The environment-protection check (`.protection_rules | length`) accepted ANY protection rule -- a wait_timer-only or branch_policy-only environment, or a required_reviewers rule saved with zero reviewers, none of which actually gate on a human. Extracted the real predicate into scripts/check_environment_protection.sh: accepts only a required_reviewers rule with >=1 reviewer. The publish job now checks out just scripts/ (sparse), fetches the environment via `gh api` into a file, and calls the script. Its permissions gained `actions: read`, which the environment-read endpoint requires alongside `contents: read`. Fixture-driven, offline test in ci.yml's new environment-protection-selftest job (runs on every ordinary push/PR, no GitHub API or real Environment needed): zero rules, wait_timer-only, branch_policy-only, and required_reviewers-with-zero-reviewers all reject; required_reviewers with one reviewer accepts. The same script and fixtures will also back action-marketplace-readiness.yml's analogous move-major-tag check (PR #248, stacked on this branch for now -- duplication is temporary until #247 merges and #248 rebases cleanly onto main). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw
|
All three requested fixes for this PR are in (
Note on branch structure: this branch is currently stacked on Generated by Claude Code |
|
@coderabbitai review |
…facade (P-013) * feat(owen-cli): release-readiness pipeline, rebuilt against the Owen facade (P-013) Rebuild of PR #244's release-prep work directly against post-#246 main, using the Owen public identity throughout instead of the pre-rebrand ownsharp/OwnSharp.Cli names PR #244 used (per review: do not merge the old-brand version, rebuild it against the merged facade). Package metadata (OwnSharp.Cli.csproj) — added Authors, PackageProjectUrl, RepositoryUrl, RepositoryType, PackageTags, PackageReadmeFile, and deterministic-pack properties (Deterministic, ContinuousIntegrationBuild) on top of #246's already-Owen-branded PackageId/ToolCommandName/Description -- these are additive metadata, not a rebrand of what #246 already set. PackageLicenseExpression is deliberately left unset: the repository has no LICENSE file, and choosing one is the repository owner's call, not this session's -- flagged as the first blocker in the release checklist, not resolved unilaterally. New release workflow (.github/workflows/owen-cli-release.yml, replacing PR #244's ownsharp-cli-release.yml under the Owen tag namespace owen-cli-v*): build-test-pack -> smoke-test (ubuntu + windows) -> publish. The smoke-test job installs and exercises the already-packed Owen.Cli artifact from build-test-pack's upload -- no checkout, no rebuild, no invocation of the pre-rebrand `ownsharp` command -- verifying `owen --version`, a leak sample (exit 1, OWN001), a clean sample (exit 0), the OWEN_PYTHON-not-found actionable-failure path (exit 3), and a full uninstall/reinstall cycle. Publish still requires both a pushed owen-cli-v* tag (workflow_dispatch can never reach it) and a `nuget-release` GitHub Environment a repo admin has to configure with required reviewers -- it doesn't exist yet. Internal filenames (ownsharp.dll, ownsharp-extract.dll -- the real, unmodified AssemblyName-derived output, per #246's established internal/ public split) are intentionally left as-is in the package-content inspection step; only the public package ID, command name, tag namespace, and env var are Owen-branded. docs/notes/owen-cli-release.md (replacing PR #244's ownsharp-cli-release.md) documents the versioning policy, the verified determinism findings (re-verified against this exact source: two packs, byte-for-byte identical payload, only NuGet's own per-pack OPC wrapper GUID differs), the metadata audit, and the release checklist -- including the license blocker addressed explicitly to the repository owner. Every step in the new workflow was manually replicated and verified locally before being encoded: dotnet pack (twice, for determinism) -> diff unzipped payload -> package-content inspection -> isolated-feed install -> owen --version / leak / clean / OWEN_PYTHON-not-found / uninstall-reinstall, all matching what the workflow asserts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw * owen-cli-release: fail loudly if nuget-release lacks protection rules Codex review (P1): GitHub auto-creates a referenced-but-never-configured environment on first use, with zero protection rules. `environment: nuget-release` alone is therefore not proof a human ever approves this job -- if the environment is never actually set up under Settings -> Environments and NUGET_API_KEY already exists as a repository secret, a tag push could reach `dotnet nuget push` with no approval at all, silently defeating the safety story the workflow documents. The publish job's first step now queries the GitHub API for the environment's protection_rules and refuses to publish -- fails loudly, before the artifact is even downloaded -- when that array is empty, converting "never configured" from a silent bypass into a loud failure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw * owen-cli-release: exercise the workflow on PRs, require actual reviewers Two independent review findings, both closed: 1. owen-cli-release.yml only triggered on tag push / workflow_dispatch, so a green PR check never actually ran build-test-pack -> smoke-test on either OS -- only a real tag push did, and PRs never push tags. Added a pull_request trigger scoped to the paths that matter (the CLI/extractor projects, ownlang, the workflow file itself, the shared protection script). The publish job's existing tag-push condition is unchanged, so it stays skipped on pull_request exactly as it already was on workflow_dispatch. 2. The environment-protection check (`.protection_rules | length`) accepted ANY protection rule -- a wait_timer-only or branch_policy-only environment, or a required_reviewers rule saved with zero reviewers, none of which actually gate on a human. Extracted the real predicate into scripts/check_environment_protection.sh: accepts only a required_reviewers rule with >=1 reviewer. The publish job now checks out just scripts/ (sparse), fetches the environment via `gh api` into a file, and calls the script. Its permissions gained `actions: read`, which the environment-read endpoint requires alongside `contents: read`. Fixture-driven, offline test in ci.yml's new environment-protection-selftest job (runs on every ordinary push/PR, no GitHub API or real Environment needed): zero rules, wait_timer-only, branch_policy-only, and required_reviewers-with-zero-reviewers all reject; required_reviewers with one reviewer accepts. The same script and fixtures will also back action-marketplace-readiness.yml's analogous move-major-tag check (PR #248, stacked on this branch for now -- duplication is temporary until #247 merges and #248 rebases cleanly onto main). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw * owen-cli-release: fix doc drift, close two script-injection nitpicks CodeRabbit review: - docs/notes/owen-cli-release.md quoted the publish gate as just `startsWith(github.ref, 'refs/tags/owen-cli-v')`, dropping the `github.event_name == 'push'` half the workflow's own comment explains is required to block a `workflow_dispatch --ref owen-cli-vX` bypass -- the exact scenario the doc's next sentence claimed was already impossible. Quoted the real condition in full. - `${{ steps.version.outputs.version }}` (grep'd from the csproj, which this workflow now also builds on pull_request) and `${{ needs.build-test-pack.outputs.version }}` were both inlined directly into run: scripts at five sites -- the same expression-into-shell-text pattern zizmor flags elsewhere in this repo. Both now flow through env: (a job-level OWEN_CLI_VERSION for smoke-test's four sites, a step-level CSPROJ_VERSION for the tag/version-match assertion) instead of raw interpolation. Not implemented: CodeRabbit's NuGet Trusted Publishing (OIDC) suggestion for the publish step. Real publication is already blocked on the license and Environment decisions the docs already flag as the repository owner's call, not this session's; swapping the auth mechanism before either of those is resolved is the same category of decision, not a code fix. Re-verified locally end-to-end after the env: refactor: pack, isolated-feed install, `owen --version`, and the full uninstall/reinstall cycle all still pass with the version flowing through the environment instead of inline interpolation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw --------- Co-authored-by: Claude <noreply@anthropic.com>
…n facade (P-013 gate B) Rebase of PR #248 onto post-#247 main. New workflow (.github/workflows/action-marketplace-readiness.yml): consumer-simulation (uses: ./ against fixtures/marketplace-consumer-demo, not the precision-test corpus) -> validate-release-tag (immutable vX.Y.Z, gated on both event_name == 'push' and the ref prefix) -> move-major-tag (workflow_dispatch-only, environment-gated via the shared scripts/check_environment_protection.sh predicate #247 introduced, template-injection-safe input handling). docs/notes/action-marketplace-readiness.md documents the versioning policy, the consumer-simulation fixture rationale, and two prior review corrections (environment auto-creation with zero rules; predicate tightened to required_reviewers-only). README.md/README.ru.md get a casing fix (PhysShell/own.net -> PhysShell/Own.NET) and a versioning-policy pointer. Final review hardening: git rev-parse "refs/tags/$TARGET" returns an annotated tag's own SHA, not the commit it points at. Peeled to ^{commit} so the major tag always ends up on the release commit rather than on the tag object itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw
3a391f0 to
831271f
Compare
Что и зачем
Rebuild of PR #245 directly against post-#246
main. Per the reviewer's instruction not to merge #245 in its current pre-Owen form, PR #245 is being closed in favor of this one.action.yml's own SHA pins and Owen display-name branding ("Owen lifetime/resource check") are already inmainvia PR #246 — this PR adds the release-readiness pipeline around that, without touching either..github/workflows/action-marketplace-readiness.yml):consumer-simulation(a dedicated fixture, not the precision-test corpus, viauses: ./) →validate-release-tag(barevX.Y.Ztags, immutable, gated on bothevent_name == 'push'and the ref prefix) →move-major-tag(workflow_dispatch-only, environment-gated, template-injection-safe input handling). Distinct tag namespace from the CLI'sowen-cli-v*(PR feat(owen-cli): release-readiness pipeline, rebuilt against the Owen facade (P-013) #247).fixtures/marketplace-consumer-demo/{Leaky.cs,Clean.cs}: unchanged from PR feat(action): Marketplace-readiness pipeline for the composite action #245 —OWN001and every other diagnostic code are explicitly untouched by the public facade rebrand. The fixture's own README now describes the product being installed as Owen (the Action's public display name) while keeping Own.NET for the repository/project identity itself.docs/notes/action-marketplace-readiness.md(companion todocs/notes/owen-cli-release.md): metadata/branding audit, the versioning policy, the consumer-simulation fixture rationale, and the publish checklist. The license blocker is addressed explicitly to the repository owner, not resolved here.README.md/README.ru.md:PhysShell/own.net→PhysShell/Own.NETcasing fix (still present onmain, confirmed before porting forward) plus a pointer to the versioning policy.Correction versus PR #245's own description: that PR's body claimed the
consumer-simulationjob references the action via a dynamicuses: PhysShell/Own.NET@${{ github.sha }}, resolved through the Actions runner's real action-fetch mechanism. That was never the actual, reviewed implementation —jobs.<job_id>.steps.usesdoes not evaluate GitHub Actions expressions at all (Codex caught this on PR #245 before merge), so the real, shipped mechanism has always beenuses: ./— the same oneci.yml'sown-check-codescandog-food job already uses.docs/notes/action-marketplace-readiness.md's "On the resolution mechanism" section gives the accurate account; this PR description states it correctly from the start rather than repeating the stale claim.Тип изменения
Как проверено
python tests/run_tests.py— 276/276ruff check ./mypy— cleanpython3 -c "import yaml; yaml.safe_load(...)"scripts/own-check.shthe action wraps:Leaky.cs→ exit 1,OWN001at the expected line;Clean.cs→ exit 0; SARIF format across the fixture directory →Owendriver name, non-empty output — matching every assertion the newconsumer-simulationjob makes.Связанные issue
Refs P-013 gate B, the "Action distribution" open question in
docs/proposals/P-013-distribution-surface.md. Supersedes #245 (closed in favor of this PR — pre-Owen branding, and a staleuses:claim in its description). 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.yml's step pins/branding are PR feat(owen): public facade rebrand for the CLI, Action, and SARIF identity #246's, untouched here; only the release workflow, fixture, and doc/README accuracy changed.continue-on-error: the one use (consumer-simulation's leak-sample step) is immediately followed by an explicit assertion onsteps.leak.outcome.Generated by Claude Code