add OD_CI_CONTROL_FALLBACK escape hatch for the control runner profile - #6290
add OD_CI_CONTROL_FALLBACK escape hatch for the control runner profile#6290dpeterson01 wants to merge 2 commits into
Conversation
Route control-profile CI jobs to GitHub-hosted runners when a maintainer sets the OD_CI_CONTROL_FALLBACK repo variable, so an unavailable self-hosted control pool no longer wedges required checks. Off by default (variable unset), and independent of OD_CI_RUNNER_MODE so it only touches the control profile. Reads a repo variable via the job env, so it needs no elevated workflow-token permissions.
|
Thanks @dpeterson01 — the least-privilege split here is nicely scoped, and the rationale in the description makes the intent easy to follow. Before pool review, could you add a short What users will see section and convert Surface area to the template checklist format with the applicable box(es) ticked? Right now the CI details are present, but those two template fields are still effectively incomplete. |
|
Both addressed: added a What users will see section, and switched Surface area to the template checklist (ticked CLI / env var for the new |
| "decision": { | ||
| "schema_version": 1, | ||
| "mode": mode, | ||
| "control_fallback": control_fallback, |
There was a problem hiding this comment.
Adding decision.control_fallback changes the runners.py output schema, but the owning workflow contract test still asserts exact equality on that object in e2e/tests/packaged-smoke-workflow.test.ts:1221, :1250, and :1261. On this PR head, python3 .github/scripts/runners.py now emits decision={"schema_version":1,"mode":"default","control_fallback":false}, which no longer matches the existing { schema_version: 1, mode: "default" } expectation, so this PR introduces a concrete regression in the current runner-profile test suite and still leaves the new fallback path unverified. Please update that test section to include the new field in the expected decision payload and add a truthy OD_CI_CONTROL_FALLBACK case that proves only runs_on.control flips to ubuntu-24.04 while the other runner tiers remain mode-driven.
Address review on nexu-io#6290: the decision payload now includes control_fallback, so update the exact-equality expectations in the runner-profile contract test and add a truthy OD_CI_CONTROL_FALLBACK case proving only runs_on.control flips to GitHub-hosted while the other tiers stay mode-driven.
|
Good catch, thanks. Fixed in |
nettee
left a comment
There was a problem hiding this comment.
@dpeterson01 I re-checked the updated head and the earlier runner-contract gap is closed: the workflow now passes OD_CI_CONTROL_FALLBACK, runners.py records decision.control_fallback, and the packaged workflow contract test expectations now cover both the new decision field and the hosted-control fallback path. I also spot-checked python3 .github/scripts/runners.py with and without OD_CI_CONTROL_FALLBACK=1, and the outputs match the intended control-only override. I couldn't run e2e/tests/packaged-smoke-workflow.test.ts in this prepared worktree because node_modules/vitest are not installed here. Nice follow-up on a narrowly scoped CI escape hatch.
Refs #6289.
Why
When the self-hosted
controlpool (od-persistent-ci) is unavailable, thecontrol-profile required checks (Detect validation scopes,Static gate,Validate workspace,Runtime summary) stall or cancel and wedge approved PRs (#6076, #6079). Today the only lever isOD_CI_RUNNER_MODE, but its non-default modes also movegeneral_mediumand the hot paths, so there's no clean way to move justcontrolto hosted.What users will see
No product or end-user change. This adds a maintainer-facing CI control: when the self-hosted
controlrunner pool is unavailable, setting theOD_CI_CONTROL_FALLBACKrepo variable routes thecontrol-profile CI jobs to GitHub-hosted runners so PRs stop wedging on cancelled required checks. With the variable unset, CI behaves exactly as it does today.What it changes
A dedicated repo variable,
OD_CI_CONTROL_FALLBACK. When a maintainer sets it truthy (1/true/yes/on/hosted),controljobs run on GitHub-hosted runners. It's off by default and independent ofOD_CI_RUNNER_MODE, so it touches only thecontrolprofile. The resolveddecisionoutput now recordscontrol_fallbackso the choice is visible in logs.Decision and rationale: least-privilege, no elevated token permissions
I considered automatic detection (have the resolve job query runner health and self-heal), but listing self-hosted runners requires
GET /repos/{owner}/{repo}/actions/runners, which needs theadministrationpermission. This workflow's token only hasactions: read. Broadening the CI token would mean a standing privilege escalation on every run and a larger attack surface, so instead this uses a maintainer-flippable repo variable that needs no token permissions at all. If you'd later prefer automatic detection and are fine grantingadministration: read, that's a clean follow-up. I kept this change minimal.Surface area
apps/weborapps/desktop(including Electron menu bar)odsubcommand or flag, newtools-dev/tools-packflag, or newOD_*env var/api/*endpoint, new SSE event, or changed shape inpackages/contractsskills/,design-systems/,design-templates/, orcraft/, or change to the skills protocolTRANSLATIONS.mdfor the locale workflow)package.json(dependenciesordevDependencies)The new box is
OD_CI_CONTROL_FALLBACK, a CI/Actions repo variable read by.github/scripts/runners.py.Bug fix verification
Ran
runners.pylocally across the relevant cases:controlstays self-hosted (unchanged default)OD_CI_CONTROL_FALLBACK=1:controlbecomesubuntu-24.04=hostedwithOD_CI_RUNNER_MODE=performance: onlycontrolflips;general_mediumand the hot paths keep their mode values;decision.control_fallbackistrueValidation
Verified
runners.pyacross the four cases above, andci.ymlparses as valid YAML. There's no test harness for.github/scriptstoday; I'm happy to add one if you have a preferred runner.