Continuous integration¶
How Shaken Fist's CI is put together: the workflows, the merge queue, the automated jobs that open their own pull requests, and the bot commands available on a PR.
GitHub Actions workflows¶
Every workflow in .github/workflows/:
| Workflow | Purpose | Trigger |
|---|---|---|
functional-tests.yml |
Main CI: lint, unit tests, functional tests, credential scanning, and the automated reviewer, delinter and exception fixer jobs. The functional jobs deploy nested test clusters via the shakenfist.shakenfist Ansible collection (shakenfist/deploy/collection/), driven by the reusable smoke-cluster workflow in the shakenfist/actions repository |
PR, merge_group |
docs-tests.yml |
Build and test documentation | PR touching docs/** or mkdocs.yml |
agent-context.yml |
Lint the agent context -- AGENTS.md and .claude/ -- for malformed frontmatter, smuggled instructions, embedded credentials and dangerous hook configuration, by running the skillsaw pre-commit hook. Advisory: see Branch Protection |
PR or push to develop touching the agent context, .pre-commit-config.yaml or the workflow itself |
mermaid-lint.yml |
Render every tracked mermaid diagram and fail on one that does not parse (runs tools/mermaid-lint.sh in the pinned mermaid-cli container, on a debian-13-docker runner because it needs a docker daemon). Advisory: see Branch Protection |
PR or push to develop touching markdown, the script, its exclude file or the workflow itself |
code-formatting.yml |
Whole-tree formatting sweep | Daily schedule, manual, self-test PR |
codeql-analysis.yml |
CodeQL static analysis | Push, PR, weekly schedule |
pin-indirect-dependencies.yml |
Reconcile pinned indirect dependencies, adding new ones and removing obsolete ones (runs tools/pin-indirect-dependencies.sh) |
Daily schedule, PR self-test |
renovate.yml |
Self-hosted Renovate dependency updates | Hourly schedule, manual |
ci-headroom-health.yml |
Harvest the ten newest merge runs' CI headroom bundles and assert the probe which wrote them is still working (runs tools/ci_headroom_health.py; see Is the instrument still alive?). The one headroom check allowed to fail, because it is scheduled: it must never gain a PR or merge_group trigger |
Weekly schedule (Mondays 07:30 UTC), manual |
export-repo-config.yml |
Export GitHub repo settings to version control, via a shared reusable workflow in the actions/ repository |
Daily schedule |
pr-re-review.yml |
Re-review PR on bot command | @shakenfist-bot please re-review |
pr-retest.yml |
Re-run the functional tests on bot command | @shakenfist-bot please retest |
pr-fix-tests.yml |
Fix test failures on bot command | @shakenfist-bot please attempt to fix |
test-drift-fix.yml |
Unit test fixer (called by pr-fix-tests.yml) |
workflow_call, workflow_dispatch |
issue-fix.yml |
Triage open issues, propose a fix as a draft PR | workflow_dispatch |
merge-failure-triage.yml |
Triage a failed merge queue run: PR-caused or systemic, record the occurrence, post a verdict | workflow_run (a failed Functional tests merge group run), workflow_dispatch |
scheduled-tests.yml |
Longer-running test sweep (schedule currently disabled) | workflow_dispatch |
publish-website.yml |
Publish the mkdocs site | Push to develop, manual |
refresh-website.yml |
Trigger a GitHub Pages rebuild | Daily schedule, manual |
sync-external-docs.yml |
Import the sibling repositories' documentation into docs/components/ |
Hourly schedule, manual |
release.yml |
Build and publish a release | Tag push, manual |
Where the functional jobs get their code¶
The functional jobs do not install released packages. shakenfist/actions's
setup-test-environment checks out three repositories --
shakenfist/shakenfist, shakenfist/client-python and
shakenfist/agent-python -- with the repository that triggered the workflow
at its triggering ref and the others at develop. Its
tools/deploy-collection.sh then runs the collection's example playbook with
sf_build_local_wheels=true, repo_path and client_repo_path pointing at
those checkouts. Play 0 of examples/_shared/site.yml builds a server wheel
and a client wheel, play 1 copies them to every node, and the node role
installs the wheel paths in place of the PyPI names it would otherwise use
(shakenfist/deploy/collection/roles/node/tasks/bootstrap.yml).
The consequence is worth stating plainly, because it was missed for two
months: an unreleased client change is available to the functional jobs as
soon as it merges to client-python's develop. No PyPI release is
involved. A test in this repository may be written against a new apiclient
verb the moment that verb merges next door.
Two things this does not mean. An operator deploying the collection normally
gets the PyPI packages, since sf_build_local_wheels defaults to false -- so
an unreleased verb reaches CI, and no operator. And the coupling runs both
ways: a regression merged to client-python's develop breaks this
repository's functional jobs with no change here.
The private CI conductor is on the same footing, not the opposite one. Its
deployment playbook pip-installs client-python@develop into the conductor
virtualenv with state: latest, overriding the unpinned shakenfist-client
its requirements.txt resolves from PyPI, and has since 2026-07-12 -- for
the same reason: develop moves API contracts faster than releases do, and
a released client that was behind a server contract once wedged the
conductor's main loop overnight. So the conductor picks up a client change
on its next deploy, and a release is not what gets a verb to it.
That leaves a released client mattering to exactly one audience -- operators -- which is worth knowing before treating a release as a prerequisite for anything internal.
Work on docs/plans/PLAN-scheduler-reservations.md reasoned from the
opposite belief and deliberately wrote its functional coverage against
apiclient.Client._request_url() to work around a constraint which had
already been gone for seven weeks. It then made the same mistake about the
conductor, in the same document that corrected the first one -- because it
checked this repository's install path and took the conductor's from
another plan. If you find yourself about to do something similar, check
this section first, and check the install path itself rather than a
description of it.
Running commands from the functional suite¶
shakenfist_ci/process.py is how the functional suite shells out. It is
subprocess.run() with an acceptable-exit-code list, and it replaced
oslo_concurrency.processutils.execute() -- which the harness was importing
without declaring, and which nothing else in this repository used.
Three things about it are load-bearing, and are the reason it is a module
rather than ten inline subprocess calls:
- Output is returned unstripped, and decoded with
os.fsdecode. Callers split on newlines and parse JSON, so a trailing newline is data.ip,sshand the client run against real hosts and occasionally emit a byte that is not UTF-8;fsdecodemaps it to a surrogate instead of losing the test to aUnicodeDecodeError. - stdin is
/dev/null.sshreads stdin, and a command that inherited the test runner's would block rather than fail. ProcessTimeoutErrorsubclassesProcessExecutionError. A node that accepts an ssh connection and then wedges is exactly as untestable as one that refuses it, so_require_node_exec()skips on both without being taught the difference.
Credentials go through the environment, not argv¶
_exec() in the command-line tests writes every command it runs to stderr,
and the harness prints it again on failure. So the namespace key reaches
sf-client as SHAKENFIST_KEY in the child's environment -- along with
SHAKENFIST_NAMESPACE and SHAKENFIST_API_URL, which the client has honoured
since v0.2.5 -- and never appears on a command line at all.
process.mask_secrets() exists as well, and rewrites the value of
--key-shaped flags to *** before a command or its output goes into an
exception. It is a backstop and not the protection. Name-based redaction is
the technique this repository already abandoned once in the API layer, for the
reason recorded under "Credential-carrying routes are not logged, not
redacted" in coding_rules.md: it silently starts leaking the day somebody
adds a name it has not heard of. It cannot match a short option, a positional
secret, or a flag nobody thought of. Keep the credential out of the string.
Node-exec commands have a timeout¶
_node_exec() passes SF_CI_NODE_EXEC_TIMEOUT (seconds, default 300) to
every command it runs, local or over ssh. These are introspection commands --
ip, iptables, virsh -- so one of them taking minutes means the node is
wedged, which is a state a test may well have just put it in. ssh's
ConnectTimeout=10 bounds the TCP connect only, and does nothing for a session
that connects and then stops responding; without this the job would stall
until the CI runner's own timeout killed it, with no indication of which test
was to blame.
Creating instances in the functional suite¶
Every functional test creates instances through self.create_instance() on
BaseTestCase (shakenfist/deploy/shakenfist_ci/base.py), never through a
client's create_instance() directly. The wrapper forwards its arguments
unchanged and waits out a transient InsufficientResourcesException (a 507)
rather than failing the test on it.
The reason is that a 507 on a shared CI cluster is usually a statement about
a moment, not about the cluster: the capacity the refusing node is holding is
very often already being returned by a sibling test that is mid-delete.
Failing the test on that makes a run's pass/fail a coin flip on sibling
timing. The wrapper polls /admin/resources for room for up to
CLUSTER_HEADROOM_WAIT -- 420 seconds, which
cluster_ci_tests/test_namespace_claims.py imports from base.py rather
than keeping its own copy of -- and only for a plain 507: an
AffinityConstraintUnsatisfiable 409 is never caught, because that refusal
does not become satisfiable by waiting.
"Room" means room for the whole create, on one node. The scheduler
pre-filters a candidate on cpus, memory and disk, and refuses for whichever
of the three binds, so the wait models all three
(retries.wait_for_capacity()): a node satisfies the wait only when every
dimension the create asks for fits on that node, and the wait record names
the dimension it was short of. Reading just one dimension is not a smaller
version of the same thing but a different failure -- a predicate that is
permanently satisfied while the server is permanently refusing, which turns
one 507 into a retry loop rather than a wait.
For the same reason the loop is paced. The first wait after a refusal may
hand control straight back, because capacity returning between the refusal
and the poll is both common and the case the whole mechanism exists for.
Every wait after that sleeps at least one poll interval, because a second
refusal following a satisfied wait means the create is being refused for
something the predicate cannot see, and an unpaced loop there re-issues as
fast as two HTTP round trips allow until the deadline -- leaving an
error-deleted instance behind on every turn. MAX_CREATE_ATTEMPTS is a
ceiling under that, and a failure that cites it says so rather than
blaming the cluster's capacity.
A retry uses a fresh name, because the refused instance holds the caller's
name until its error delete completes: the instance object is created and
then error-deleted, and Instance.ACTIVE_STATES includes the error states,
so the name still resolves while that delete is in flight. The base name is
trimmed so that the name plus the retry suffix stays inside the server's 63
character limit.
This means the instance's name is not always the name you asked for. A
test that looks an instance up by the literal it passed in will 404 after a
wait; read the name back from the returned dict (inst['name']) instead,
which is what test_object_names.py does. A test whose subject is the
name -- one asserting that two namespaces can hold the same name -- cannot
be renamed at all without silently asserting nothing, and so uses a
# raw-create: marker rather than the wrapper.
If the deadline passes, which is what it means for a test that is genuinely
asking for more than the cloud has, the test fails with the last refusal's
body, the node the wrapper waited for (or "unpinned" if the create had no
placement), and the per_node capacity roster from the last successful
poll, plus the dimensions the waits were short of -- so the failure carries
the diagnosis a reader needs rather than a bare 507.
A raw call -- self.test_client.create_instance(...), a local client,
another test's client, anything that does not go through
self.create_instance() -- skips the wait silently, and needs a
# raw-create: <reason> comment in the block of comment lines immediately
above it, with a non-empty reason. A unit test in shakenfist/tests/
(test_ci_raw_creates.py) walks the suite's source with ast looking for
exactly this, rather than importing the suite, which depends on
shakenfist_client -- not a test dependency of this repository. An empty
reason fails the same as no marker at all, so the allowlist cannot grow by
copy-pasting an existing line.
A call is not the only way to reach a client's create_instance().
assertRaises(Exc, self.test_client.create_instance, ...) hands the bound
method over as a reference and lets assertRaises do the calling, and the
guard checks those too -- the marker goes on the line above the reference,
inside the assertRaises call. Six sites in the suite use that shape, each
asserting a 400, 404 or 409 that must not be waited out, and each carries a
marker saying which.
The marker exists for a caller that must see the 507 rather than have it
waited out. cluster_ci_tests/test_coalescing.py's mesh burst is the
current example: it pins instances round robin across the hypervisors
inside a try that tolerates and records a capacity refusal as a normal
outcome on a shared cluster, and carries on -- wrapping that create would
break it twice, since the wrapper's own deadline failure is a test failure
rather than an APIException the except clause could catch, and waiting
up to seven minutes per refusal would serialise a burst whose simultaneity
is the thing under test. The other reserved use is the wrapper's own call
inside base.py, which has to reach the client somehow. The saturation
tests planned in
PLAN-ci-cloud-sizing are expected to
need the marker too, to assert that a genuinely full cluster refuses
rather than have the refusal waited away.
Deploy profiles¶
A cluster job can deploy more than the default stack without that change
living in shakenfist/actions. It names a deploy profile, a Jinja2 template
in this repository that the actions render against the topology's facts. A
profile can do five things, all optional:
- add inventory groups, with group variables, for hosts named in the topology;
- pass extra variables to
site.yml; - run MariaDB SQL once on the primary before the deploy;
- ask for a second deploy straight afterwards, after which no unit matching
a given pattern may have restarted (compared by systemd
InvocationID, which changes on every start); and - export environment variables into the functional test run.
The profile schema, the facts it renders against and the validation rules
are in the
actions consuming guide.
Profiles live in tools/ci-deploy-profiles/ and are named
<purpose>-<topology>.yml.j2.
The Kerbside profile¶
tools/ci-deploy-profiles/kerbside-slim-tier.yml.j2 is used by the
debian-13-slim-tier merge row. It deploys Kerbside on sf2, a hypervisor
outside the database tier, scraping the primary's sf-api across hosts. It
creates Kerbside's database on the primary's MariaDB, sets throwaway CI
credentials, and exports SF_CI_EXPECT_VDI_CONSOLE_PROXY into the test run.
It leaves kerbside_package at the role's default, so this row needs a
Kerbside release satisfying kerbside>=0.7.0. Kerbside is addressed by IP
because CI nodes have no DNS, so its certificate carries an IP SAN.
The redeploy check runs straight after the first deploy, before anything
restarts sf-api on purpose and before the tests. It asserts that a second
site.yml run restarts none of the sf-* or kerbside-* units on any node,
and a failure names the host and unit. This is the restart-on-change guarantee
for all three node roles and for Kerbside.
Kerbside's tests skip on clusters without it. On this row
SF_CI_EXPECT_VDI_CONSOLE_PROXY turns those skips into failures, so a deploy
that silently stopped enabling Kerbside cannot go green. The tests that run
there are test_vdi_tokens.py (minting and verifying a console token) and
test_vdi_kerbside_exchange.py (exchanging a token for a console file at
Kerbside, replay rejection, the proxy's TLS endpoint, and a check that no
daemon holds the signing key in its environment).
Profiles are checked offline, without a cluster, by
shakenfist/tests/test_ci_deploy_profiles.py. It renders every profile and
checks the result against the roles' own validation rules.
CI cloud sizing and headroom¶
CI's clouds are sized against the scheduler's admission ledger rather
than against real CPU, because the ledger is what the scheduler
actually admits onto. Of the bounds checked against it, only the
cluster-wide upper bound can fail a job -- nothing else does. A probe
samples every cluster job that runs through the reusable smoke-cluster
workflow with test_kind: functional or test_kind: ansible-modules,
and publishes a series recording what the cluster actually carried. The
jobs that deploy through the composite action instead are not probed yet
(#4367). See
CI cloud sizing and headroom for the sizing
model, the instrumentation and how to re-measure it.
Coverage the functional suite does not have¶
Affinity test skips are topology dependent¶
cluster_ci_tests/test_scheduler.py's test_affinity skips rather
than passes on two degeneracies, because in both the assertion could
not have failed and a pass would be a false green. The two carry
different messages, and which one is expected depends on the
topology -- so a permanently skipping run cannot be read as a healthy
one:
only N candidates-- the scorer considered fewer than two nodes, so affinity was never consulted. Expected on any cluster with fewer than three hypervisors, where the test's ownlen(nodes) < 3guard usually fires first.affine node not a candidate-- the node the test is affine to was ejected by an admission filter (CPU, memory or disk) before scoring. This is issue #3565's real mechanism and is not an affinity defect. Expected onslim-tieruntilPLAN-ci-cloud-sizinglands, since that topology runs the same suite on roughly half the resources and the admission filters bind there.
Neither skip is expected on slim-primary. A slim-primary run
reporting either one is a real signal: the cluster is smaller than it
should be, or the scheduler is ejecting nodes it should not. Treat it
as a failure to investigate rather than a pass.
This expectation is documentation, not enforcement, and that is a
deliberate trade rather than an oversight. A skip is green, so nothing
in CI fails if slim-primary starts skipping too -- the "does not skip
on a healthy three-node run" check was made once by hand. The obvious
automation, failing rather than skipping when get_nodes() returns
three or more, would fail every slim-tier run, and slim-tier is
expected to skip until PLAN-ci-cloud-sizing lands. Once it does, and
neither topology is expected to skip, that guard becomes worth adding:
fail rather than skip on any cluster with three or more hypervisors.
test_binary_affinity_prefers_the_tagged_node shares both skips and
reads the same way, since it asserts the binary model's soft half
through the same helper.
test_unsatisfiable_require_with_tag_is_refused shares neither: it
asserts a refusal, needs no successful create and no candidate count,
and so is expected to run on every topology including a single-node
one. A skip there is a bug in the test.
Upgrade data verification¶
Every functional job deploys a fresh cluster, so nothing in the suite
observes an upgraded one. cluster_ci_tests/test_upgrades.py used to
look like coverage for this: it asserted that a namespace called
upgrade still held its networks, its instances and their connectivity
after an upgrade, guarded by a skipTest() for clusters which have no
such namespace. Nothing has ever built that namespace, and the guard was
in any case written as 'upgrade' not in client.get_namespaces() --
a name against a list of dicts, which is always true -- so the test
would have skipped even on a cluster which did have one. It was removed
in the change which fixed that comparison across the suite, rather than
left to report as a skip in every run's summary.
Reinstating it means building the upgrade namespace and its objects
before the upgrade, in a job which then upgrades in place. Until that
job exists, upgrade-data verification is unimplemented, and the nearest
thing the suite does have is the Schema ENUM widening job in
functional-tests.yml, which covers exactly one upgrade hazard (ENUM
columns frozen at CREATE TABLE time) on a purpose-built database
rather than a deployed cluster.
Merge Queue Pattern¶
The CI uses a two-stage merge queue pattern (see this blog post):
Can enqueue- Runs onpull_requestevents, gates entry to merge queueCan merge- Runs onmerge_groupevents, gates the actual merge
Important: Only Can see status and Can enqueue are required status checks
in branch protection. Can merge is evaluated by the merge queue itself, not as
a required check.
A failed Can merge ejects the pull request from the queue, and
merge failure triage then classifies the failure
automatically.
The issue link check¶
Can enqueue also needs the Issue links job, which calls the reusable
issue-link-check.yml in shakenfist/actions. It compares what a pull
request claims to fix -- its branch name, its description, and its commit
messages -- against closingIssuesReferences, which is GitHub's own parse
of what will actually close, and fails when the two disagree.
The closing stanza has to be in the pull request description. Two things that look like they would work do not:
- A stanza wrapped in backticks is a code span. GitHub renders it and parses nothing.
- A stanza in a commit message closes nothing on this repository. GitHub
acts on commit-message stanzas only for commits pushed to the default
branch in the ordinary way, and the merge queue advances
developfrom its own staging ref. Putting the stanza in the commit message as well is still worth doing -- it is howgit logrecords why a change was made -- but it is traceability, not closure.
So a description needs a plain, unbackticked Fixes #NNNN on a line of its
own. issue-fix.yml already emits one: the workflow appends its own
Fixes #${ISSUE_NUMBER} as the first line of the body, after
tools/neutralise-pr-body.sh has run over the model's prose, so bot-authored
fix pull requests satisfy this gate as they stand. Do not "fix" a failure
here by loosening the branch-name pattern in the checker.
Where a pull request is deliberately only part of the work on an issue, opt
out with a line of the form X-No-Autoclose: #NNNN in the description.
The job also prints the issues GitHub will close that nobody asked for -- the other direction of the same defect, and how a closing keyword sitting beside a reference in ordinary prose reveals itself before it shuts a live bug. That direction is reported and never enforced: real descriptions carry headings and table cells of the same shape, and a check that cried wolf on those would be ignored within a week.
Editing the description does not re-run the check. functional-tests.yml
declares no types: on its pull_request trigger, so it fires on opened,
synchronize and reopened only. After correcting a description, re-run the
failed jobs from the Actions tab (or push a commit) to clear the required
check. Adding edited to the trigger is not the fix -- it would re-run the
entire functional matrix on every description tweak.
Superseded merge groups are cancelled¶
Every job that can run in the queue and holds a scarce runner carries a
concurrency: block whose key is merge-group aware — the four
functional_matrix_merge_collection entries and ansible_modules_collection
through the shared smoke-cluster.yml, and node_lifecycle_collection,
schema_enum_widening and automated_delinter in this repository.
github.ref is the obvious key and is right on every other event. On
merge_group it is the per-attempt queue branch
gh-readonly-queue/develop/pr-<N>-<SHA>, and GitHub mints a fresh SHA every
time it rebuilds the group — which it does on every push to develop. Keyed
on that, every rebuild lands in a concurrency group of its own,
cancel-in-progress never matches, and superseded merge groups run to
completion. Each one deploys several nested clusters onto the sfcbr
under-cloud that every Shaken Fist repository shares, so the cost lands on
the neighbours as well as here: shakenfist/kerbside#284 records a kerbside
merge group failing to place a 12 vCPU instance while two superseded
shakenfist merge groups held the capacity. So the key branches on the event
and uses github.event.merge_group.base_ref in the queue, with a
merge_group- prefix so a queue run does not share a group with a
workflow_dispatch run on develop.
The queue is serial — the develop ruleset sets max_entries_to_build: 1 — so
when two merge_group runs share one of these groups, one of them is
superseded. That alone does not make cancelling safe, because
cancel-in-progress has no idea which one: it cancels whichever run entered
the group first. When GitHub rebuilds a queue entry, the old and new runs
both run Check paths, and the collection jobs of whichever finishes last
enter the groups last and win. Several times that was the superseded run, so
the live merge group lost every functional job, Can merge failed on
cancelled dependencies, and the pull request was ejected with nothing
tested (issue #3998).
So a superseded run removes itself first. The last step of Check paths runs
tools/ci-merge-group-current.sh, which compares the event's
merge_group.base_sha with the current head of the base branch. With
max_entries_to_build: 1 a live merge group is always built directly on that
head, so a mismatch means GitHub has already rebuilt the entry — after the
queue merged the entry ahead of it, or after anything pushed to develop
outside the queue, such as sfconductor landing a previously ejected pull
request. The run then cancels itself (which is why Check paths holds
actions: write) before any queue job enters a concurrency group. A rebuild
that lands just after the check passed still resolves the right way: the
replacement run is only created after the push, and its jobs cannot enter the
groups until its own Check paths has run, so they enter last and cancel the
superseded run's.
The check fails in the direction that cannot eject a pull request. If the
base branch's head cannot be read, the run is treated as current and carries
on, which is no worse than before the check existed. A run judged superseded
whose self-cancellation does not land exits non-zero instead, skipping every
queue job and failing Can merge on a commit the queue has already
abandoned.
Raising max_entries_to_build breaks both halves: the key would cancel live
queue entries building in parallel, and a parallel entry is not built on the
base branch's head, so the check would cancel it too. They have to move
together. The key is audited fleet-wide; see
merge-group-cancellation.
Exported Repository Configuration¶
Repository settings (rulesets, branch protection, merge queue config) are
exported to .github/exported-config/ for version control and audit purposes:
repository-settings.json- Repo-level settingsrulesets-summary.json- List of all rulesetsruleset-*.json- Full details for each ruleset
If the export-repo-config workflow creates a PR, it means GitHub UI settings
have changed and should be reviewed.
Credential scanning¶
The credential_scan job in functional-tests.yml runs
tools/gitleaks-scan.sh, which scans every commit reachable from HEAD
for leaked credentials. On a pull request that is the whole of develop
plus the branch under test: about three seconds over five and a half
thousand commits. It is one of Can enqueue's dependencies, so a
credential cannot be merged, and unlike most jobs it is not skipped for
documentation-only changes -- a credential pasted into a code sample is
still a credential, and the one real key secret this scan found in our
own history had been published in the user guide.
The scan is scoped to HEAD rather than every ref because gh-pages
carries the built documentation site, whose search index is a single
enormous JSON blob quoting every code sample we have. Scanning it takes
five minutes instead of three seconds, produces around a hundred and
fifty findings which are all duplicates of source files already
scanned, and -- in gitleaks 8.16 -- attributes them to unrelated
develop merge commits, so they cannot even be triaged by commit.
The scan carries a positive control: it plants a key secret and an SSH private key in a scratch directory and fails if gitleaks does not report both. An empty result is otherwise indistinguishable from a broken scanner, and the allowlists described below could in principle grow until they forgive everything.
To reproduce a CI failure, run tools/gitleaks-scan.sh yourself, passing
--gitleaks PATH if the available binary is not the pinned 8.16.0. It
does not matter which directory you run it from: it changes to the top of
the working tree first, because both .gitleaks.toml and
.gitleaksignore are resolved relative to the working directory, and
from a subdirectory the ignore file would be missed silently and the
three accepted historical findings reported as new. It does need a full
clone -- a shallow one cannot see the history the scan claims to cover,
so the script says so and exits rather than passing over a fraction of
it.
Two rules are ours rather than upstream's:
shakenfist-key-secretmatches thesfk_credential format. Unit tests inshakenfist/tests/test_credentials.pyread the rule's regex out of.gitleaks.tomland assert it matches whatcredentials.generate()actually produces, so the format and the scanner cannot drift apart silently.shakenfist/tests/test_no_committed_credentials.pywalks the working tree for the same format but verifies the checksum, which distinguishes a real credential from a documented example. It runs in the unit suite and needs no allowlist at all.
Accepting a finding¶
There are two places to record a finding you have decided to accept, and they are not interchangeable.
Content which will recur -- a documentation placeholder, a test fixture,
an upstream default -- goes in the [allowlist] regexes list in
.gitleaks.toml, keyed on the text itself. Editing the paragraph around
a placeholder creates a new finding in a new commit, so anything keyed
on a commit would need replacing every time. Do not use paths for
this: blinding a whole file also blinds a real credential added to it
later. Note that 8.16 matches these regexes against the whole match
rather than the secret alone, so anchoring one with ^...$ quietly
stops it matching.
A specific historical event goes in .gitleaksignore as a
commit:path:rule-id:line fingerprint, which forgives that one
occurrence and nothing else -- the same secret in a new commit fails the
scan again. History cannot be rewritten to make such an entry
unnecessary: this repository is public, so anything committed here has
been world-readable since the day it landed. An entry therefore asserts
that the credential has been dealt with where it was trusted, not that
it has been tidied out of sight. Write down which credential, and what
was done. A unit test enforces that every entry is well formed and
carries a comment.
Automated CI Jobs¶
functional-tests.yml carries three jobs which act on the pull request
themselves rather than only reporting on it. A fourth piece of automation,
merge failure triage, is a separate workflow for the reasons given below.
Automated Delinter¶
When flake8 fails, the automated_delinter job runs Claude Code to fix lint
errors automatically. It skips if the last commit was from the bot to prevent
loops.
Automated Exception Fixer¶
When functional tests detect exceptions in logs, the automated_exception_fixer
job downloads the test bundles and runs Claude Code to analyze and fix the
issues.
Automated Reviewer¶
After successful tests, the automated_reviewer job calls the shared
shakenfist/actions/.github/workflows/pr-auto-review.yml@main reusable
workflow, which reviews the PR with the review-pr-with-claude action.
All the gating other than "CI passed" lives in that shared workflow: the
runner, the 60 minute timeout, the pull-request-event and
same-repository restrictions, its own concurrency group, and the
bot-commit check which keeps a bot push from triggering a review which
triggers another bot push. What this repository supplies is the needs:
list naming the test jobs and the token permissions, which a
cross-repository reusable workflow cannot grant itself.
The @shakenfist-bot please re-review command in pr-re-review.yml
still uses the shakenfist/actions/review-pr-with-claude@main action
directly, because it deliberately passes force to review a PR the bot
has already reviewed.
The reviewer produces structured JSON reviews, creates GitHub issues for actionable items, and embeds the JSON in the PR comment so a later reader -- human or machine -- can recover the item vocabulary, severities and verdicts that the rendered markdown flattens. Nothing consumes it today: the comment addresser was the only reader, and it has been retired.
Merge failure triage¶
When Functional tests fails on a merge_group event, GitHub ejects the pull
request from the merge queue and somebody has to decide whether the pull
request broke something or whether the test cloud fell over again.
merge-failure-triage.yml does that first pass automatically, following the
same steps a maintainer follows by hand: find the first failing step (later
failures are usually cascade), classify the failure as caused by the pull
request or systemic, search for an existing tracking issue and record the
occurrence on it — or file one — and recommend re-queueing or fixing first. The
verdict is posted as a comment on the ejected pull request.
Merge groups here launch several nested test clusters at once, so merge CI is far more sensitive to under-cloud capacity than pull request CI is, and historically most merge failures have been environmental rather than code regressions. That prior is in the prompt, which is why the evidence rules below matter: a model given no evidence and that prior will happily conclude "systemic, re-queue".
Most of the work is in tools/merge-ci-triage.sh, which gathers the evidence,
runs the model, and publishes the result; tools/merge-triage.py parses,
validates and renders the verdict. The evidence includes the other recent
merge group runs of the workflow that failed — --event merge_group on its
own is every workflow in the repository, and those twenty slots exist to answer
"is this happening to everybody?" about this suite.
Why it is a separate workflow¶
The obvious implementation is another job in functional-tests.yml, gated on
can_merge having failed. It races the queue: once Can merge reports its
failure the queue ejects the pull request and tears down the
gh-readonly-queue ref the run is on, and a triage job which takes minutes
sits in exactly that window. Moving the triage into an earlier step of
can_merge avoids the race but holds the merge queue open while a model reads
logs. A job reachable on merge_group would also fall under the
merge-group-cancellation
audit, whose correct behaviour — be cancelled when a newer merge group
supersedes you — is the opposite of what a triage job wants.
The workflow's permissions: block names actions: read, which is not
optional: naming any scope at all sets every unnamed scope to none, and every
piece of evidence the triage gathers is an Actions API read.
workflow_run runs after the merge group run has finished, on the default
branch, in the base repository context, and none of that can cancel it. The
trigger carries a branches: ['gh-readonly-queue/**'] filter so the workflow is
only invoked for merge group runs; without it every pull request run of the
test suite would create a skipped run here. workflow_run has no conclusion
filter, so the "did it actually fail?" half of the gate is the job's if:.
The workflow never checks out or runs the code it is triaging. It reads the
failed run through gh and checks out the default branch, which is what makes
a writable token safe in a workflow that reads a pull request diff.
The verdict document¶
Each triage produces one JSON document matching
tools/merge-triage-schema.json. It is published twice: as the
merge-triage-<run id> build artifact, and embedded in a collapsed <details>
section of the pull request comment, the same way the automated reviewer embeds
its review. The private-ci conductor consumes these to track which merge
failures have been triaged and which of them blamed the pull request, so the
fields that matter to a consumer are:
| Field | Meaning |
|---|---|
verdict |
pr_caused, systemic, ambiguous, or unknown |
recommendation |
requeue, fix_first, or investigate |
failure_signature |
Short stable string for grouping recurrences |
tracking_issue |
Issue recording a systemic failure, or null |
tracking_issue_action |
commented, created, or none |
run_id, pull_request |
What was triaged |
Four properties of that document are worth knowing before consuming it:
- A document is always written. A model that answers in prose, a model
that could not be run at all, a run that cannot be read, a document that
fails its own schema — each yields
verdict: unknownwith anerrorexplaining why, never a missing file. Those are different failures and theerrornames which one it was: the model wrapper's exit status is kept, so "every model is out of subscription credit" does not reach a maintainer disguised as "the response contained no JSON verdict object". The guarantee is kept by anEXITtrap over an envelope written before anything can fail, so it holds for the paths that fall over early too. A missing document is indistinguishable from a triage that never ran; anunknownone is not. The single exception is a run that was not a failed merge group run at all: nothing was triaged, so there is nothing to publish a verdict about. A document that failed the schema is uploaded alongside the replacement astriage.invalid.json, since that is a bug in the tooling rather than a triage outcome and the discarded document is the only evidence of it. - No evidence means no verdict. If neither the failed job list nor the
failed step logs can be read, no model is run: the document says so and the
verdict is
unknown. Anything else that could not be gathered — the sibling runs, the pull request diff — is named inevidence, so a verdict always carries its own caveats rather than reading as though the model saw everything. - The envelope is not model output. The repository, run id and pull request
number are written from what GitHub said and overwrite whatever the model put
in those fields, so a verdict cannot be filed against the wrong failure. Only
the fields in
MODEL_FIELDSare taken from the response at all. - A cited tracking issue has been checked. A consumer can treat a non-null
tracking_issueas evidence the occurrence really was recorded — but only whentracking_issue_actioniscommentedorcreated, and those two are the claims that get verified. The issue has to exist and the issue or one of its comments has to carry the failed run's URL; a claim that does not check out is downgraded to an action ofnone, keeping the number as a reference and losing only the assertion, with the reason appended toevidence. An issue that cannot be read at all is dropped outright, number and all — on a dry run too, since an unreadable issue is not a useful pointer whichever mode produced the citation. A citation whose action is alreadynoneis not checked for the run URL — nothing was written to it, so it will not reference this run, and checking anyway would drop every reference-only citation ever made. A dry run forcesnoneunconditionally, whatever the model reports, because nothing was written to GitHub on that path; the verification still runs against what the model claimed, so that path is exercised outside production too. On a dry run the missing reference is reported as such — nothing was written, so the citation is unverified — rather than in the words used for a claim that did not check out, which on that path would always read as a caught lie.
The comment is only posted if the body could be neutralised —
tools/neutralise-pr-body.sh rewrites it in place, and a failure inside it
would otherwise leave the un-neutralised model prose where gh pr comment
reads it from. A triage nobody sees is a smaller problem than one that fires an
@mention on publication, and the verdict is still in the artifact.
An unknown verdict is posted on the pull request like any other. It is a thin
comment, but the alternative is silence, and silence on an ejected pull request
reads as "triage never ran" — the same ambiguity the always-write-a-document
rule exists to remove, moved from the artifact to the thread. The comment names
why triage reached nothing, which is what tells a maintainer whether to look at
the failed run or at this workflow.
Re-triaging the same run does not double-post: the comment carries an invisible
<!-- merge-triage run:<id> --> marker which a later run recognises.
A comment that could not be posted fails the triage run. The verdict is in the artifact either way, but a green run with nothing on the pull request is indistinguishable from a triage that never ran, so the run itself carries the signal.
What the model can and cannot touch¶
The model runs with --dangerously-skip-permissions and the checkout as its
working directory, so the job stages everything it executes into runner.temp
before the model starts: merge-ci-triage.sh itself, merge-triage.py, its
schema, neutralise-pr-body.sh and claude-model-fallback.sh. issue-fix.yml
stages the same set for the same two reasons. Bash reads a script lazily as it
executes, so an edit to the running driver would corrupt it mid-run; and the
extractor and the neutraliser run after the model has exited, so reading them
from the workspace would mean parsing and defusing model output with a copy the
model could have rewritten. With the staging in place a workspace write has no
effect at all — the checkout is discarded with the runner and is never pushed.
The failed step logs are cut to a byte budget before they reach the prompt, and
both ends are kept with the tail given the larger share. Keeping only the head
is the obvious implementation and it is wrong here: a single Ansible
cluster-build step routinely exceeds the whole budget in progress output, and
the message saying what actually broke is the last thing it emits. The elision
is marked inline with the number of bytes dropped, and each half is passed
through iconv -c so a cut landing inside a multi-byte character cannot leave
invalid UTF-8 at the join.
The prompt reaches the model as a file, through
claude-model-fallback.sh --prompt-file, which feeds it on stdin. Never as an
argument: Linux caps a single argv element at 128KiB whatever ulimit says,
this prompt is mostly log, and an argument over the cap does not truncate — the
exec fails with Argument list too long and no model runs at all, on precisely
the failures whose logs are largest. The byte budget above is therefore about
what a model can usefully attend to and what the run costs, not about what can
be passed to it. issue-fix.yml feeds its prompt the same way, for the same
reason.
What the staging does not bound is worth stating plainly: the model runs with
--dangerously-skip-permissions and no tool restrictions on a persistent
self-hosted runner, so it can run arbitrary commands as the runner user, and
anything it leaves outside the workspace — ~/.claude state, caches, the
runner's own tree, a cron entry — survives the job and is there for whatever
runs on that runner next. A narrower tool list does not help: triage reads
GitHub through arbitrary gh sub-commands, so it needs Bash, and a model
with Bash can write anything a --disallowed-tools list would have stopped.
The risk is accepted on the same argument as the token — the content it is fed
has already passed review, on the same runner fleet issue-fix.yml already
hands a model with the same flags. Anything stronger means a disposable runner,
not a longer flag list.
Developer Automation (Bot Commands)¶
Authorized users can trigger automation by commenting on PRs:
@shakenfist-bot please re-review- Triggers a fresh automated review of the PR using the shared review action.@shakenfist-bot please retest- Dispatchesfunctional-tests.ymlagainst the PR branch, which is how a run is re-tried after a bot commit or an infrastructure failure without pushing a new commit. This is the merge queue's lane, not the pull request's, and it is the most expensive thing a comment can start here. The dispatch arrives as aworkflow_dispatchevent, andfunctional_matrix_merge_collection,ansible_modules_collection,node_lifecycle_collectionandschema_enum_wideningare each gated onmerge_group || workflow_dispatch-- so a retest runs the multi-node nested-cluster jobs on top of the pull request set. Nor does the docs-only skip apply:check_pathsruns its path filter only for events other thanworkflow_dispatchand otherwise defaultscode_changedtotrue, so a documentation-only branch gets the whole suite. That is usually what you want, because the failure worth re-trying is normally a merge queue one -- but it is a poor way to re-run a lint job, and on a busy day it is several nested clusters' worth of under-cloud capacity. Re-run the individual failed workflow from the Actions tab when that is all that is needed.@shakenfist-bot please attempt to fix- Runs Claude Code to fix unit test failures (tox -ecover). Usestest-drift-fix.ymlwith structured commit summaries.
issue-fix.yml is required to check a proposed fix against the plans
in docs/plans/ before writing any code. Triage skims
docs/plans/index.md and deprioritises issues an unlanded plan
already owns; the fix job reads the plan files covering the code it
means to change, follows the pattern established by phases which have
landed, and declines with NO_FIX when an outstanding phase is the
proper home for the fix. The plans are read from the checkout at run
time rather than summarised into the workflow, because they change
constantly. This exists because one-off automated fixes had been
landing across partially implemented plans and having to be unpicked
(see the step 3 note in
docs/plans/PLAN-scheduler-reservations-phase-01-node-metrics-columns.md).
issue-fix.yml asks the model for two marker delimited blocks on
stdout: a commit message and a pull request description. The commit
message is pushed and its first line becomes the pull request title;
the description is published as the pull request body, so an automated
fix arrives explaining its own root cause, what it deliberately did not
do, and any judgement call a reviewer might make differently, rather
than as a diffstat under boilerplate.
Neither block is required. A missing description falls back to the commit message body, and a missing commit message to a generic note telling the reviewer to read the diff -- a correct fix is worth publishing even when the prose is lost. Both cases are logged rather than fatal, because the run cannot be retried to recover them.
The parsing lives in tools/extract-model-block.sh rather than inline
in the workflow, and is covered by
shakenfist/tests/test_extract_model_block.py. It takes only the first
complete block, requires both markers (an unterminated block would
otherwise run to the end of the captured output and swallow the block
after it), treats a marker as a terminator only when it is the entire
line, and strips a code fence only when one wraps the whole block --
never globally, because a description may legitimately quote code.
The description is model output, so the workflow assembles the pull
request body into a file and passes it with gh pr create --body-file.
It must never be interpolated into a shell string.
It is also run through tools/neutralise-pr-body.sh before
publication, which drops the @ from a mention and separates an
issue-closing keyword from its reference. Both of those are things
GitHub acts on rather than renders: a mention notifies a real person
the instant the draft is created, and a closing keyword closes an
unrelated issue on merge. The prompt forbids both, and a side effect
which fires automatically and cannot be undone should not rest on the
model having complied. Fenced code is passed through untouched --
GitHub does not linkify inside a fence, and a description quoting a
decorator or an email address is a normal description.
shakenfist/tests/test_neutralise_pr_body.py covers both halves.
Neutralising the model's prose does not leave the draft unable to close
its issue. The workflow appends its own Fixes #${ISSUE_NUMBER} as the
first line of the body after this pass, above the model text and
outside any fence the model may have left open, which is what satisfies
the issue link check.
issue-fix.yml runs its fix attempt through
tools/claude-model-fallback.sh, which takes a comma-separated
preference list (--models, default claude-fable-5,claude-opus-5) and
moves to the next model when one reports its subscription credit is
exhausted. That case arrives as an HTTP 429 in the --output-format json
payload (api_error_status), which the claude CLI's own
--fallback-model flag does not handle -- it only covers overloaded or
unavailable models. A refused request is free, so the wrapper attempts
the real job rather than paying for a pre-flight probe.
Give it the prompt with --prompt-file, never as a claude argument.
Linux caps a single argv element at 128KiB whatever ulimit says, and an
argument over the cap does not truncate -- the exec fails with Argument
list too long, so no model runs and the caller sees an empty answer it
will blame on the model. --prompt-file feeds the prompt on stdin, which
has no such limit. The option takes a path rather than reading the
wrapper's own stdin because the wrapper may run claude more than once as
it falls back through the model list, and one stream is drained by the
first attempt. shakenfist/tests/test_claude_model_fallback.py covers
both properties.
CI Caching¶
Workflows that download packages use environment variables to route traffic through local caches:
- HTTP proxy:
http_proxy/https_proxyset tohttp://192.168.1.15:3128(Squid cache) for apt, curl, and general HTTP downloads. - PyPI mirror:
PIP_INDEX_URLset tohttps://devpi.home.stillhq.com/root/pypi/+simple/(devpi) for pip package installs. - uv mirror:
uvdoes not read pip'sPIP_*variables, so workflows that resolve withuvmust also setUV_INDEX_URL(andUV_EXTRA_INDEX_URLif a fallback index is wanted) to the same values. Setting onlyPIP_INDEX_URLsilently sends the uv resolve straight to pypi.
CI VMs provisioned by the shakenfist/actions Ansible playbooks also
get system-level config files (/etc/apt/apt.conf.d/01proxy and
/etc/pip.conf) so that the collection deploy and other tools use the
caches.
- Proxy bypass: no_proxy/NO_PROXY set to
localhost,127.0.0.1,10.0.0.0/8 to prevent local service traffic from
being routed through the proxy.
Fork pull requests and the runner pool¶
Workflow runs from a fork wait for a maintainer's approval:
fork-pr-contributor-approval on this repository is
all_external_contributors, not just first-time ones. That is what
makes the static runner pool safe to use for jobs that execute
repository content.
It matters most for agent-context.yml. That lane runs on
[self-hosted, static] -- a long-lived, shared host with the fleet
proxy and devpi reachable -- and it builds its hook environment from
the pull request head's own .pre-commit-config.yaml, which is also
one of its triggers. So the set of third-party code it installs and
runs is chosen by the change under test. Nothing else in this
repository has that shape; every other PR-head job on the static pool
runs code from a fixed configuration. The fork-approval setting is the
control, so if it is ever relaxed this lane moves to an ephemeral vm
runner rather than staying where it is.
mermaid-lint.yml executes tools/mermaid-lint.sh from the pull
request head under the same rule, but on an ephemeral vm runner, and
the container it starts gets --network none.
Branch Protection¶
The develop branch uses:
- Required status checks: Can see status, Can enqueue
- Merge queue with ALLGREEN grouping strategy
- Configuration exported to .github/exported-config/
agent-context.yml and mermaid-lint.yml are deliberate
exceptions: both can fail a pull request and neither gates one.
They are path-filtered, and a path-filtered workflow made a
required check reports pending forever on the pull requests it
filters out -- so a required Mermaid lint would block every pull
request that touches no markdown. needs: cannot reach across
workflow files either, so neither is named in can_enqueue or
can_merge. A red result on either is visible on the pull request
and blocks nothing automatically; treat it as something to fix
before merging rather than as something the queue will catch.
Both lanes are vendored from shakenfist/development's templates:
tools/mermaid-lint.sh is byte-identical to
templates/mermaid-lint/mermaid-lint.sh there and is synced rather
than edited. What this repository chooses for itself lives beside
it in tools/mermaid-lint-exclude, which drops docs/components/
-- sync-external-docs.yml imports those 664 files hourly from the
sibling repositories and AGENTS.md forbids editing them here, so
a diagram broken upstream is fixed upstream rather than failing a
pull request here that cannot fix it. GitHub Actions cannot read
that file, so mermaid-lint.yml's path filters carry the same
exclusion by hand and the two have to move together.