Skip to content

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, ssh and the client run against real hosts and occasionally emit a byte that is not UTF-8; fsdecode maps it to a surrogate instead of losing the test to a UnicodeDecodeError.
  • stdin is /dev/null. ssh reads stdin, and a command that inherited the test runner's would block rather than fail.
  • ProcessTimeoutError subclasses ProcessExecutionError. 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 own len(nodes) < 3 guard 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 on slim-tier until PLAN-ci-cloud-sizing lands, 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):

  1. Can enqueue - Runs on pull_request events, gates entry to merge queue
  2. Can merge - Runs on merge_group events, 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.

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 develop from its own staging ref. Putting the stanza in the commit message as well is still worth doing -- it is how git log records 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 settings
  • rulesets-summary.json - List of all rulesets
  • ruleset-*.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-secret matches the sfk_ credential format. Unit tests in shakenfist/tests/test_credentials.py read the rule's regex out of .gitleaks.toml and assert it matches what credentials.generate() actually produces, so the format and the scanner cannot drift apart silently.
  • shakenfist/tests/test_no_committed_credentials.py walks 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: unknown with an error explaining why, never a missing file. Those are different failures and the error names 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 an EXIT trap 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; an unknown one 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 as triage.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 in evidence, 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_FIELDS are taken from the response at all.
  • A cited tracking issue has been checked. A consumer can treat a non-null tracking_issue as evidence the occurrence really was recorded — but only when tracking_issue_action is commented or created, 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 of none, keeping the number as a reference and losing only the assertion, with the reason appended to evidence. 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 already none is 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 forces none unconditionally, 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 - Dispatches functional-tests.yml against 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 a workflow_dispatch event, and functional_matrix_merge_collection, ansible_modules_collection, node_lifecycle_collection and schema_enum_widening are each gated on merge_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_paths runs its path filter only for events other than workflow_dispatch and otherwise defaults code_changed to true, 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). Uses test-drift-fix.yml with 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_proxy set to http://192.168.1.15:3128 (Squid cache) for apt, curl, and general HTTP downloads.
  • PyPI mirror: PIP_INDEX_URL set to https://devpi.home.stillhq.com/root/pypi/+simple/ (devpi) for pip package installs.
  • uv mirror: uv does not read pip's PIP_* variables, so workflows that resolve with uv must also set UV_INDEX_URL (and UV_EXTRA_INDEX_URL if a fallback index is wanted) to the same values. Setting only PIP_INDEX_URL silently 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.

📝 Report an issue with this page