Add self-hosted fail-over for the ubuntu-latest CI legs#1274
Conversation
📝 WalkthroughWalkthroughAdds a new Bash script that polls the GitHub Actions Jobs API to decide whether a self-hosted fallback should defer, self-run, or flag an error, and introduces a new ChangesUbuntu Fallback CI Job
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant UbuntuFallbackJob
participant CiWaitScript as ci-wait-or-fallback.sh
participant GitHubJobsAPI
participant PrimaryJob
UbuntuFallbackJob->>CiWaitScript: poll leg status (ut, st-sim-a2a3, st-sim-a5)
CiWaitScript->>GitHubJobsAPI: query job status/conclusion
GitHubJobsAPI-->>CiWaitScript: status/conclusion
alt primary in_progress or completed
CiWaitScript-->>UbuntuFallbackJob: DEFER
else start timeout reached
CiWaitScript-->>UbuntuFallbackJob: SELFRUN or RENAME_ERROR
end
UbuntuFallbackJob->>UbuntuFallbackJob: install deps and run local tests if SELFRUN
UbuntuFallbackJob->>UbuntuFallbackJob: warn if RENAME_ERROR/ERROR
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces the .github/scripts/ci-wait-or-fallback.sh script, which implements a fallback mechanism to run tests on self-hosted runners if GitHub-hosted primary jobs stall. The feedback highlights a critical bug where persistent API fetch failures could cause an infinite loop and bypass the timeout check, potentially hogging runners indefinitely. Additionally, improvements are suggested to enhance error visibility by using curl -sSf and to optimize performance by reading JSON directly from standard input in the field function rather than passing it as an argument.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| while true; do | ||
| JSON="$(fetch || true)" | ||
| if [ -z "${JSON}" ]; then | ||
| log "jobs API fetch failed; retrying in ${POLL}s (elapsed=${elapsed}s)" | ||
| sleep "${POLL}"; elapsed=$((elapsed + POLL)); continue | ||
| fi | ||
|
|
||
| STATUS="$(field "${JSON}" "${PRIMARY}" 'status')" | ||
| CONCL="$(field "${JSON}" "${PRIMARY}" 'conclusion')" | ||
|
|
||
| if [ -n "${STATUS}" ] && [ "${STATUS}" != "null" ]; then | ||
| seen=1 | ||
| case "${STATUS}" in | ||
| in_progress|completed) | ||
| # GitHub is running or has finished the primary. The primary is the | ||
| # required check and gates itself (its pass/fail/skip is authoritative), | ||
| # so this convenience job has nothing to do. | ||
| log "primary status=${STATUS} (conclusion=${CONCL:-n/a}) -> DEFER (primary gates)" | ||
| echo "DEFER"; exit 0 ;; | ||
| *) | ||
| # queued / waiting / pending / requested -> not started yet. | ||
| : ;; | ||
| esac | ||
| fi | ||
|
|
||
| # Primary is queued or not yet present. Enforce the start deadline. | ||
| if [ "${elapsed}" -ge "${START_TIMEOUT}" ]; then | ||
| if [ "${seen}" -eq 0 ]; then | ||
| # Never once observed. Distinguish "stall" from "matching broke". | ||
| if [ "${EXPECT_FAST}" -eq 1 ]; then | ||
| printf '::error::Fallback never observed primary job %s within %ss. It has no `needs` and should appear immediately -- the job name almost certainly drifted (matrix os/python change). Update the primary name in ci.yml.\n' "${PRIMARY}" "${START_TIMEOUT}" >&2 | ||
| echo "RENAME_ERROR"; exit 0 | ||
| fi | ||
| DC_STATUS="$(field "${JSON}" 'detect-changes' 'status')" | ||
| if [ "${DC_STATUS}" = "completed" ]; then | ||
| printf '::error::Fallback never observed primary job %s though detect-changes has completed -- likely a renamed matrix leg or a pruned primary. Refusing to self-run blindly.\n' "${PRIMARY}" >&2 | ||
| echo "RENAME_ERROR"; exit 0 | ||
| fi | ||
| # detect-changes has NOT completed (stuck or still running on GitHub), so | ||
| # the primary was never created. This fallback already confirmed via its | ||
| # own arch-diff that the work is needed, so a stalled detect-changes is | ||
| # just another GitHub scheduling stall -- take over rather than wait | ||
| # forever for a primary that will never appear. | ||
| printf '::warning::Primary %s never appeared and detect-changes has not completed (status=%s) within %ss -- detect-changes itself appears stalled; self-running on this self-hosted runner.\n' "${PRIMARY}" "${DC_STATUS:-absent}" "${START_TIMEOUT}" >&2 | ||
| echo "SELFRUN"; exit 0 | ||
| fi | ||
| printf '::warning::GitHub did not start primary job %s within %ss -- self-running the tests on this self-hosted runner.\n' "${PRIMARY}" "${START_TIMEOUT}" >&2 | ||
| echo "SELFRUN"; exit 0 | ||
| fi | ||
|
|
||
| sleep "${POLL}"; elapsed=$((elapsed + POLL)) | ||
| done |
There was a problem hiding this comment.
There is a critical bug in the loop structure: if fetch fails persistently (e.g., due to a bad token, rate limiting, or a GitHub outage), the continue statement on line 89 skips the rest of the loop, including the timeout check on line 111. This causes the script to loop forever, hogging the self-hosted runner indefinitely.
To fix this, we should:
- Move the timeout check to the top of the loop so it is always evaluated.
- Handle the case where
JSONis empty due to persistent fetch failures by exiting with a safe fallback token (e.g.,FETCH_ERROR) instead of self-running blindly. - Preserve the last known good state of the jobs list in
JSONacross transient fetch failures. - Optimize the loop to extract both
statusandconclusionin a singlepython3invocation, reducing process overhead.
while true; do
if [ "${elapsed}" -ge "${START_TIMEOUT}" ]; then
if [ "${seen}" -eq 0 ]; then
# Never once observed. Distinguish "stall" from "matching broke".
if [ "${EXPECT_FAST}" -eq 1 ]; then
printf '::error::Fallback never observed primary job %s within %ss. It has no `needs` and should appear immediately -- the job name almost certainly drifted (matrix os/python change). Update the primary name in ci.yml.\n' "${PRIMARY}" "${START_TIMEOUT}" >&2
echo "RENAME_ERROR"; exit 0
fi
if [ -z "${JSON:-}" ]; then
printf '::error::Fallback failed to fetch jobs API within %ss. Refusing to self-run blindly.\n' "${START_TIMEOUT}" >&2
echo "FETCH_ERROR"; exit 0
fi
DC_STATUS="$(printf '%s' "${JSON}" | field 'detect-changes' 'status')"
if [ "${DC_STATUS}" = "completed" ]; then
printf '::error::Fallback never observed primary job %s though detect-changes has completed -- likely a renamed matrix leg or a pruned primary. Refusing to self-run blindly.\n' "${PRIMARY}" >&2
echo "RENAME_ERROR"; exit 0
fi
# detect-changes has NOT completed (stuck or still running on GitHub), so
# the primary was never created. This fallback already confirmed via its
# own arch-diff that the work is needed, so a stalled detect-changes is
# just another GitHub scheduling stall -- take over rather than wait
# forever for a primary that will never appear.
printf '::warning::Primary %s never appeared and detect-changes has not completed (status=%s) within %ss -- detect-changes itself appears stalled; self-running on this self-hosted runner.\n' "${PRIMARY}" "${DC_STATUS:-absent}" "${START_TIMEOUT}" >&2
echo "SELFRUN"; exit 0
fi
printf '::warning::GitHub did not start primary job %s within %ss -- self-running the tests on this self-hosted runner.\n' "${PRIMARY}" "${START_TIMEOUT}" >&2
echo "SELFRUN"; exit 0
fi
NEW_JSON="$(fetch || true)"
if [ -z "${NEW_JSON}" ]; then
log "jobs API fetch failed; retrying in ${POLL}s (elapsed=${elapsed}s)"
sleep "${POLL}"; elapsed=$((elapsed + POLL)); continue
fi
JSON="${NEW_JSON}"
# Extract status and conclusion in a single python invocation to avoid process overhead
read -r STATUS CONCL <<< "$(printf '%s' "${JSON}" | python3 -c '
import json, sys
try:
data = json.load(sys.stdin)
except Exception as e:
print(f"JSON parse error: {e}", file=sys.stderr)
sys.exit(0)
name = sys.argv[1]
for j in data.get("jobs", []):
if j.get("name") == name:
print(f"{j.get(\"status\") or \"\"} {j.get(\"conclusion\") or \"\"}")
break
' "${PRIMARY}")" || true
if [ -n "${STATUS}" ] && [ "${STATUS}" != "null" ]; then
seen=1
case "${STATUS}" in
in_progress|completed)
# GitHub is running or has finished the primary. The primary is the
# required check and gates itself (its pass/fail/skip is authoritative),
# so this convenience job has nothing to do.
log "primary status=${STATUS} (conclusion=${CONCL:-n/a}) -> DEFER (primary gates)"
echo "DEFER"; exit 0 ;;
*)
# queued / waiting / pending / requested -> not started yet.
: ;;
esac
fi
sleep "${POLL}"; elapsed=$((elapsed + POLL))
doneThere was a problem hiding this comment.
Fixed in eae1c30. The start-deadline check is now evaluated on every iteration (including when the fetch fails), so a persistent API failure can no longer skip it and loop forever. When the API is unreachable for the whole START_TIMEOUT and the primary was never observed, the script emits a new FETCH_ERROR token and refuses to self-run blindly. curl also gained --connect-timeout 10 --max-time 30, so a stalled connection can't hang past a poll interval either.
| fetch() { | ||
| curl -sf \ | ||
| -H "Authorization: Bearer ${GITHUB_TOKEN}" \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| -H "X-GitHub-Api-Version: 2022-11-28" \ | ||
| "${API}" | ||
| } |
There was a problem hiding this comment.
Improve observability of API fetch failures by using curl -sSf instead of curl -sf. The -S (--show-error) flag ensures that if the request fails (e.g., due to network issues, DNS resolution failure, or invalid credentials), curl will print the error message to stderr, making debugging much easier while remaining silent on success.
| fetch() { | |
| curl -sf \ | |
| -H "Authorization: Bearer ${GITHUB_TOKEN}" \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| "${API}" | |
| } | |
| fetch() { | |
| curl -sSf \ | |
| -H "Authorization: Bearer ${GITHUB_TOKEN}" \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| "${API}" | |
| } |
There was a problem hiding this comment.
Done in eae1c30 — fetch() now uses curl -sSf --connect-timeout 10 --max-time 30.
| field() { # field <json> <job-name> <field> (e.g. status, conclusion) | ||
| # python3 (always present on these runners) instead of jq (often absent). | ||
| printf '%s' "$1" | python3 -c ' | ||
| import json, sys | ||
| try: | ||
| data = json.load(sys.stdin) | ||
| except Exception: | ||
| sys.exit(0) | ||
| name, key = sys.argv[1], sys.argv[2] | ||
| for j in data.get("jobs", []): | ||
| if j.get("name") == name: | ||
| v = j.get(key) | ||
| print("" if v is None else v) | ||
| break | ||
| ' "$2" "$3" | ||
| } |
There was a problem hiding this comment.
Avoid passing the potentially large JSON string as a command-line argument to the field function. Instead, redefine the function to read the JSON directly from standard input. This is more efficient and avoids copying large strings in memory. Additionally, we can log JSON parsing errors to stderr to improve observability when the API returns invalid JSON or an HTML error page.
| field() { # field <json> <job-name> <field> (e.g. status, conclusion) | |
| # python3 (always present on these runners) instead of jq (often absent). | |
| printf '%s' "$1" | python3 -c ' | |
| import json, sys | |
| try: | |
| data = json.load(sys.stdin) | |
| except Exception: | |
| sys.exit(0) | |
| name, key = sys.argv[1], sys.argv[2] | |
| for j in data.get("jobs", []): | |
| if j.get("name") == name: | |
| v = j.get(key) | |
| print("" if v is None else v) | |
| break | |
| ' "$2" "$3" | |
| } | |
| field() { # field <job-name> <field> (e.g. status, conclusion) | |
| # python3 (always present on these runners) instead of jq (often absent). | |
| python3 -c ' | |
| import json, sys | |
| try: | |
| data = json.load(sys.stdin) | |
| except Exception as e: | |
| print(f"JSON parse error: {e}", file=sys.stderr) | |
| sys.exit(0) | |
| name, key = sys.argv[1], sys.argv[2] | |
| for j in data.get("jobs", []): | |
| if j.get("name") == name: | |
| v = j.get(key) | |
| print("" if v is None else v) | |
| break | |
| ' "$1" "$2" | |
| } |
There was a problem hiding this comment.
Left as-is for now. The jobs payload is small here (this workflow has ~18 jobs, well under the 100/page cap), so passing it as an argument isn't a memory concern and the current form keeps the helper simple. Happy to switch to stdin if you feel strongly.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
764-839: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTemplate-injection pattern flagged by zizmor: step outputs interpolated directly into shell via
${{ }}.Lines 771, 772, 788-790, and 835 splice
steps.arch.outputs.*/steps.decide.outputs.*directly into therun:script text rather than passing them throughenv:. Today these values are constrained to known literals (true/false,DEFER/SELFRUN/RENAME_ERROR/SKIP) produced by this workflow's own steps, so it isn't currently exploitable, but it's the same syntactic pattern responsible for real GitHub Actions injection CVEs when the source of the value changes later. Passing viaenv:and referencing"$VAR"in the script removes the class of risk entirely and is a trivial change.🔧 Example fix for the "Decide defer vs self-run" step
- name: Decide defer vs self-run id: decide env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + A2A3_CHANGED: ${{ steps.arch.outputs.a2a3 }} + A5_CHANGED: ${{ steps.arch.outputs.a5 }} run: | poll() { .github/scripts/ci-wait-or-fallback.sh "$1" 600 "$2" > "$3" 2> "$3.log" & } poll "ut (ubuntu-latest, 3.10)" 1 ut.dec - [ "${{ steps.arch.outputs.a2a3 }}" = "true" ] && poll "st-sim-a2a3 (ubuntu-latest, 3.10)" 0 a2a3.dec - [ "${{ steps.arch.outputs.a5 }}" = "true" ] && poll "st-sim-a5 (ubuntu-latest, 3.10)" 0 a5.dec + [ "$A2A3_CHANGED" = "true" ] && poll "st-sim-a2a3 (ubuntu-latest, 3.10)" 0 a2a3.dec + [ "$A5_CHANGED" = "true" ] && poll "st-sim-a5 (ubuntu-latest, 3.10)" 0 a5.decApply the same
env:-based pattern to the "Announce stalled legs" and "Warn on unresolved legs" steps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 764 - 839, The “Decide defer vs self-run”, “Announce stalled legs”, and “Warn on unresolved legs” steps are interpolating step outputs directly into shell with `${{ }}`, which triggers the template-injection pattern. Move the `steps.arch.outputs.*` and `steps.decide.outputs.*` values into `env:` for those steps, then reference the corresponding shell variables inside the `run:` blocks in the `decide`, `Announce stalled legs`, and `Warn on unresolved legs` steps. This keeps the logic the same while removing direct expression expansion from the script text.Source: Linters/SAST tools
793-829: 🚀 Performance & Scalability | 🔵 TrivialWorst-case timing: verify the 90-minute job timeout covers sequential self-runs of all three legs.
The ~10 min polling window happens in parallel (background
poll()+wait), but once decided,ut/a2a3/a5self-run steps execute sequentially in this single job. If GitHub stalls all three legs simultaneously, cumulative dependency install + unit tests + cmake/ctest + multiple sim pytest invocations could approach or exceed the 90-minutetimeout-minutesbudget, silently truncating a legitimate self-run rather than completing it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 793 - 829, The self-run path in the CI workflow can exceed the single job’s 90-minute timeout when multiple legs are selected, since the dependency install and the ut, a2a3, and a5 test steps run sequentially in the same job. Update the workflow logic around the self-run steps so the timeout budget is sufficient for the worst-case combined runtime, either by increasing the job timeout or splitting the self-run legs into separate jobs/steps keyed off steps.decide outputs, and keep the relevant gating on the ut, st-sim-a2a3, and st-sim-a5 steps consistent.
748-761: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffArch-detection regex duplicated from
detect-changes.The comment notes this is intentionally inlined so a
detect-changesstall can't block the fallback, which is a reasonable tradeoff. Worth keeping in mind that theNON_CODE/A5_ONLY/A2A3_ONLYpatterns here and indetect-changeswill need to be updated in lockstep going forward, or the fallback's own path filtering can silently diverge from the required check's filtering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 748 - 761, The arch-detection path filters in the fallback step are duplicated from detect-changes, so update both locations together or extract the shared NON_CODE/A5_ONLY/A2A3_ONLY patterns into a single reusable source; make sure the logic in the arch detection step stays identical to the detect-changes job by referencing Detect arch changes and detect-changes when editing these regexes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/ci-wait-or-fallback.sh:
- Around line 60-66: The fetch() helper in ci-wait-or-fallback.sh uses curl
without any timeout, so add explicit connection and overall request time limits
to the curl invocation in fetch() to prevent indefinite hangs. Keep the existing
headers and behavior, but make sure the request can fail fast so the
START_TIMEOUT budget in the surrounding logic is still respected and the
DEFER/SELFRUN decision path can continue.
- Line 53: The jobs lookup in ci-wait-or-fallback.sh only requests a single page
via the API URL, so jobs beyond the first 100 can be missed. Update the job
collection logic around the API fetch and field() lookup to paginate through all
pages (by following the Link header or incrementing page until no jobs remain),
merge all returned jobs, and then search the full set so the primary job is
found correctly.
In @.github/workflows/ci.yml:
- Around line 743-746: The Checkout repository step in the CI workflow should
disable persisted git credentials by setting persist-credentials to false on
actions/checkout@v5. Update that checkout block so the token is not written into
.git/config, while keeping the existing fetch-depth setting and leaving the
later polling step to use the GITHUB_TOKEN from env as intended.
- Around line 764-780: The decision helper in the “Decide defer vs self-run”
step only treats missing .dec files as SKIP, so an existing-but-empty file from
ci-wait-or-fallback.sh can produce an empty decision and bypass the ERROR
handling. Update get() to treat empty output the same as a failure state,
mapping both missing and empty decision files to ERROR before the UT/A2A3/A5
outputs are written.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 764-839: The “Decide defer vs self-run”, “Announce stalled legs”,
and “Warn on unresolved legs” steps are interpolating step outputs directly into
shell with `${{ }}`, which triggers the template-injection pattern. Move the
`steps.arch.outputs.*` and `steps.decide.outputs.*` values into `env:` for those
steps, then reference the corresponding shell variables inside the `run:` blocks
in the `decide`, `Announce stalled legs`, and `Warn on unresolved legs` steps.
This keeps the logic the same while removing direct expression expansion from
the script text.
- Around line 793-829: The self-run path in the CI workflow can exceed the
single job’s 90-minute timeout when multiple legs are selected, since the
dependency install and the ut, a2a3, and a5 test steps run sequentially in the
same job. Update the workflow logic around the self-run steps so the timeout
budget is sufficient for the worst-case combined runtime, either by increasing
the job timeout or splitting the self-run legs into separate jobs/steps keyed
off steps.decide outputs, and keep the relevant gating on the ut, st-sim-a2a3,
and st-sim-a5 steps consistent.
- Around line 748-761: The arch-detection path filters in the fallback step are
duplicated from detect-changes, so update both locations together or extract the
shared NON_CODE/A5_ONLY/A2A3_ONLY patterns into a single reusable source; make
sure the logic in the arch detection step stays identical to the detect-changes
job by referencing Detect arch changes and detect-changes when editing these
regexes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfc29d2a-7e33-42c0-bf1a-b0c3643f8627
📒 Files selected for processing (2)
.github/scripts/ci-wait-or-fallback.sh.github/workflows/ci.yml
When GitHub's hosted runner pool is congested, the ubuntu-latest CI legs (ut / st-sim-a2a3 / st-sim-a5) can queue for a long time. Add a single non-required, self-hosted ubuntu-fallback job that covers that rare stall without changing how CI gates: - The GitHub primaries stay the required checks and gate on their own results. - ubuntu-fallback polls the three primaries in parallel and, per leg, DEFERS the moment GitHub starts running it (exiting in seconds) and only self-runs a leg GitHub never scheduled. On such a stall the required primary stays pending; a maintainer admin-merges using this job's green self-run. - Runs for every PR including forks (contributors push from their own forks and are trusted on the self-hosted runner, consistent with the existing onboard jobs). Existing jobs are unchanged; this only adds the actions:read permission and the new job. Decision logic lives in .github/scripts/ci-wait-or-fallback.sh (DEFER / SELFRUN / RENAME_ERROR / FETCH_ERROR; python3, no jq; bounded curl and a poll deadline so a stalled API can never hang the runner). Activation: register a self-hosted runner labelled 'fallback' carrying GCC 15 / gtest / graphviz. The job is non-required, so without such a runner it stays queued but blocks nothing.
When GitHub's hosted runner pool is congested, the ubuntu-latest CI legs (ut / st-sim-a2a3 / st-sim-a5) can queue for a long time. This adds a single non-required, self-hosted
ubuntu-fallbackjob that covers that rare stall without changing how CI gates.What it does
ubuntu-fallbackpolls the three primaries in parallel and, per leg, DEFERS the moment GitHub starts running it (exiting in seconds, so the self-hosted runner is free in the common case) and only self-runs a leg GitHub never scheduled.Footprint
actions: readpermission and the new job are added..github/scripts/ci-wait-or-fallback.sh(DEFER/SELFRUN/RENAME_ERROR/FETCH_ERROR; python3, no jq).curlis bounded (--connect-timeout/--max-time) and the poll deadline is evaluated every iteration, so a stalled API can never hang the runner.Runs for all PRs (including forks)
Contributors push from their own forks, so the job intentionally runs on fork PRs too — consistent with the existing self-hosted onboard jobs, which already do the same. The persisted-token exposure is identical to those jobs and the workflow's token is read-only.
Activation
Register a self-hosted runner labelled
fallbackcarrying GCC 15 / gtest / graphviz. The job is non-required, so without such a runner it stays queued but blocks nothing.