feat(runner-policy): register the ci-workflows v0.6.0 revision - #175
Conversation
No linked issue. Registers ec91c3433a8c3c0a7ebbdd239286e5a6a25eeec5 (ci-workflows v0.6.0) for the consumer pin wave: - Selector: appended to the melodic-software owner scope. The revision admits merge_group and pull_request_target for metadata-only gates (merge_group has no fork variant; pull_request_target executes the trusted base-ref definition) and extends the fork guard to both pull-request event names. - claude-review contract: mirrors the df54d0e runner-input shape exactly (runner + skip-actors, CLAUDE_CODE_OAUTH_TOKEN, same caller-permission waiver). The revision's changes are behavioral only — per-(PR, head) concurrency keying plus a superseded-head guard; the standards-ref mount inputs/secrets stay unregistered until a consumer needs them. - link-check contract: carries the 3dfb1845 runner-input shape forward and admits the new rolling-issue interface (issue-title, issue-labels, issue-type, auto-close) for the medley adoption. ## Verification - GITHUB_REPOSITORY=melodic-software/standards npm run lint:runner-policy passes. - npm run test:runner-policy: 208/208 (selector-allowlist snapshot updated). - npm run lint:md: 0 errors. ## Related - melodic-software/github-iac#78 (epic — Campaign A hosted-floor elimination) - melodic-software/ci-workflows v0.6.0 (#135/#136/#137) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
PR Review
Verdict: LGTM. Three points verified below.
|
The pin wave moves every ci-workflows reusable pin to ec91c3433a8c3c0a7ebbdd239286e5a6a25eeec5, but five workflows cannot ride the structural auto-approval extension: - semantic-pr, do-not-merge-gate, pr-issue-linkage: selector-result contracts decline auto-approval unconditionally, even byte-identical (pr-issue-linkage is byte-identical; the other two changed only the prerequisite-result description text). - pulumi-version-drift-check: carries allowedCallerPermissions, which declines unconditionally; the workflow is byte-identical to the reviewed 3dfb1845 revision. - pester: input-declaration surface changed (pester-version default 5.7.1 to 6.0.0 plus an import version assertion step), so the surface diff correctly refuses to treat it as unchanged. Each entry carries its existing reviewed shape forward verbatim; no terms widen. zizmor and osv-scanner stay unregistered at v0.6.0 -- both are byte-identical to reviewed revisions with no unobservable-trust terms, so the auto-approval path covers them. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Scope extended after empirically checking the auto-approval paths: selector-result contracts (semantic-pr, do-not-merge-gate, pr-issue-linkage) and allowedCallerPermissions contracts (pulumi-version-drift-check) decline the structural auto-approval unconditionally — even byte-identical — and pester's input-declaration surface changed (pester-version default 5.7.1 → 6.0.0). Registered all five at ec91c343 with their existing reviewed shapes carried forward verbatim. zizmor and osv-scanner intentionally stay unregistered: byte-identical to reviewed revisions with no unobservable-trust terms, so the auto-approval extension covers them. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8f610c93b
ℹ️ 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".
|
Claude finished @kyle-sexton's task in 1m 35s —— View job PR Review
Verdict: LGTM. The scope-extended commit (d8f610c) passes on all dimensions. Findings below. Expanded scope — five contracts at
|
| Contract | Prior revision | Shape match |
|---|---|---|
semantic-pr@ec91c343 |
51012e2c |
✅ Identical — selectorResultInput, runner + prerequisite-result, no secrets |
do-not-merge-gate@ec91c343 |
88530217 |
✅ Identical — selectorResultInput, runner + prerequisite-result + label, no secrets |
pr-issue-linkage@ec91c343 |
f7e94a80 |
✅ Identical — selectorResultInput, runner + prerequisite-result, no secrets |
pulumi-version-drift-check@ec91c343 |
3dfb1845 |
✅ Identical — runner, contents: read + issues: write |
pester@ec91c343 |
99ac2f8c |
✅ Identical — hosted-only, windows-2025, same five-input allowlist |
pester-version default change (5.7.1 → 6.0.0) is a behavioral delta inside the workflow, not a contract surface change — it doesn't appear in allowedInputs, and the registration correctly uses the same shape.
zizmor and osv-scanner — intentional omission
Both are byte-identical to their most recent reviewed revisions and carry no unobservable-trust terms, so the auto-approval extension applies. No registration needed; intentionally absent.
runner-policy.test.mjs — Snapshot
MERGE_GROUP_ROUTING_SHA is added to the melodic-software owner-scope assertion and the corresponding audit loop — correct placement for a strict-scheduling revision. The five new contracts at ec91c343 don't require dedicated snapshot test entries; the contract validation path is separate from the selector allowlist snapshot.
Codex inline comment — resolved
The selectorResultInput / !cancelled() divergence raised by Codex is addressed in kyle-sexton's reply: the validator enforces exactly if: ${{ always() }} for selector-result reporters, callers ship if: ${{ always() }} and keep it through the repin, and the docs-only reconciliation is tracked in ci-workflows#140. Not a blocker.
No linked issue. Repins every ci-workflows selector and reusable-workflow reference to `ec91c3433a8c3c0a7ebbdd239286e5a6a25eeec5` (v0.6.0), registered by #175. Gate callers gain the `merge_group` / same-repo `pull_request_target` selector routing; the claude-review lane picks up per-head concurrency and the superseded-head guard. `standards-sync.yml` stays at `35f2684a`: its secret-capable contract is excluded from auto-approval and converts in the github-iac#157 runner-input lane instead. ## Verification - `GITHUB_REPOSITORY=melodic-software/standards node components/runner-policy/runner-policy.mjs --root .` passes locally against the merged #175 policy. - No residual pre-v0.6.0 ci-workflows workflow pins (grep-verified) apart from the intentional standards-sync hold. ## Related - #175 (policy registration) - melodic-software/github-iac#78 (epic — Campaign A) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
No linked issue. Repins every ci-workflows selector and reusable-workflow reference to `ec91c3433a8c3c0a7ebbdd239286e5a6a25eeec5` (v0.6.0), registered in the runner policy by melodic-software/standards#175 and distributed by the just-merged standards-sync PR. Gate callers gain `merge_group` / same-repo `pull_request_target` selector routing; the claude-review lane picks up per-head concurrency and the superseded-head guard. ## Verification - Local dogfood: `GITHUB_REPOSITORY=melodic-software/claude-code-plugins node .github/standards/runner-policy/runner-policy.mjs --root .` passes against the #175 policy. - No residual pre-v0.6.0 ci-workflows workflow pins (grep-verified). ## Related - melodic-software/standards#175 (policy registration) - melodic-software/github-iac#78 (epic — Campaign A) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
## Summary - correct `semantic-pr.yml`'s `prerequisite-result` description to require the validator-enforced `if: always()` caller shape - replace the stale job comment with the deliberate fail-closed cancellation tradeoff already documented by #134 - add a regression test that requires the `always()`/fail-closed relationship and rejects `!cancelled()` on both semantic-pr documentation surfaces ## Why The reusable workflow enforced every delivered non-success prerequisite result as a failure, while its public input description and job comment still recommended `!cancelled()`. That contradicted the standards validator and the canonical caller shape documented by #134. GitHub documents that a job depending on a failed or skipped job is itself skipped unless its condition continues execution, and specifically recommends `always()` for required dependent checks: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-jobs#defining-prerequisite-jobs GitHub also documents both sides of the deliberate tradeoff: `always()` remains true during cancellation, while `!cancelled()` is the general alternative for work that should stop: https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#always For this small required reporter, the repository intentionally chooses `always()` so a cancelled prerequisite cannot disappear as a skipped-success required check. ## Impact Documentation now matches the enforced selector-result contract. Runtime behavior and caller configuration are unchanged. ## Verification - `node --test .github/scripts/semantic-pr.test.cjs` — 3/3 passed - `node --test .github/scripts/*.test.cjs` — 207/207 passed - focused and repository-wide `actionlint -shellcheck=` - `git diff --check` Fixes #140 ## Related - #131 - #134 - melodic-software/standards#175
No linked issue.
Registers
ec91c3433a8c3c0a7ebbdd239286e5a6a25eeec5(ci-workflows v0.6.0, containing ci-workflows#135/#136/#137) so the consumer pin wave can proceed. Selector-lockstep step 2; the consumer repin PRs (step 3) follow after this syncs.melodic-softwareowner scope. The revision admitsmerge_groupandpull_request_targetfor metadata-only required gates (merge_grouphas no fork variant and only write-access users can enqueue one;pull_request_targetexecutes the trusted base-ref definition) and extends the fork guard to both pull-request event names, so fork-origin PR contexts still route off the managed fleet. README review note added; approved-revision counts updated (nine total, six strict-scheduling).df54d0erunner-input shape exactly (runner+skip-actors,CLAUDE_CODE_OAUTH_TOKEN, sameallowedCallerPermissionswaiver). The interface is unchanged sincedf54d0e; the revision's delta is behavioral only (per-(PR, head-SHA) concurrency keying + superseded-head guard). Thestandards-refmount input andSTANDARDS_REVIEW_APP_*secrets stay unregistered — no consumer passes them, and grants/allowances are never provisioned ahead of need.3dfb1845runner-input shape forward (runner,args,contents: read+issues: writewaiver) and admits the revision's new rolling-issue interface (issue-title,issue-labels,issue-type,auto-close), which the medley docs-link-check adoption consumes next.f7e94a80; the other two changed onlyprerequisite-resultdescription text. Shapes carried forward verbatim.allowedCallerPermissions, which declines auto-approval unconditionally; byte-identical to the reviewed3dfb1845revision. Shape carried forward verbatim.pester-versiondefault 5.7.1 → 6.0.0 plus an import version assertion), so auto-approval correctly refuses; hosted-onlywindows-2025shape carried forward verbatim.ec91c343: both are byte-identical to reviewed revisions with no unobservable-trust terms, so the deterministic auto-approval extension covers their pin bumps.Verification
GITHUB_REPOSITORY=melodic-software/standards npm run lint:runner-policy— passednpm run test:runner-policy— 208/208 (selector-allowlist snapshot extended)npm run lint:md— 0 errorsRelated
🤖 Generated with Claude Code