feat(select-runner): admit capped review tier label for strict routing - #123
Merged
Conversation
Add melodic-review-ubuntu-24.04-x64 to the self-hosted-only allowlist so claude-review callers can route to the dedicated capped review tier (provisioning#135). The selector job itself stays on the default tier, so selecting a review job never consumes the review tier's small capacity. Exact-set admission is preserved: the dormant build tier and any other namespace-matching label are still rejected. Regenerate the embedded selector and extend the fleet-label agreement and selector tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt
|
Claude encountered an error after 5m 3s —— 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. |
kyle-sexton
added a commit
that referenced
this pull request
Jul 22, 2026
…224) Closes #217 ## Related - #209 / #212 (github-script port pattern this issue's fix depends on for consumers) - medley#1628 report Theme 1 (the exit-127 defect class that motivates never routing a gh-CLI-dependent job self-hosted without porting it first) - medley#1632 (comment-review-gate.yml github-script port -- the reviewed no-checkout consumer this change enables) ## Summary - Adds `admits-comment-events` (workflow_call input, default `false`) to `select-runner.yml`. When a caller sets it `true`, `issue_comment`, `pull_request_review`, and `pull_request_review_comment` jobs become eligible for local (self-hosted) routing, subject to every existing policy/private-repo/fork gate. - Fixes a second, independent event enumeration in the "Mint read-only observer token" step (`prefer-self-hosted` policy 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's `comment-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 on `issue_comment`/`pull_request_review_comment`, checks out the repository, and runs Claude with `contents: write` and commit signing via the same selector. Under the current `self-hosted-only` policy that job is dormant (comment events route hosted, so its `route == '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-events` defaults `false`, so `claude-assistant.yml` is 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 revisits `claude-assistant.yml`'s routing later. `comment-review-gate.yml` (medley#1632) is the reviewed consumer this opt-in is for: verified pure `gh api`/now `github-script` calls, zero checkout anywhere in that workflow. ## Consumer follow-up (not in this PR) medley pins `select-runner.yml` by full commit SHA (currently `90f1c54935203fa31b5b3d1f41531228be2c2b7f # v0.6.1`). Once this merges and cuts a tag, medley's normal repin process picks up the new SHA; only *after* that repin can `comment-review-gate.yml` add `admits-comment-events: true` to its `select-runner` call (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-events` true/false x the three comment/review events x `self-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-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` 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
Summary
melodic-review-ubuntu-24.04-x64in theself-hosted-onlyselector allowlist (SELF_HOSTED_ONLY_LABELS) soclaude-reviewcallers can route to the dedicated capped review tier.melodic-ubuntu-24.04-x64tier, so selecting a review job never consumes the review tier's small (max 2) capacity.select-runner.ymlfrom source.melodic-build-ubuntu-24.04-x64tier is still rejected asunapproved-label— exact-set admission, not the broad namespace, is the strict-routing boundary.Why
Step 3 of the review-tier governance path. The tier's scale set and label are provisioned but dormant and must not be requested as a raw label until the selector admits it.
prefer-self-hostedalready accepts any managed label via liveness routing, but the org runsself-hosted-only, whose exact allowlist rejected the new label. Admitting it here makes the tier routable while keeping fail-closed strict routing intact (unknown and other tiered labels still throwunapproved-label).Scope note — routable, not yet per-workflow selectable
Admitting the label makes it routable but is not by itself sufficient for a consumer to select the review tier per workflow: the governed selector's
self-hosted-labelinput resolves the org-wideCI_SELF_HOSTED_LABEL(the default tier), and runner-policy admits no other selector-input expression. Routingclaude-reviewto the review tier while a repo's other self-hosted jobs stay on the default tier needs a follow-up governed selector input in standards plus a review-tier label variable. That is the standards half of this workstream and the Campaign A rework.Validation
node --test .github/scripts/*.test.cjs: 251 passed (2 new).render-select-runner-workflow.cjs --check: embedded selector in sync with source.Related
No linked issue.
🤖 Generated with Claude Code
https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt