Probe the runner pools and lint what CI executes - #8
Merged
Conversation
Groundwork for moving the builds onto larger runners. Neither workflow builds an image; both are safe to land before anything touches the publishing path. runner-smoke.yml (workflow_dispatch only) reports arch, vCPU, RAM and free disk for every label the repo intends to use. Two things make guessing unsafe: larger-runner labels are pool names an org admin created, not anything GitHub predefines - so a typo is indistinguishable from a pool that does not exist, and the job sits queued for 24 hours without a diagnostic - and GitHub publishes no free-space figure for those pools, while images/esp-matter needs ~50 GB for the connectedhomeip tree. lint.yml gates on actionlint and reports hadolint advisorily. actionlint catches exactly the class above: with .github/actionlint.yaml listing the valid labels, an unlisted one is a lint error rather than a hung job. That file doubles as the only written record of which pools exist. hadolint runs --no-fail because these Dockerfiles break some of its rules deliberately. Also quotes $GITHUB_OUTPUT in the six existing tag steps - shellcheck SC2086, surfaced by making actionlint a gate. No behaviour change.
Codex, the code-review workflow and the security review ran over c326ef5; the security review found nothing. What the other two found: - The fact-gathering step reported blanks instead of failures. A fallback after a pipe binds to the last command in it, so `docker buildx version | awk ... || echo unavailable` printed an empty cell, and `df` on a non-existent docker root did the same - on exactly the machines the probe exists to characterise. Every value is now captured separately and defaulted on empty. - The probe listed the eight pool labels a second time while actionlint.yaml claimed to be the only place they are written down. The matrix is now generated from that file, so a pool added for the linter is probed too. - /mnt is reported: on hosted runners it is a separate and usually much larger volume, and the root figure alone answers the disk question wrongly. - `df -h` gained `if: always()` - it is the fallback for a probe that aborted, so skipping it leaves that runner with no data at all. - hadolint is `continue-on-error` and the exit-code plumbing around it is gone. With --no-fail it could only ever go non-zero on a failed image pull, which would have turned the gate red on behalf of the one check documented as unable to. - shellcheck now gates the tracked *.sh files at --severity=error. It ships in the actionlint image, and it already had two real findings: print_color and the unknown-image message spliced arrays into strings (SC2145), so a multi-word message printed wrongly. Both fixed. - actionlint.yaml says what it actually is: the larger-runner pools, not every label, and a check on names that cannot tell whether a pool still exists. - CLAUDE.md no longer states the packages: write rule as universal - it is not, for the two workflows that touch no registry. - README documents both new workflows and the new script; CLAUDE.md assigns it the repo-wide CI topic. Adds scripts/lint.sh so the gate has a local counterpart, pinning the same two image versions as the workflow. Running it is what caught a comment inside a `run:` block being parsed as a shellcheck directive.
workflow_dispatch is only offered for workflows already on the default branch, so until this file is merged there is no way to run it - backwards for a probe meant to answer a question before a decision is made. A push trigger scoped to runner-probe/** branches closes that, and gives forks a way in. It never runs on pull_request, where it would spend money on every push.
README and CLAUDE.md called it workflow_dispatch-only, which was true when they were written and stopped being true one commit later. The runner-probe/** push trigger is the only way to run the probe before it reaches master - precisely the case the probe is for - so leaving it undocumented hides the invocation that matters.
There was a problem hiding this comment.
Pull request overview
This PR adds CI and local tooling to proactively validate runner-pool labels and lint workflows/scripts/Dockerfiles, as groundwork for migrating image builds onto larger runner pools without changing image build/publish behavior.
Changes:
- Add a new
🧹 Lintworkflow (actionlint + shellcheck as gates; hadolint advisory) plus a matching./scripts/lint.shfor local runs. - Add a new
🔎 Runner Smoke Testworkflow that probes all runner-pool labels sourced from.github/actionlint.yaml. - Minor hardening/cleanup: fix shell message formatting in
scripts/build.sh, quote$GITHUB_OUTPUTin workflows, and update documentation.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/lint.sh |
Adds a local lint runner mirroring CI checks (actionlint/shellcheck/hadolint). |
scripts/build.sh |
Fixes multi-word message printing in print_color and available-images output. |
README.md |
Documents the new lint script and runner smoke test; updates repo layout tree. |
CLAUDE.md |
Updates repository conventions to include the new lint/probe workflows and runner-pool guidance. |
.github/workflows/runner-smoke.yml |
Adds a workflow to probe runner pools from .github/actionlint.yaml and report capabilities. |
.github/workflows/platformio.yml |
Quotes $GITHUB_OUTPUT writes for robustness. |
.github/workflows/lint.yml |
Adds CI lint workflow gating workflow/shell scripts and advisory Dockerfile linting. |
.github/workflows/esp-idf.yml |
Quotes $GITHUB_OUTPUT writes for robustness. |
.github/actionlint.yaml |
Central list of runner-pool labels for actionlint validation and runner-smoke probing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Groundwork for moving the image builds onto larger runners. Nothing here builds or
publishes an image, so it can land before anything touches the publishing path.
Why a runner probe
Larger-runner labels are not predefined by GitHub — each is the name of a pool an
org admin created. A
runs-onnaming a pool that is unreachable does not fail: itqueues, for up to 24 hours, and
timeout-minutesnever fires because it startscounting at assignment rather than at queueing. For
esp-matter-buildthat wouldmean a six-hour job silently pinned to a branch.
That is not hypothetical — it happened on the first run of this workflow. Both
standard runners finished in two seconds while all eight pool labels sat queued;
the pools existed but the repository had no access to their group. The probe cost
nothing to find that out, because no paid leg ever started.
Once access was granted, the same run reported this:
ubuntu-latest-4coreubuntu-latest-8coreubuntu-latest-16coreubuntu-latest-32coreubuntu-latest-4core-armubuntu-latest-8core-armubuntu-latest-16core-armubuntu-latest-32core-armFree space is not what the pool configuration advertises — 372 GB on a pool
described as a 300 GB SSD — which settles the open question for
esp-matter-build:8-core, where ~50 GB of connectedhomeip tree leaves a fivefold margin, rather than
4-core, where it leaves about half of one.
The matrix is generated from
.github/actionlint.yamlrather than repeated in theworkflow, so a pool added for the linter is probed as well. Triggers are
workflow_dispatchplus a push torunner-probe/**; the second exists becauseworkflow_dispatchis only offered for workflows already on the default branch,which would make a probe unusable exactly when it is needed. It never runs on
pull_request.Why a lint workflow
actionlintis what turns the failure above into a red check rather than a hungjob: with the pools listed in
.github/actionlint.yaml, an unlisted label is alint error. Worth being precise about its limit — it compares names against that
list and cannot ask GitHub whether a pool is still alive; the probe is what
answers that.
shellcheckships inside the same image and now gates the tracked*.shfiles at--severity=error. It had two real findings on arrival:print_colorand theunknown-image message both spliced an array into a string (SC2145), so any
multi-word message printed wrongly. Both fixed here.
hadolintis advisory —--no-failpluscontinue-on-error. These Dockerfilesbreak some of its rules deliberately, and it reports
sourceas undefined withoutseeing that the ESP images set
SHELL ["/bin/bash", "-c"].continue-on-erroriswhat keeps that promise honest: without it a Docker Hub rate limit would turn the
gate red on behalf of the one check documented as unable to.
scripts/lint.shruns the same three checks locally, pinning the same imageversions. Its first run caught a comment inside a
run:block being parsed as ashellcheck directive in this very PR.
Verification
./scripts/lint.sh— green: actionlint clean, shellcheck clean, hadolintadvisory output only
scripts/build.sh --helpand its unknown-image path run to confirm the SC2145fixes print correctly
Reviewed by
codex-review(xhigh), thecode-reviewworkflow (high) andsecurity-review.Security found nothing. The other two found twelve defects, all applied — the
substantial ones being blank cells printed instead of failures (a fallback after a
pipe binds to the last command in it, so a runner without
docker buildxreportedan empty string), the label list existing in two places while claiming to exist in
one, and the hadolint exit-code plumbing described above. A second Codex pass over
the full range caught the docs still calling the probe dispatch-only after the
branch trigger was added.