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

Author: claude

.github/workflows/ci.yml

@@ -97,6 +97,44 @@ jobs:
           python audit/static/run_static.py --selftest
           python audit/runtime/ingest.py --selftest
 
+  environment-protection-selftest:
+    name: release-workflow environment-protection predicate (fixture-driven)
+    runs-on: ubuntu-latest
+    # No GitHub API call, no real Environment needed here -- this tests only
+    # the accept/reject PREDICATE the owen-cli-release.yml `publish` job and
+    # action-marketplace-readiness.yml `move-major-tag` job both call
+    # (scripts/check_environment_protection.sh) against fixture "Get an
+    # environment" API responses, entirely offline. Review: a bare
+    # `.protection_rules | length` check would have accepted a wait_timer-
+    # or branch_policy-only environment, or a required_reviewers rule with
+    # zero actual reviewers, as if it were a real human-approval gate.
+    steps:
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+      - name: zero protection rules -> reject
+        run: |
+          if ./scripts/check_environment_protection.sh scripts/fixtures/environment-protection/zero-rules.json; then
+            echo "FAIL: expected rejection (zero rules)"; exit 1
+          fi
+      - name: wait_timer only -> reject
+        run: |
+          if ./scripts/check_environment_protection.sh scripts/fixtures/environment-protection/wait-timer-only.json; then
+            echo "FAIL: expected rejection (wait_timer only)"; exit 1
+          fi
+      - name: branch_policy only -> reject
+        run: |
+          if ./scripts/check_environment_protection.sh scripts/fixtures/environment-protection/branch-policy-only.json; then
+            echo "FAIL: expected rejection (branch_policy only)"; exit 1
+          fi
+      - name: required_reviewers with zero users -> reject
+        run: |
+          if ./scripts/check_environment_protection.sh scripts/fixtures/environment-protection/required-reviewers-empty.json; then
+            echo "FAIL: expected rejection (required_reviewers, zero reviewers)"; exit 1
+          fi
+      - name: required_reviewers with a reviewer -> accept
+        run: |
+          ./scripts/check_environment_protection.sh scripts/fixtures/environment-protection/required-reviewers-with-reviewer.json \
+            || { echo "FAIL: expected acceptance (required_reviewers with a reviewer)"; exit 1; }
+
   tests:
     name: tests (py${{ matrix.python-version }})
     runs-on: ubuntu-latest

.github/workflows/owen-cli-release.yml

@@ -12,10 +12,16 @@ name: Owen.Cli release
 # Publish safety (two independent gates, both required):
 #   1. `publish` only runs when the trigger is a pushed tag matching
 #      `owen-cli-v*` — a `workflow_dispatch` run (no such tag ref) can
-#      build/test/pack/smoke-test but can never reach the publish job.
+#      build/test/pack/smoke-test but can never reach the publish job. A
+#      `pull_request` run (below) has neither a tag ref nor `push` as its
+#      event_name, so the same condition also skips it there.
 #   2. `publish` targets the `nuget-release` GitHub Environment, which a repo
 #      admin must configure with required reviewers (Settings -> Environments)
-#      before this can ever run unattended — see the release checklist.
+#      before this can ever run unattended — see the release checklist. The
+#      job additionally self-checks that the environment actually has a
+#      required_reviewers rule (scripts/check_environment_protection.sh)
+#      before doing anything else, since GitHub auto-creates a referenced-
+#      but-never-configured environment with zero protection rules.
 # The workflow never echoes secrets.NUGET_API_KEY; `dotnet nuget push` takes
 # it as a CLI argument (not printed by the tool) and Actions' own log
 # redaction masks any accidental echo of a registered secret value.
@@ -27,6 +33,18 @@ on:
   push:
     tags:
       - "owen-cli-v*"
+  # Review: the green PR check on this workflow's own PRs never actually ran
+  # build-test-pack -> smoke-test (ubuntu + windows) -- only the tag-push
+  # trigger did, and no tag is ever pushed from a PR. `publish` stays fully
+  # skipped on a pull_request event (gate 1 above), so this only exercises
+  # the pack/inspect/install/smoke path, never the publish path.
+  pull_request:
+    paths:
+      - "frontend/roslyn/OwnSharp.Cli/**"
+      - "frontend/roslyn/OwnSharp.Extractor/**"
+      - "ownlang/**"
+      - ".github/workflows/owen-cli-release.yml"
+      - "scripts/check_environment_protection.sh"
   workflow_dispatch: {}
 
 env:
@@ -246,7 +264,17 @@ jobs:
     environment: nuget-release
     permissions:
       contents: read
+      # Required to call GET /repos/.../environments/{name} below (Codex
+      # review: "the environment-read API requires Actions read permission
+      # for fine-grained repository tokens") -- contents:read alone is not
+      # enough for that specific endpoint.
+      actions: read
     steps:
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+        with:
+          persist-credentials: false
+          sparse-checkout: |
+            scripts
       # GitHub auto-creates a REFERENCED-BUT-NEVER-CONFIGURED environment on
       # first use, with NO protection rules (Codex review: "referencing a
       # missing environment creates it and the newly created environment has
@@ -256,19 +284,19 @@ jobs:
       # any step runs already let this run through if the environment was
       # never actually configured with required reviewers. This step is the
       # loud, fail-closed check for that: it runs first, before the artifact
-      # is even downloaded, and refuses to publish if the environment
-      # currently has zero protection rules, rather than silently letting an
-      # unconfigured "protected" environment behave like no gate at all.
+      # is even downloaded, and refuses to publish unless the environment
+      # has a REQUIRED_REVIEWERS rule with at least one reviewer (Codex
+      # review: a bare protection_rules count also accepts a wait_timer- or
+      # branch_policy-only environment, neither of which waits for a
+      # human) -- scripts/check_environment_protection.sh is the single
+      # source of truth for that predicate, fixture-tested in ci.yml.
       - name: Refuse to publish unless nuget-release actually has protection rules
         env:
           GH_TOKEN: ${{ github.token }}
         run: |
-          rules=$(gh api "repos/${{ github.repository }}/environments/nuget-release" --jq '.protection_rules | length')
-          if [ "$rules" -eq 0 ]; then
-            echo "::error::the 'nuget-release' GitHub Environment has no protection rules (required reviewers) configured -- a repo admin must set that up under Settings -> Environments before a tag push can safely reach 'dotnet nuget push'. Refusing to publish."
-            exit 1
-          fi
-          echo "OK: nuget-release environment has $rules protection rule(s) configured"
+          gh api "repos/${{ github.repository }}/environments/nuget-release" > "$RUNNER_TEMP/nuget-release-env.json"
+          ./scripts/check_environment_protection.sh "$RUNNER_TEMP/nuget-release-env.json" \
+            || { echo "::error::the 'nuget-release' GitHub Environment does not have a required_reviewers rule with at least one reviewer -- a repo admin must set that up under Settings -> Environments before a tag push can safely reach 'dotnet nuget push'. Refusing to publish."; exit 1; }
       - uses: actions/setup-dotnet@67a3573c9a986a3f9c594539f4ab511d57bb3ce9 # v4
         with:
           dotnet-version: "8.0.x"

docs/notes/owen-cli-release.md

@@ -97,8 +97,16 @@ cannot be missed.
 
 Three jobs, each gated on the previous succeeding:
 
-1. **`build-test-pack`** (always runs, on a push of an `owen-cli-v*` tag
-   or a manual `workflow_dispatch`) — the standard repo gates
+1. **`build-test-pack`** (runs on a push of an `owen-cli-v*` tag, a manual
+   `workflow_dispatch`, **or a pull request** touching
+   `frontend/roslyn/OwnSharp.Cli/**`, `frontend/roslyn/OwnSharp.Extractor/**`,
+   `ownlang/**`, this workflow file, or the environment-protection script —
+   **correction (review):** the `pull_request` trigger was missing entirely
+   at first, so a green PR check on *this workflow's own PRs* never actually
+   ran `build-test-pack` → `smoke-test` — only a real tag push did, and no
+   PR ever pushes a tag. The `publish` job's existing tag-push condition
+   (item 3 below) is untouched, so it stays skipped on a `pull_request`
+   event exactly as it already was on `workflow_dispatch`.) — the standard repo gates
    (`run_tests.py`, `ruff`, `mypy`), `dotnet build`, the tag/version-match
    assertion above (skipped on manual dispatch, since there's no tag),
    `dotnet pack`, then **inspects the packed `.nupkg` contents** and fails
@@ -142,6 +150,18 @@ Three jobs, each gated on the previous succeeding:
    fails loudly, before the artifact is even downloaded — unless
    `protection_rules` is non-empty. This converts "never configured" from a
    silent bypass into a loud failure.
+   **Second correction (review):** a bare `protection_rules` count is too
+   permissive — GitHub's `protection_rules` array can also hold `wait_timer`
+   and `branch_policy` rules, neither of which waits for a human, and a
+   `required_reviewers` rule can itself be saved with zero reviewers (also
+   not a real gate). The check now calls
+   `scripts/check_environment_protection.sh` — a small, shared, fixture-
+   tested predicate (also used by `action-marketplace-readiness.yml`'s
+   `move-major-tag` job) that only accepts a `required_reviewers` rule with
+   at least one actual reviewer. The job's `permissions:` also gained
+   `actions: read`, which the environment-read endpoint requires alongside
+   `contents: read`. See "Testing the environment-protection predicate"
+   below.
 
 `ci.yml`'s existing `ownsharp-cli-smoke` job (job key kept, content already
 Owen-branded by PR #246) is untouched by this workflow and keeps proving the
@@ -150,6 +170,35 @@ release-specific path (slower, gated, gives the "did the *actual release
 artifact* survive a clean install on both OSes" answer right before
 publish).
 
+## Testing the environment-protection predicate
+
+`scripts/check_environment_protection.sh <environment-json-file>` is the
+single source of truth both release workflows' environment-gate checks call
+(`owen-cli-release.yml`'s `publish` job here; `action-marketplace-readiness.yml`'s
+`move-major-tag` job for the Action's own `action-major-tag-move`
+environment). It takes a GitHub "Get an environment" API response and exits
+0 only if `protection_rules` contains a `required_reviewers` rule with at
+least one reviewer — rejecting a `wait_timer`-only or `branch_policy`-only
+environment, and rejecting a `required_reviewers` rule saved with zero
+reviewers, both of which a bare `protection_rules | length` check would
+have wrongly accepted as "protected."
+
+Tested entirely offline, no GitHub API call and no real Environment needed:
+`ci.yml`'s `environment-protection-selftest` job runs the script against
+five fixtures under `scripts/fixtures/environment-protection/` and asserts
+the expected accept/reject outcome for each —
+
+| Fixture | Expected |
+|---|---|
+| `zero-rules.json` | reject |
+| `wait-timer-only.json` | reject |
+| `branch-policy-only.json` | reject |
+| `required-reviewers-empty.json` | reject |
+| `required-reviewers-with-reviewer.json` | accept |
+
+— so this predicate is exercised on every ordinary push/PR, not only when a
+real release or major-tag move actually runs.
+
 ## Release checklist
 
 Run through this, in order, for every release:

scripts/check_environment_protection.sh

@@ -0,0 +1,34 @@
+#!/usr/bin/env bash
+# Decides whether a GitHub Environment's "Get an environment" API response
+# proves an actual human-reviewer gate is configured -- the ONLY protection
+# rule type that means "this job pauses for a person to approve it".
+#
+# GitHub's `protection_rules` array can hold three rule types (wait_timer,
+# required_reviewers, branch_policy); a bare `.protection_rules | length`
+# check accepts any of them, including a wait_timer-only or
+# branch_policy-only environment that never actually waits for a human
+# (review: two release workflows -- owen-cli-release.yml's `publish` job,
+# action-marketplace-readiness.yml's `move-major-tag` job -- both once made
+# this mistake). A `required_reviewers` rule with an EMPTY `reviewers` array
+# is likewise not a real gate -- GitHub allows saving one, and it approves
+# nothing.
+#
+# Usage: check_environment_protection.sh <environment-json-file>
+# Exit 0 (ACCEPT) only if at least one required_reviewers rule has >=1
+# reviewer. Exit 1 (REJECT) for every other case, with a reason on stderr.
+set -euo pipefail
+
+file="${1:?usage: check_environment_protection.sh <environment-json-file>}"
+
+count=$(jq '[
+  .protection_rules[]?
+  | select(.type == "required_reviewers")
+  | select((.reviewers // []) | length > 0)
+] | length' "$file")
+
+if [ "$count" -eq 0 ]; then
+  echo "REJECT: no required_reviewers protection rule with at least one reviewer found" >&2
+  exit 1
+fi
+
+echo "ACCEPT: $count required_reviewers rule(s) with at least one reviewer configured"

scripts/fixtures/environment-protection/branch-policy-only.json

@@ -0,0 +1,9 @@
+{
+  "protection_rules": [
+    {
+      "id": 2,
+      "node_id": "MDQ6R2F0ZTI=",
+      "type": "branch_policy"
+    }
+  ]
+}

scripts/fixtures/environment-protection/required-reviewers-empty.json

@@ -0,0 +1,11 @@
+{
+  "protection_rules": [
+    {
+      "id": 3,
+      "node_id": "MDQ6R2F0ZTM=",
+      "type": "required_reviewers",
+      "prevent_self_review": true,
+      "reviewers": []
+    }
+  ]
+}

scripts/fixtures/environment-protection/required-reviewers-with-reviewer.json

@@ -0,0 +1,19 @@
+{
+  "protection_rules": [
+    {
+      "id": 4,
+      "node_id": "MDQ6R2F0ZTQ=",
+      "type": "required_reviewers",
+      "prevent_self_review": true,
+      "reviewers": [
+        {
+          "type": "User",
+          "reviewer": {
+            "login": "octocat",
+            "id": 1
+          }
+        }
+      ]
+    }
+  ]
+}

scripts/fixtures/environment-protection/wait-timer-only.json

@@ -0,0 +1,10 @@
+{
+  "protection_rules": [
+    {
+      "id": 1,
+      "node_id": "MDQ6R2F0ZTE=",
+      "type": "wait_timer",
+      "wait_timer": 30
+    }
+  ]
+}

scripts/fixtures/environment-protection/zero-rules.json

@@ -0,0 +1,3 @@
+{
+  "protection_rules": []
+}