Skip to content

feat(runner-policy): register prerequisite gate runner contracts - #249

Merged
kyle-sexton merged 2 commits into
mainfrom
codex/ci177-runner-policy-registration
Jul 23, 2026
Merged

feat(runner-policy): register prerequisite gate runner contracts#249
kyle-sexton merged 2 commits into
mainfrom
codex/ci177-runner-policy-registration

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • register the exact merged melodic-software/ci-workflows prerequisite-gate contracts at 380612ae1d4e0cc9741efbac7b6ffb3d3da63a04
  • preserve each workflow's reviewed runner, prerequisite-result, allowed-input, and empty-secret boundaries
  • lock the production inventory in the runner-policy regression suite and document the strict-selector sentinel caveat

Why

ci-workflows#177 moved semantic-pr, do-not-merge-gate, and pr-issue-linkage to the caller-selected runner for every prerequisite outcome. Private consumers cannot repin to that immutable merge until the canonical standards runner policy recognizes the exact reviewed contracts.

The canonical components/runner-policy/policy.json is the source of truth. Its managed downstream copies will flow through the normal standards synchronization process; no generated or consumer copy is edited here.

Validation

  • focused production-contract test: 1 passed
  • complete runner-policy suite: 228 passed
  • runner-policy self-audit: passed
  • markdownlint: 0 errors
  • pre-commit: Biome, EditorConfig, markdownlint, typos, and gitleaks passed

No linked issue: this PR is the required standards registration follow-up to an already completed ci-workflows issue.

Related

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 27s —— View job


PR Review

  • Gather context and read changed files
  • Review policy.json changes
  • Review runner-policy.test.mjs changes
  • Review README.md changes
  • Check against REVIEW.md criteria
  • Post findings

Summary

Clean, narrowly scoped registration. The three new contracts at 380612ae are internally consistent with the established prerequisite-gate pattern, the SHA is identical across policy.json, the test constant, and the README prose, and no secrets are introduced. No blocking issues found.

REVIEW.md "Always check" pass

Criterion Result
Object-level authorization N/A — config-only change
Tenant-scoped data access N/A
Secrets in tracked source Clean — all three new contracts carry "allowedSecrets": {}
Injection via external input N/A
Audit logging for high-risk actions N/A
Atomicity across related writes N/A

Observations

Test loop departs from surrounding flat-assertion style — a minor consistency note, not a defect.

The rest of the "production contracts pin" test block uses individual assert.deepEqual calls. The three new contracts are verified through a for...of loop:

// runner-policy.test.mjs:3254-3271
for (const [workflow, allowedInputs] of [
  ["semantic-pr.yml", ["runner", "prerequisite-result"]],
  ...
]) {
  assert.deepEqual(
    contracts[`.../${workflow}@${PREREQUISITE_GATE_RUNNER_SHA}`],
    { ... allowedInputs, ... },
  );
}

The loop is functionally correct — assert.deepEqual inside a loop still pinpoints which workflow failed — and reduces repetition of an identical shape. The departure is worth noting, but it's a reasonable authoring choice, not something to change.

pr-issue-linkage@d7734df not covered by the production-contracts test — pre-existing gap, not introduced here. Line 400–405 of policy.json shows this contract exists, but the production-contracts test block has no corresponding assertion for it. This PR follows the same approach for the three new 380612ae entries (they are asserted), so the gap isn't widened.

No allowedCallerPermissions on any of the three new contracts — this matches every other prerequisite-gate contract at prior SHAs and is correct; the selection privilege flows via the selector result, not a caller-token permission.

Verdict

No blocking, important, or nit-level issues under the REVIEW.md criteria that survive verification. The PR is ready to merge.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c9df202167

ℹ️ 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".

Comment thread components/runner-policy/README.md
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

🔒 babysit-prs lane claiming this PR for on-branch fix work this cycle (amendment-round: 16, safe tier). Will fix clear branch-owned findings and push; will not resolve threads or merge (safe tier).

Resolve additive conflict in components/runner-policy/policy.json:
both branches append disjoint entries to approvedReusableWorkflowContracts.
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

@github-actions

Copy link
Copy Markdown

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Re-running the job, or pushing a new commit, will retry the review.

@kyle-sexton
kyle-sexton merged commit 39c1918 into main Jul 23, 2026
43 checks passed
@kyle-sexton
kyle-sexton deleted the codex/ci177-runner-policy-registration branch July 23, 2026 21:09
kyle-sexton added a commit that referenced this pull request Jul 26, 2026
…in e295107 (#275)

## What

Add the reviewed runner-input contracts for

`melodic-software/ci-workflows/.github/workflows/claude-review.yml@e2951077a7b43c09fc5a8dee4da52ba6f0fb39ed`
and

`melodic-software/ci-workflows/.github/workflows/claude-security-review.yml@e2951077a7b43c09fc5a8dee4da52ba6f0fb39ed`
(the ci-workflows#248/#249 squash-merge SHA) to
`approvedReusableWorkflowContracts`. Both entries are shape-identical to
the
previously reviewed pins (`claude-review@90f1c54`,
`claude-security-review@99cb082e`): same routing (`runner-input` via
`runner`),
same `allowedInputs` (`runner`, `skip-actors`; plus `paths` for
security-review), same single `CLAUDE_CODE_OAUTH_TOKEN` secret mapping,
and
same `allowedCallerPermissions` (`contents:read`, `pull-requests:write`,
`id-token:write`). The superseded entries stay until the last caller
re-pins
and can then be retired, per the #229 precedent.

## Why

The runner-policy gate correctly declined auto-approval on
melodic-software/claude-code-plugins#1552: both contracts carry
`allowedCallerPermissions`, so a bumped SHA's step content must be
human-reviewed and recorded here — auto-approval is categorically
declined for
that contract class regardless of surface identity. This PR is that
review.

Contract review of `e2951077` against the reviewed pins:

- **No** `workflow_call` input, secret, permission, `runs-on`, or
routing
change in either workflow. `claude-security-review`'s `prompt` input
only
changed its **default prose** (defer to zizmor's static lane for what it
  already covers) — no input added or removed, no type/required change.
- Step content changed (the reason human review is required):
- `gh` CLI run steps (PR head freshness check, infra-failure PR comment
    lifecycle, PR file listing) replaced with
    `actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3`,
verified against upstream as the official `v9.0.0` annotated-tag commit.
Same API operations under the same implicit `github.token` and unchanged
    job permission grants.
  - The generated infra-failure classifier
(`.github/scripts/classify-infra-failure.sh`) reads the SDK result file
    and publishes only structured metadata fields plus a constrained
`class` token (`auth|rate-limit|overloaded|other`) — the model-authored
    free text and raw error stacks never leave the block.
- No new secret, no widened permission, no runner-label/routing change —
  no privilege widening.

Once merged, the standards-sync PR carries
`.github/standards/runner-policy/policy.json` into consumers, and
claude-code-plugins#1552 passes unchanged.

## Verification

- `npm run lint:runner-policy` — Runner policy passed.
- `npm run test:runner-policy` — 238 pass / 0 fail.
- `check-jsonschema --schemafile
components/runner-policy/policy.schema.json
components/runner-policy/policy.json` — ok.

No linked issue (reviewed-contract update; the blocked PR lives in
claude-code-plugins).

## Related

- melodic-software/ci-workflows#248, melodic-software/ci-workflows#249
(merged, `e295107`)
- melodic-software/claude-code-plugins#1552 (blocked on this review +
sync)
- Precedent: #229 → sync → caller bump

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant