feat(select-runner): admit issues event under generalized ancillary opt-in - #225
Conversation
…pt-in The comment/review opt-in added in #224 (admits-comment-events) closed the hosted-fallback gap for issue_comment / pull_request_review / pull_request_review_comment but left the `issues` event uncovered: it is in neither the unconditional local allowlist nor the opt-in set, so a private-repo caller triggering on `issues` under `self-hosted-only` routes HOSTED (billable) on selector SUCCESS. Confirmed live consumer: medley `issue-labeling.yml` (on: issues) whose auto-label job is a pure github-script no-checkout job that then trips its own require-self-hosted sentinel. Generalize the single opt-in to admit `issues` under the identical no-checkout, fork-safe, fail-closed gate: - Rename input `admits-comment-events` -> `admits-ancillary-events` and the admitted event set to ANCILLARY_EVENTS = {issue_comment, pull_request_review, pull_request_review_comment, issues}. The name now honestly covers the issue-lifecycle event; safe to rename because no caller sets the flag yet (fleet audit confirmed zero adopters). - `issues` is strictly lower-risk than the already-admitted events: it carries no PR head-ref, so there is nothing for the fork guard to protect, and the caller's no-checkout attestation is the whole gate. Preserved unchanged: fork-origin PRs route hosted; self-hosted-only config errors yield the ci-runner-selection-failed sentinel; public repos route hosted; the observer-token mint surface widens for `issues` ONLY inside the opt-in-gated clause, never as an unconditional route. Regenerated the embedded selector block via render-select-runner-workflow.cjs. Full suite 251/251 (selector 116/116, +4 issues cases), actionlint / zizmor / typos / editorconfig-checker clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rYiP56ofZpx7oCWPkJ6No
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Warning Automated security 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. |
|
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e4a266d57
ℹ️ 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".
|
🔒 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). |
…alias Renaming the opt-in input to admits-ancillary-events is a breaking change to the workflow_call input contract: a caller that repins to this revision while still passing the former name (including anyone following #224's merged guidance, which documented `admits-comment-events: true`) would hit a hard unknown-input rejection rather than a hosted fallback. The audit found zero current adopters, but out-of-tree and mid-flight consumer branches cannot be verified away, and this reusable is pinned by SHA across every repo. Retain `admits-comment-events` as a deprecated alias, ORed with admits-ancillary-events at the yml input boundary so either name opts a caller in under both policies: - Env passthrough feeds the single flag the selector reads from the OR of both inputs (`inputs.admits-ancillary-events || inputs.admits-comment-events`). - The prefer-self-hosted observer-token mint gate admits either input. - The tested select-runner.cjs core stays single-flag; the shim lives only in the yml input layer, so removing it later (once no caller sets it) reverts one file. Tests: token-mint guard now locks both inputs and the OR'd env mapping; a new test asserts the deprecated alias input remains declared. Full suite 252/252; render --check in sync; actionlint / zizmor / typos / editorconfig-checker clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rYiP56ofZpx7oCWPkJ6No
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e29f396ec
ℹ️ 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".
| github.event.pull_request.head.repo.full_name != github.repository }} | ||
| API_TIMEOUT_SECONDS: ${{ inputs.api-timeout-seconds }} | ||
| ADMITS_COMMENT_EVENTS: ${{ inputs.admits-comment-events }} | ||
| ADMITS_ANCILLARY_EVENTS: ${{ inputs.admits-ancillary-events || inputs.admits-comment-events }} |
There was a problem hiding this comment.
When an existing caller has a multi-event workflow that already passes admits-comment-events: true for comment/review handling and also triggers on issues, this OR turns that old comment-only attestation into admission for every ANCILLARY_EVENTS member, including issues. On an issues run the selector now sees ADMITS_ANCILLARY_EVENTS=true and can return a self-hosted label even though the caller never opted its issue-triggered job into the new broader contract; keep the alias scoped to the three original comment/review events or pass both booleans separately.
Useful? React with 👍 / 👎.
1 participant
No related issue: no tracked issue exists for this gap — it is the residual of #224 (which scoped only comment/review events) and part of the 2026-07-22 private-repo CI/CD fleet-audit runner-routing remediation.
Related
admits-comment-eventsopt-in for comment/review events) and Route private-repo comment-event jobs to self-hosted runners (Actions budget capped at $0) #217 (route private-repo ancillary jobs to the fleet, $0 hosted budget).issue-labeling.yml(on: issues).The routing gap
#224 closed the hosted-fallback gap for
issue_comment/pull_request_review/pull_request_review_commentvia the per-calleradmits-comment-eventsopt-in. It did not cover theissuesevent, which is in neither the unconditionalLOCAL_EVENT_ALLOWLISTnor the opt-in set. So underself-hosted-only, a private-repo caller triggering onissuesgetshostedResult(..., "hosted-only")on selector success and its work job runs on a GitHub-hosted (billable) runner — violating the $0-budget / 100%-self-hosted mandate for private repos.Confirmed live consumer: medley
issue-labeling.ymltriggers onissues: [opened], routes throughselect-runnerunder${{ vars.CI_RUNNER_POLICY }}(self-hosted-only), and itsauto-labeljob is a pureactions/github-scriptjob with zero checkout. Today it routes hosted and then trips its ownrequire-self-hostedsentinel, failing the run.The fix
Generalize the single opt-in to admit
issuesunder the identical no-checkout, fork-safe, fail-closed gate the comment/review events already use:admits-comment-events→admits-ancillary-events; admitted set →ANCILLARY_EVENTS = {issue_comment, pull_request_review, pull_request_review_comment, issues}.issuesis strictly lower-risk than the events already admitted: it carries no PR head-ref, so the fork guard has nothing to protect; the caller's no-checkout attestation is the whole gate.Fork-safety, fail-closed, public preservation (unchanged)
pull_request/pull_request_targetstill route hosted (FORK_GUARDED_EVENTSuntouched).self-hosted-onlyconfig errors still throw and yield theci-runner-selection-failedsentinel — no hosted fail-open.issuesonly inside theadmits-ancillary-events-gated clause; the unconditional token-mint event set is unchanged (test-locked).Blast radius
select-runner.ymlis a shared reusable pinned by SHA across every repo (medley, github-iac, provisioning, dotfiles). This change is additive and default-off: callers that do not passadmits-ancillary-events: trueare unaffected; public callers are unaffected. Because the input was renamed while still unadopted, any future caller adopts the new name directly.Companion event-routing audit (all private repos) found this is the ONLY remaining gap event; github-iac, provisioning, and dotfiles have zero gap-class triggers. A separate, consumer-side gap remains out of scope here: ~84
|| 'ubuntu-24.04'fail-open fallbacks in medley/github-iac/provisioning lanes (dotfiles already uses the correct|| 'melodic-ubuntu-24.04-x64'/ sentinel pattern) — tracked separately.Consumer follow-up (not in this PR)
After this merges and cuts a tag, medley repins
select-runner.ymland then addsadmits-ancillary-events: truetoissue-labeling.yml's selector call (and tocomment-review-gate.ymlfor the comment/review events #224 enabled). Note the generalized flag: any caller setting ittruenow also admitsissuesunder the same no-checkout attestation.Verification
node --test .github/scripts/*.test.cjs— 251/251 (selector suite 116/116; +4issuescases: opt-in true/false ×prefer-self-hosted/self-hosted-only). Theworkflow_runnegative test confirms a non-admitted event stays hosted even with the flag true.node .github/scripts/render-select-runner-workflow.cjs --check— embedded block byte-in-sync with source.actionlint,zizmor,typos,editorconfig-checkeron all changed files — clean.🤖 Generated with Claude Code
https://claude.ai/code/session_015rYiP56ofZpx7oCWPkJ6No