feat(select-runner): opt-in local routing for comment/review events - #224
Merged
Conversation
Closes #217 Org GitHub Actions spending is capped at $0 (free private-repo minutes only, by design). issue_comment, pull_request_review, and pull_request_review_comment jobs were categorically excluded from local routing, so under the private-repo self-hosted-only policy they fell back to GitHub-hosted runners and now fail at startup with a billing-limit error whenever free minutes are exhausted. Studied the original exclusion (git blame/log through #92, #103, #123, #135): LOCAL_EVENT_ALLOWLIST is a plain default-deny allowlist ("only explicitly reviewed caller event classes may route locally"), not a comment-specific security ban. Comment/review events were simply never reviewed, not deliberately blocked. Auditing fleet consumers of this selector surfaced a real checkout-bearing comment-event job: medley's claude-assistant.yml triggers on issue_comment/pull_request_review_comment, checks out the repository, and runs Claude with contents:write and commit signing. Blanket-admitting the event classes (as originally scoped) would have made that currently-dormant job self-hosted-eligible as a side effect -- exactly the exposure the exclusion happened to protect, even though nothing in the exclusion's own history named it. Adds admits-comment-events (workflow_call input, default false) so a caller opts a *specific* comment/review-triggered job into local routing only when that job performs no checkout -- a per-caller declaration, not a blanket event-class change. permitsLocalExecution now treats issue_comment/pull_request_review/pull_request_review_comment as eligible only when the caller sets the flag; every existing policy/private-repo/fork gate still applies on top. claude-assistant.yml is untouched by this change and stays exactly as dormant as it is today -- its own opt-in, if ever made, is a separate decision with its own security review of checkout+write-on-fleet. Also fixes a second, independent event enumeration in this workflow: the "Mint read-only observer token" step's inline `if:` gates App-token minting for prefer-self-hosted policy and had its own hardcoded event list, unreachable for comment/review events even with the flag set. Extended it so prefer-self-hosted mode can mint the observer token and route these jobs locally too, not just self-hosted-only. ## Verification - `node --test .github/scripts/select-runner.test.cjs` -- 112/112 passing, including new coverage: admits-comment-events true/false x the three comment/review events x self-hosted-only/prefer-self-hosted, plus a regression test that the flag does not widen an unrelated blocked event class - `node --test .github/scripts/*.test.cjs` -- 247/247 passing repo-wide - `actionlint .github/workflows/select-runner.yml` -- clean - `node .github/scripts/render-select-runner-workflow.cjs --check` -- generated block in sync with select-runner.cjs ## Related - #209 / #212 (github-script port pattern; medley#1632 ports the concrete consumer this depends on) - medley#1632 (comment-review-gate.yml port -- the reviewed no-checkout consumer intended to opt in once repinned) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Claude-Session: https://claude.ai/code/session_01KYvF6bWGqemS9aYFfWJRiW
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
Closed
3 tasks
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
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. |
|
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. |
kyle-sexton
added a commit
that referenced
this pull request
Jul 23, 2026
…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
kyle-sexton
added a commit
that referenced
this pull request
Jul 23, 2026
…pt-in (#225) 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 - Builds on #224 (`admits-comment-events` opt-in for comment/review events) and #217 (route private-repo ancillary jobs to the fleet, $0 hosted budget). - Consumer that motivates it: medley `issue-labeling.yml` (`on: issues`). ## The routing gap #224 closed the hosted-fallback gap for `issue_comment` / `pull_request_review` / `pull_request_review_comment` via the per-caller `admits-comment-events` opt-in. It did **not** cover the `issues` event, which is in neither the unconditional `LOCAL_EVENT_ALLOWLIST` nor the opt-in set. So under `self-hosted-only`, a private-repo caller triggering on `issues` gets `hostedResult(..., "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.yml` triggers on `issues: [opened]`, routes through `select-runner` under `${{ vars.CI_RUNNER_POLICY }}` (`self-hosted-only`), and its `auto-label` job is a pure `actions/github-script` job with **zero checkout**. Today it routes hosted and then trips its own `require-self-hosted` sentinel, failing the run. ## The fix Generalize the single opt-in to admit `issues` under the identical no-checkout, fork-safe, fail-closed gate the comment/review events already use: - Input `admits-comment-events` → **`admits-ancillary-events`**; admitted set → `ANCILLARY_EVENTS = {issue_comment, pull_request_review, pull_request_review_comment, issues}`. - The rename makes the name honest for the broadened, heterogeneous set (comment + review + issue-lifecycle). It is safe to rename now because a fleet-wide audit confirmed **no caller sets the flag yet** (the #224 consumer opt-in is still deferred pending a medley repin), so there is zero breakage. - `issues` is 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) - Fork-origin `pull_request` / `pull_request_target` still route hosted (`FORK_GUARDED_EVENTS` untouched). - `self-hosted-only` config errors still throw and yield the `ci-runner-selection-failed` sentinel — no hosted fail-open. - Public repos still route hosted regardless of event/opt-in. - The observer-token mint step widens for `issues` **only inside** the `admits-ancillary-events`-gated clause; the unconditional token-mint event set is unchanged (test-locked). ## Blast radius `select-runner.yml` is 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 pass `admits-ancillary-events: true` are 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.yml` and then adds `admits-ancillary-events: true` to `issue-labeling.yml`'s selector call (and to `comment-review-gate.yml` for the comment/review events #224 enabled). Note the generalized flag: any caller setting it `true` now also admits `issues` under the same no-checkout attestation. ## Verification - `node --test .github/scripts/*.test.cjs` — **251/251** (selector suite 116/116; +4 `issues` cases: opt-in true/false × `prefer-self-hosted`/`self-hosted-only`). The `workflow_run` negative 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-checker` on all changed files — clean. - Independent fresh-context review — all invariants hold, no CRITICAL/IMPORTANT findings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_015rYiP56ofZpx7oCWPkJ6No --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 23, 2026
kyle-sexton
added a commit
that referenced
this pull request
Jul 24, 2026
…to a cheap page fetch and a per-candidate merge-state probe with retry (#234) Closes #226 ## Summary - Splits the stuck-automerge scan into two phases so the hourly cron stops dying on GitHub's opaque server-side GraphQL error: phase 1 pages open PRs selecting only `number`/`url`/`author`/`autoMergeRequest { enabledAt }` (no `mergeStateStatus`), phase 2 probes `mergeStateStatus` one PR at a time via `repository.pullRequest(number:)`, and only for armed, past-threshold bot PRs. - Wraps every GraphQL call in `graphqlWithRetry` — exponential backoff (`GRAPHQL_RETRY_ATTEMPTS=4`, `GRAPHQL_RETRY_BASE_MS=1000`, both step-env-tunable), rethrowing the last error on exhaustion so a persistent failure still fails the run loudly. On a throw the `stuck-count` output is never set, so the close-issue branch cannot fire a false all-clear. - Retains the existing manual-cursor pagination and `MAX_PAGES` soundness guard unchanged; "stuck" semantics are preserved (the `BLOCKED` check moved from the bulk page filter to the per-candidate probe). - No changes to the trigger surface, App-authored issue identity, `uses:` SHA pins, or permissions blocks. ## Test plan - `node --test .github/scripts/*.test.cjs` (CI's exact command): 257/257 pass, including 24 in `standards-sync-stuck-automerge-alert.test.cjs` — new coverage asserts the page query never selects `mergeStateStatus`, exactly one phase-2 probe per candidate (no bulk fan-out), transient-error retry on both page fetch and probe, and persistent-error loud failure with no false all-clear. - `actionlint` on the changed workflow: clean. - `biome check` (CI config/scope): clean. - Post-merge follow-through: the phase-2 query shape is mock-validated only — confirm ≥2 consecutive green scheduled runs against the real fleet (the issue's remaining acceptance criterion). ## Related - Refs melodic-software/standards#251 — deployment PR for the failing hourly caller - Refs #224 — comment-events opt-in context named by the issue - Refs #223 — prior fix on the same workflow (App-authored issue identity), deliberately untouched here 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- 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 #217
Related
Summary
admits-comment-events(workflow_call input, defaultfalse) toselect-runner.yml. When a caller sets ittrue,issue_comment,pull_request_review, andpull_request_review_commentjobs become eligible for local (self-hosted) routing, subject to every existing policy/private-repo/fork gate.prefer-self-hostedpolicy path) that had its own hardcoded event list and was otherwise unreachable for comment/review events even with the flag set.Why (and the design this landed on)
Org GitHub Actions spending is capped at $0. The governed policy is
self-hosted-only, so private-repo required checks already run on the fleet -- but comment/review-event jobs were categorically excluded from local routing and fell back to GitHub-hosted runners, which now fail at startup on a billing-limit error whenever free minutes are exhausted (observed on medley'scomment-review-gate.yml).Original-rationale finding:
LOCAL_EVENT_ALLOWLIST(git blame/log through #92, #103, #123, #135) is a plain default-deny allowlist -- "only explicitly reviewed caller event classes may route locally" -- not a comment-specific security ban. Comment/review events were simply never reviewed, not deliberately blocked.Checkout-bearing consumer found, scoped out: Auditing fleet consumers surfaced medley's
claude-assistant.yml, which triggers onissue_comment/pull_request_review_comment, checks out the repository, and runs Claude withcontents: writeand commit signing via the same selector. Under the currentself-hosted-onlypolicy that job is dormant (comment events route hosted, so itsroute == 'self-hosted'gate never passes). A blanket "admit these event classes" change -- the originally scoped approach -- would have made that dormant job self-hosted-eligible as a side effect: comment-triggered checkout + code execution + write perms on the fleet, without a security review of that specific exposure.Landed on a per-caller opt-in instead:
admits-comment-eventsdefaultsfalse, soclaude-assistant.ymlis untouched by this PR and stays exactly as dormant as it is today. Its dormant assist job would only ever go live by its own deliberate future opt-in -- a separate decision requiring its own security review of checkout+write-on-fleet exposure, not a side effect of this change. Recording this explicitly here per the review discussion; will also leave a comment on #217 for anyone who revisitsclaude-assistant.yml's routing later.comment-review-gate.yml(medley#1632) is the reviewed consumer this opt-in is for: verified puregh api/nowgithub-scriptcalls, zero checkout anywhere in that workflow.Consumer follow-up (not in this PR)
medley pins
select-runner.ymlby full commit SHA (currently90f1c54935203fa31b5b3d1f41531228be2c2b7f # v0.6.1). Once this merges and cuts a tag, medley's normal repin process picks up the new SHA; only after that repin cancomment-review-gate.ymladdadmits-comment-events: trueto itsselect-runnercall (the pinned-SHA reusable doesn't know the input yet, so adding it any earlier would be rejected as an unexpected input).Verification
node --test .github/scripts/select-runner.test.cjs-- 112/112 passing, including new coverage:admits-comment-eventstrue/false x the three comment/review events xself-hosted-only/prefer-self-hosted, plus a regression test proving the flag does not widen an unrelated blocked event class (workflow_run)node --test .github/scripts/*.test.cjs-- 247/247 passing repo-wideactionlint .github/workflows/select-runner.yml-- cleannode .github/scripts/render-select-runner-workflow.cjs --check-- generated block in sync withselect-runner.cjs🤖 Generated with Claude Code