feat(runner-policy): admit claude-review@df54d0e as runner-input - #161
Merged
Conversation
Closes #159. Flips the claude-review.yml@df54d0e8 contract from hosted-only to runner-input (runner + skip-actors, same secret and caller-permission waiver as 4dbb0dfc). df54d0e (ci-workflows#117, claude-code-action v1.0.174) surfaces real review failures as explicit PR comments; the Campaign A callers pinned 4dbb0dfc sit in that failure-reporting regression window until they can bump to this SHA. In-place mutation is safe: zero callers reference df54d0e on any main (verified across all nine requires-ci repos), so no caller depends on the hosted-only shape the issue proposed to retain. Full component suite passes (181/181). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 20s —— View job PR Review
The change is correct and well-reasoned. Here's what I verified: Shape fidelity. The mutated Schema compliance. The Permission scope. In-place mutation rationale. The PR correctly identifies that JSON object keys must be unique, so a No findings. The change is minimal, schema-valid, and mirrors an already-approved entry exactly. The 181/181 test suite pass confirms mechanical correctness. |
kyle-sexton
added a commit
that referenced
this pull request
Jul 31, 2026
…contract (#307) ## Summary Adds `skip-actors` to the `allowedInputs` of the runner-policy contract entry for `melodic-software/ci-workflows/.github/workflows/claude-security-review.yml@c136b27` — the pin both security-lane consumers run today. Why now: `claude-code-plugins` re-deliberated ADR 0002's skip-actor exception (melodic-software/claude-code-plugins#1767) and **rejected** the reusable workflow's inherited default (`dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot]`) for the *security* lane. `claude[bot]` and `melodic-ai[bot]` carry none of the compensating controls that justified the two operator-ratified actors — a Dependabot pin bump is forced through this very runner-policy contract, and a standards-sync PR materializes byte-exact upstream-reviewed content — so an AI-app-pushed commit would satisfy that repo's **required** security check with no review run of any kind. Its caller must therefore pass an explicit narrow list (`dependabot[bot],melodic-standards-sync[bot]`), and the caller can only pass an input the contract admits. This **restores parity rather than widening new ground**: the previous approved security SHA's contract (`66073e58`) already admitted `skip-actors`; the `c136b27` entry dropped it, which is why the consumer's caller could not keep its explicit list through the re-pin. The `claude-review.yml` contracts at `df54d0e` and `4dbb0df` admit it too. A second commit reconciles `components/runner-policy/README.md`, whose `c136b27` review note still recorded the security caller as able to pass only `runner` and `paths-file` — a reviewer consulting it would have read `skip-actors` as denied. The `(of 8)` denominator is unchanged: `skip-actors` is one of the reusable's eight declared inputs at that SHA (`runner`, `paths`, `paths-file`, `prompt`, `claude-args`, `exclude-comments-by-actor`, `skip-actors`, `retry-delay-seconds`), so the admitted set stays narrower than the declared surface. **Blast radius — permissive widening only.** `runner-policy` is a managed component across the fleet, so this materializes everywhere, but nothing starts passing the input: the validator rejects inputs *outside* `allowedInputs` and never requires one. The only behavior change is that a caller may now pass `skip-actors` to this pin without the policy check failing. The entry carries no `autoApproved` block, so it is a human-reviewed contract edit by design. Known residual gap (follow-up, not this PR): the validator rejects unexpected inputs but cannot *require* one, so a future re-pin that drops a caller's `skip-actors` line passes CI and silently re-widens the exception. Guarding that needs a `requiredInputs`-style contract field — filed as #308, which also records the matching hazard in the parked `claude-security-review-caller` component (its bytes pass no `skip-actors`, so an adopter inherits the widened default when it unparks). ## Test plan - `node -e "JSON.parse(...)"` on `components/runner-policy/policy.json` — parses. - `biome check components/runner-policy/policy.json` — clean (also clean via the `pre-commit` lefthook run: typos, gitleaks, editorconfig, biome all pass). - Diff is one line: `"allowedInputs": ["runner", "paths-file"]` → `["runner", "paths-file", "skip-actors"]`, matching the file's one-line-array style and the schema's `uniqueStringArray`. - Input verified against the reusable at the pinned SHA: `skip-actors` is a declared `workflow_call` input on `claude-security-review.yml@c136b27`, defaulting to the four-actor list and gating the `security-review` **job** (not the workflow), so a skipped actor still reports a name-stable passing check. - Downstream verification: with this change applied locally to the consumer's materialized `policy.json`, `runner-policy.mjs` reports `Runner policy passed.` against the narrowed caller; without it, exactly one finding (`inputs absent from its reviewed contract: skip-actors`). ## Related No linked issue in this repository — deliberately. The deliberation and its record live in the consuming repo (melodic-software/claude-code-plugins#1767), which is closed by its own caller PR; a cross-repo closing keyword here would close that issue on the wrong merge. - Decision: melodic-software/claude-code-plugins#1767 (ADR 0002 re-deliberation) - Sequencing: this PR → standards→claude-code-plugins sync PR → the caller + ADR PR in claude-code-plugins - Consuming PR (blocked on this one): melodic-software/claude-code-plugins#1845 — its `runner-policy` check stays red until this merges and the sync materializes - Follow-ups filed, not implemented here: #308 (`requiredInputs` contract field) and melodic-software/ci-workflows#330 (narrow the security reusable's own `skip-actors` default) - Contract precedent: #211 (security-review contract), #229 (`paths-file` input), #161 (`claude-review` runner-input contract) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVoZoMYXqf8ZVbQYixPVPW --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Closes #159.
Flips the
claude-review.yml@df54d0e8…contract from hosted-only to runner-input (runner+skip-actors, same secret and caller-permission waiver as the4dbb0dfcentry).df54d0e(melodic-software/ci-workflows#117, claude-code-action v1.0.174) surfaces real review failures — usage limits, OIDC/SDK errors, max-turns — as explicit PR comments; the six Campaign A callers pinned at4dbb0dfcsit in that failure-reporting regression window until they can bump to this SHA.Deviation from the issue's step 1: the issue asked to ADD a second
df54d0eentry and keep the hosted-only one, but contract entries are keyed bypath@SHA— one JSON key cannot carry both shapes. In-place mutation is safe instead: zero callers referencedf54d0eon any main (verified across all nine requires-ci repos), so nothing depends on the hosted-only shape.Pin-bump wave (issue step 3) follows separately after this syncs to consumers.
Verification
4dbb0dfc's registered runner-input shape exactly, plusrunnerinallowedInputs.Related
🤖 Generated with Claude Code
https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt