The rule only fired one way: a stack role with named volumes had to carry
the flag, but a role without any volume could carry it unnoticed. That
entry pulls svc-bkp-volume-2-local into the role's dependency graph and
schedules a backup run over a role with nothing to back up.
_scan() now classifies every invokable role instead of skipping the ones
that need no flag, and returns both halves. The nocheck opt-out still
exempts a role from both directions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A container without a healthcheck counts as healthy the moment its process starts. That is how a service which came back empty passes a restore drill: the drill asks the host's health service, and the host says yes.
The image side is read from the registry rather than by pulling: the manifest names the config blob, the blob carries the HEALTHCHECK. The answer is three-valued on purpose. A mapping means the image ships a probe, an empty mapping means it definitely ships none, and None means the question could not be answered. Only the empty mapping is evidence; reading None as absent turns one network hiccup into a hundred false findings.
Probes address the GHCR mirror first and fall back upstream only for images the mirror does not carry, and a second test requires mirror plus cache to answer more often than upstream, so the fallback cannot quietly become the rule and spend the anonymous rate limit again.
Detection spans the four places a probe can come from: the services.yml declaration, a literal block in a template, a partial pulled in by include or lookup('template'), and a HEALTHCHECK in a Dockerfile the role builds its own image from. Without the last three the sweep reported nextcloud, every redis sidecar and openldap as gaps they are not.
The opposite direction gets its own lint: a declared healthcheck that no template renders is dead configuration. Fixing it surfaced that the shared resolver silently dropped every service_name call site, because it accepted only quoted literals and a role var resolves to a bare key. And the literal-protocol lint could never be suppressed on a wrapped value, since autoformat moves the marker to the value's last line while the offence is reported on its first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The collector walked every line of a vars: block and treated each key it met
as a variable passed to the include, whatever its depth. In
vars:
role_templates:
- src_glob: "config/*.j2"
dest_dir: "{{ NEXTCLOUD_HOST_CONF_ADD_PATH }}"
mode: "0644"
exactly one variable is passed. src_glob, dest_dir and mode are fields of its
value, and demanding a Jinja appearance for each of them is a question the data
cannot answer.
It went unnoticed while every such field happened to occur in some template
somewhere. Moving the role-templates expansion into a lookup ended that:
dest_dir is now read in Python, the lint stopped seeing it, and a correct
refactor came out as a lint failure naming a variable that is very much used.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
roles/__init__.py makes roles a Python package, so every pytest run leaves a roles/__pycache__ behind. The required_by coverage lint walked ROLES_DIR.iterdir() unfiltered and reported that cache as roles/__pycache__/meta/services.yml (missing), i.e. as a non-invokable role lacking required_by.
Enumeration now skips names starting with . or _. The only such entries under roles/ are .gitignore and __init__.py, both files that is_dir() already excluded, so no real role leaves the check: 256 -> 255 directories, the single loss being __pycache__. With the cache present the lint still fails on an injected roles/sys-svc-covtest/ and lists only that offender.
A sibling fix for the same class lives on other branches as 38c8b59fe, covering tests/integration/test_roles_have_meta_main.py and test_roles_naming.py -- different files, no overlap with this one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four tests were guarding two rules. Both duplicates are gone and what they
were meant to check now lives with the rule it belongs to.
The handler name/length pair: a 120-character limit sat over the same files
as an 80-character one, so it could never fire first, and outside that overlap
it matched `name:` keys that were no task at all -- a volume name, a var. It
found nothing in either half. Deleted; the surviving lint instead grows the
scope it was missing, `roles/*/handlers/**` and `playbooks/**`, because a
handler is a task, and the dispatch input descriptions, because a form line is
read like a job name.
The templated-handler-name pair is worse: the second one skipped every
document with `if not isinstance(doc, dict)`, and a handlers file is a list.
It has been vacuously green for its whole life. Its intent -- every
`handlers/*.yml`, not just `main.yml` -- is now what the surviving test does.
The notify/listen contract itself gains the rule that made the rename in
a8dd7f1 safe: a notify must land on a `listen:` topic, never on a display
name, and both spell out as [a-z_-]+. Following `import_tasks` chains by hand
is gone with it; scanning every project YAML sees the same handlers, plus the
ones a role hands to a generic renderer through a data structure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ruff bump in 6320da703 changed how the formatter picks quote style
inside nested strings and where it wraps a generator argument. Both
files were written before that landed and this is the first autoformat
pass since, so make quality rewrote them.
Formatting only; no assertion, message or rule changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
72fd9c870 required the onion in every variant but exempted variant 1, on the
argument that the minimal round is the one place the stack runs without it. That
argument does not hold: the onion is part of the deployment under test, not a
feature bolted onto the full shape. A round that skips it is not a smaller
deployment, it is a different one.
So the exemption is gone and 95 variant-1 entries pin the onion on. Two
exclusions remain, both read from data rather than a comment: a role with no tor
block has no onion to pin, and a role whose meta/services.yml pins tor.enabled to
a literal false has already declared it has none.
That put the rule in conflict with test_coverage, which requires both polarities
of every dynamic flag: with tor on everywhere, the false side has no variant left
to live in. 94 roles take the exemption that contract already provides for
exactly this - "flags whose group-derived value is a hard invariant that variants
must never override", the same one 4e6ef2542 used for the seaweedfs frontend.
web-app-nextcloud needs none: its variant 8 pairs the clearnet-only PeerTube and
keeps the false polarity.
Measured, because the declaration is not what CI runs. assign_tor gives the onion
to every second capable entry with the run number as the parity switch, so more
capable variants do not mean proportionally more onion jobs: across five roles
and 21 entries the count goes 8 -> 10 on an even run and 9 on an odd one. The
clearnet path also survives - checkmk reports v1=false on run 0 and v1=true on
run 1, so both states are still covered across two consecutive runs. What the
94 markers give up is a declarative guarantee in variants.yml, not the coverage
itself.
The real cost is weight, not job count. web-app-nextcloud variant 1 goes from
3.09GB reservation / 7.27GB limit to 18.26GB / 43.01GB once svc-net-tor and its
closure join, so the cheap round is no longer cheap. Every variant stays under
the 32/64GB budget, and the same effect applies to the other 94 unmeasured.
Verified: tests/lint 336 passed / 307 subtests, tests/integration/roles/meta/
variants plus the role-meta lints 39 passed. The rule was checked in both
directions - it reported the variant-1 offenders before the flips and none after.
Not measured: what the heavier rounds do to wall-clock, against swarm jobs capped
at 355 minutes and one that has hit that ceiling before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7a70d27b7 made a variant that switches SSO on also switch the onion on, because
that pairing is where the platform breaks in ways clearnet never shows. The same
argument reaches further than SSO: a Secure cookie is dropped, an https identity
provider refuses to embed, and crypto.subtle is undefined for any feature on a
non-secure origin, not only for a login. A variant that never runs the onion is
not testing the deployment users get.
So the rule now covers every variant of a role that declares a tor block.
Variant 1 stays exempt - it is the minimal, partner-free round every role
declares and the one place the stack is exercised without the onion at all - and
a role whose meta/services.yml pins tor.enabled to a literal false is skipped
outright, because that declaration already says the role has no onion and needs
no comment repeating it.
That found 137 variants. 101 were variant 1. Of the rest, 28 are the LDAP-only
auth-isolation rounds, which gain real coverage from running LDAP over the onion
and are flipped on here. Four cannot: svc-net-tor is the provider itself, and
nextcloud's variant 8 pairs the clearnet-only PeerTube. Both carry a marker
naming the reason; opentalk and coturn need none, since their services.yml pins
the literal false the rule now reads.
Flipping them exposed a contract that contradicted itself. test_auth_isolation
and test_non_baseline_explicit_disables demanded `shared: false` beside
`enabled: false`, while variants-static-override flagged that same line as a
static override - which is why all 133 of them carried a nocheck marker. The
demand only makes sense where `shared` is dynamic: a literal base value cannot
leak through deep-merge, so restating it protects nothing. Both contracts now
ask for it only in that case, and the 133 lines are gone.
The result is a stricter rule with fewer lines: 189 insertions against 193
deletions.
Verified: tests/lint 336 passed / 307 subtests, tests/integration/roles 107
passed / 610 subtests. The rule is checked in both directions - it reported 137
offenders before the resolution and none after.
Not measured: 28 additional onion rounds cost matrix wall-clock, against swarm
jobs already capped at 355 minutes and one that has hit that ceiling before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The readiness probe 5bd323999 added for the swarm path dialled the role's public
domain on port 80. wait_for opens a plain TCP socket from the controller, and
once 1d9d355bc gave the domain roles onion variant coverage that target became
a .onion authority, which needs Tor's SOCKS resolver and cannot be reached with
a socket. Variant 6 of run 31823845576 therefore spent the full budget on it -
"Timeout when waiting for lab.git.<onion>:80", elapsed: 600 - while the
preceding resolve_host_cid had already converged, so GitLab itself was up the
whole time.
The repository had already settled this. DOCKER_REACH_HOST
(group_vars/all/00_general.yml:47) is 127.0.0.1, swapped for the Docker bridge
gateway when the controller runs in a container, and every one of the other
eleven wait_for probes under roles/ uses it. The nearest neighbour is the same
task shape - same resolve preamble, same retries and delay - reaching its
partner as `host: DOCKER_REACH_HOST` with `ports.local.http`
(integration_openproject_provision.yml:12-17). This one was written from that
template with the host and port substituted, and the substitution is the defect.
Restoring the idiom keeps the readiness gate 5bd323999 wanted instead of
dropping it, so retries stays at 12.
The lint pins the shape, because the divergence is invisible in review: both
forms read as a plausible readiness probe, and the wrong one only fails where
the domain happens to be unroutable. It parses each wait_for block under roles/,
locates the host key by indentation, and requires DOCKER_REACH_HOST unless the
line carries `# nocheck: wait-for-reach-host` naming what the probe proves - the
opt-out a probe needs when it is verifying the ingress rather than a backend.
Checked against the original line, where it reports
integration_gitlab_provision.yml:14, and against the eleven correct probes,
which it accepts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
make quality-high stopped at autoformat: ruff reported PERF401 in the addon
variant-pinning lint, whose third check collected findings through a loop whose
entire body was findings.append(). It hands a generator to findings.extend() now.
The two sibling checks were not flagged -- their `continue` guard makes them
something other than a pure transformation.
Rewriting a finding collector can silence a rule while every test still passes,
so the rule was broken again afterwards: a dynamic addon pinned in variant 1
still reports all eight variants, the same set as before the change.
The rest is what the formatters rewrote by themselves -- a trailing blank line in
the ganesha task file and two over-long lines wrapped in the volume spot lints.
make quality-high is green end to end after this: four test suites, ansible-lint
0 failures over 3632 files at the production profile, eleven lint targets.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An addon's enabled is either dynamic -- a Jinja expression resolving per deploy
against a service flag, an API credential, the SSO plugin choice or group_names
-- or static, a literal boolean. A dynamic addon already follows its variant:
switch the service off and the expression yields false. A statically-on addon
does not. It installs in every round, so each non-baseline variant pays install
time for plugins it never exercises, and variant 0 stops being the only
maximum-footprint shape that meta/variants.yml exists to document.
web-app-nextcloud carried 22 of them across nine variants. They are now pinned
off once in variant 1 behind a YAML anchor, and variants 2-8 alias it, so the
set has one spot -- the same shape meta/volumes.yml and the
volumes-target-single-spot lint already prescribe.
test_addons_variant_pinning.py holds the rule for every role that ships
meta/addons/ and declares more than one variant:
1. an addon whose base enabled is literal true MUST be pinned enabled: false
in variant 1; a literal false is out of scope, the pin would override
nothing,
2. an addon whose base enabled is a Jinja expression MUST NOT be pinned in
any variant -- that replaces the expression with a constant and decouples
it from the service it follows,
3. every later variant MUST carry the same set, in practice by aliasing
variant 1. Variant-specific pins beyond that set stay allowed, so a
variant may still switch a statically-off addon on (web-app-matrix does
exactly that for its bridges in variant 2).
Measured on the real role through utils.cache.applications.get_variants: all
nine variants keep 51 addon keys, variant 0 keeps its 22 statics on, variants
1-8 hold none, and every dynamic entry keeps its Jinja string -- xwiki still
resolves true only where services.xwiki.enabled is. The 51 keys matter: the
override merges per addon instead of replacing the map, which is what an
earlier lookup-based attempt at this got wrong.
Each rule was broken on purpose and seen to fail: calendar pinned true, xwiki
pinned (reported for variants 1-8, which also proves the alias resolves), and
variant 5's alias removed (22 findings, exactly the required set).
web-app-discourse is exempted per addon with a reason. Its plugin set is
compiled into the image from services.discourse.plugins, and meta/addons/ is a
second declaration of the same thing; an addon-level disable would still build
the plugin and only flip the gate flag, saving no time while silencing the
spec. web-app-wordpress is not exempt: vars/main.yml:16 builds the plugin list
from the merged map and 07_manager_ops.yml:45 gates the install loop on
enabled, so its variant 1 genuinely skips a wp plugin install. Its README
claimed activitypub was "always enabled"; that is corrected rather than left to
contradict the file.
Not verified: no deploy has run. The saving is derived from the install loops
being gated, not observed, and the WordPress Multisite variant losing
activitypub rests on the same reading plus its skipUnlessAddonEnabled gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Moodle renders the same msmtprc as WordPress and mounts it into two services,
but nothing ever proved a message left the container: its healthcheck opens the
FPM port and nothing else. Moodle 0 of run 31658241728 passed green while mail
could not have worked -- the defect 2da285067 fixed for WordPress was invisible
here, and would have stayed invisible until a user reset a password.
WordPress proves delivery with the msmtp_curl flavor: the probe sends one mail
and writes a marker, and a task after convergence waits for that marker. Moodle
cannot use it, because curl cannot judge a port that answers FastCGI. Rather
than add a second msmtp_* class per liveness shape, the declaration now composes:
healthcheck:
flavor:
- msmtp
- connect
port_key: fpm
msmtp stops being a flavor and becomes a prefix -- shell that runs on the way
past and never decides liveness. connect joins the probes as the one that only
asks whether the port accepts a TCP connection. A declaration names exactly one
probe and any number of prefixes, as a string or a list; msmtp_curl stays as an
alias for [msmtp, curl], and the lint that validates declarations now shares
resolve_flavors with the lookup instead of keeping its own idea of a flavor.
The module became five responsibilities in one file, so it is a package:
probes.py answers which probes exist, prefixes.py what runs before them,
compose.py how a declaration becomes a rendered block. The curl helpers sit with
the probes and mail_branch with the prefix -- each next to its only caller,
rather than together in a module header.
Two defects in this work were found by measuring, not reading:
- the timing merge took the maximum across prefixes only, which cut Moodle's
start period from 15m to 120s and would have marked a heavy PHP app dead
while it was still installing. A prefix may now raise a timing and never
lower one, pinned by test.
- composing moved msmtp_curl's interval from 30s to 1m, because the composed
class derives from Curl where MsmtpCurl derived from Probe. That is the
interval every other curl probe uses, but it was not intended; it now has
its own test rather than riding along unseen.
Regression measured across the whole repo rather than argued: every declared
healthcheck was rendered with the pre-refactor code and with this one. 58 render
byte-identically, 1 is Moodle's intentionally changed declaration, and 2 differ
-- friendica and wordpress, in that interval alone, with the probe command
itself unchanged.
Coverage was checked in both directions. No test was removed: the four deleted
lines are an import, the registry literal, one class name in a url test, and the
helper, which now goes through compose() instead of reaching into PROBES -- a
tightening. New code had two gaps, both closed: connect was the only probe whose
emitted command no test pinned, and compose() raised KeyError for an unknown
name unobserved.
Not verified: no deploy has run any of these probes. The composed class is built
dynamically, so what it emits is proven for all 59 services while docker
accepting it is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
extra_hosts is the one compose key a service may not carry twice. The onion SSO
back-channel pin was emitted by a Jinja partial that six roles included, while
six other services wrote the key by hand; nothing stopped a service from
receiving both. YAML then either rejects the document or keeps one list and
drops the other, silently.
The partial becomes plugins/lookup/container_extra_hosts.py, and every pin now
goes through it. Callers add their own entries with extra_hosts=[...], the way
container_volumes already takes extra_volumes=, and the lookup merges them after
the automatic pin and deduplicates in first-seen order. With nothing to emit it
returns an empty string.
The swarm address reasoning from 7b1839681 is preserved and now written down in
the plugin rather than only in that commit: the pin must NOT be host-gateway in
swarm, because that alias resolves per node while openresty publishes :80 with
mode: host under node.role == manager. It carries the stack host's address,
which reaches every task because the compose file is rendered once with the
manager as inventory host. The mode is read as compose_mode_force or
DEPLOYMENT_MODE, not the bare cluster mode, so a role with an override does not
lose its pin and its overlay at once.
Two roles kept a pin that now collides with the automatic one, and both are
resolved the same way -- the hand-written entry stays for clearnet and yields
the .onion case to the lookup, which picks the address per mode. In
web-app-mailu the three conditional entries move into a role-local
mailu_extra_hosts lookup exposed as MAILU_EXTRA_HOSTS, because an eight-line
conditional expression in the template argument is worse than the block it
replaced. It returns its entries as separate results, so vars/main.yml uses
query(); plain lookup() would join them into one comma-separated string.
tests/lint/ansible/jinja/test_compose_extra_hosts_use_lookup.py fails on any
literal extra_hosts: key in a role template, so the invariant survives the next
author who needs a pin. It follows test_compose_ports_use_container_ports_lookup:
the raw construct is forbidden, the lookup is not mandated per service -- a
service with no pins is correct without it, and the swarm branch raises when
facts are absent, which would turn a no-op into a failure surface for containers
that never speak OIDC.
Equivalence checked by rendering rather than by reading: under the trim_blocks +
lstrip_blocks the stack-host action forces for compose.yml.j2, both former call
site shapes (indented, and matrix's column 0) emit byte-identical text to the
new form when the block applies. The no-op case differs -- a whitespace-only
line where the include emitted none -- and yaml.safe_load returns the same
document for both.
31 unit tests; make test-lint 325 passed. Does not touch web-app-espocrm, whose
missing pin is a separate claim and gets its own commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eight prose references still pointed at the pre-rename root, and one of them
did more than mislead a reader: the addons lint told an author to reach a
secret through lookup('config', application_id, 'credentials.<name>') at the
exact moment it rejected their literal, so the guidance produced the next
defect. The check itself only tests whether the value is templated, so no
enforcement changes here.
variants.md gains the reason rather than a corrected list. A variant entry is
baked into host_vars raw, which makes its top-level key the inventory path, so
a key no meta topic produces is silently discarded. The previous list drifted
precisely because it stated the set without stating why it was the set.
design/variants.md was wrong twice over: the old root, and an argument order in
lookup('config', 'credentials', '<provider>') that could never have resolved.
The erpnext key was checked against svc-db-mariadb/vars/main.yml rather than
prefixed on faith.
Left alone on purpose: test_credentials_roundtrip and the layout lint name
meta/schema.yml and schema/main.yml as history and as the legacy directory the
lint forbids. Both are correct.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The only shape check on meta/variants.yml rejected a single key, `compose`.
Its docstring stated the actual rule -- top keys must be payload roots -- and
never enforced it, and had itself gone stale: it still named `credentials`,
the root f713ba835 retired. That is how the previous commit's four blocks
passed every gate while writing where nothing reads.
The legal roots are now derived from the meta files instead of restated: every
roles/*/meta/*.yml stem, plus addons when that directory carries specs, minus
main and variants, which are mechanism rather than payload. A role adding a new
topic makes its root legal without touching this test, so the list cannot drift
from the loader again.
compose needs no special case anymore -- it is not a meta file, so it fails
like any other unknown key, as does a typo such as `servicse`. Verified by
running the pre-fix keycloak and lam variants through the new whitelist: all
four `credentials` blocks are rejected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The adapter was the only image in the tree pinned by a hand-maintained digest,
and the pin was never recomputed by anything. It drifted twice: run 31374959721
lost seven of eight jobs to mirror/docker.io/python@sha256:6771159c... not
found, and the renewal to the newest upstream digest earlier today would have
reproduced it, because builds resolve FROM against the GHCR mirror while the
mirror is copied by tag and lags upstream by design.
The base is now FROM <image>:<version>, taken from the sidecar service in
meta/services.yml - the only file iter_role_images scans, so the tag the build
asks for is by construction the tag the mirror was told to copy. The version
moves from the moving 3.13-slim to 3.13.15-slim, which upstream stops rewriting
once published, and which the existing version drift job renews like every other
image.
With the build reading services.yml, the adapter block's digest, image and
version had no consumer left. They are removed together with the schema check
for the digest form, the warn-only drift test, and the manifest_digest helper
that existed only for it. What remains in the block describes the adapter rather
than the image it is built from.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every one of the eleven declared tools carried the placeholder schema
{"properties": {}, "type": "object"}. The adapter answers tools/list purely
from this contract, so each client saw parameterless tools: nothing could know
that read_post needs a post_id, and the call failed upstream instead of being
formed correctly. The schemas here were measured against the running upstream;
list_agents keeps an empty property set because upstream declares none.
Both pins of the file follow, as the adapter rehashes the contract at startup.
Add a lint binding the declared surface to the shipped one. Only the allowlist
is an equality: writer_allowlist is a superset by construction, because
passthrough.py rejects a whole contract as soon as one tool declares
mutating: true while mutations are off, and upstream_serves is an observation
that may legitimately exceed what the platform allows.
The suppression lookup both MCP lints need moves into suppress.py rather than
being written twice.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three fields pinned artefacts and nothing recomputed any of them.
The adapter base digest was stale for all twelve provider roles: the tag
python:3.13-slim moved to ffb752e1 while every role still pinned 9662417a.
The GHCR mirror copies by tag, so the pinned manifest is one it was never
asked to copy - a mirrored build resolves FROM to nothing while a direct
build keeps working.
adapter.specification_sha256 matched no revision of the file it pins in ten
of twelve roles; three of the old values were byte-identical to the same
role's tools.schema_sha256, so they had been copied rather than computed.
The field is enforced nowhere at runtime, which is exactly why it needs a
check.
Add a hard lint for both offline pins, computing the tool digest with the
adapter's own policy.schema_digest so it cannot drift from the code that
enforces it in production, and a warn-only external check that reports a
base digest the upstream tag no longer resolves to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
75 commits: the socialhome role reimplemented against the project it actually
deploys, native-login specs for funkwhale and shopware, a window.open IdP
handoff and a settings-link fallback in the shared persona helpers, a swarm
converge budget that gives up on a terminal task, and a long tail of role
fixes.
Eleven conflicts. The shared persona helpers (keycloak.js, logout.js) take the
incoming structure — they gained real behaviour this branch never had — with
this branch's resolveTimeout convention re-applied over it and
confirmKeycloakLogoutIfPrompted re-attached at every return point; without it
the Keycloak SSO session outlives the logout. playwright.config.js keeps this
branch's version: both sides set actionTimeout, but ours routes through an env
var and 15s is too short behind an onion. socialhome's spec takes the incoming
content — 2dbd6eb10 found the role described the wrong upstream project
entirely — and keeps gotoOnion and expectHstsWhenTls, since demanding HSTS over
a plaintext onion is a false failure. Both READMEs were regenerated rather than
merged by hand.
Three collisions arrived with no conflict marker at all, because each side
changed a different place in a different file:
- group_vars/all/18_swarm.yml declared swarm_stack_converge_retries twice.
Same value, so nothing breaks today; editing the documented one would have
silently changed nothing.
- svc-net-tor and web-app-socialhome both claimed 192.168.74.0/24. Socialhome,
the newer role, moves to 192.168.75.0/24.
- web-app-socialhome/meta/schema.yml arrived as a new file, so ef5ea7cb5's
rename never saw it. Its credential would not have resolved and the role
would have deployed with an unusable administrator account.
The incoming specs also needed this branch's conventions: raw timeouts wrapped,
four page.goto calls routed through gotoOnion, a request.get given an
onion-aware timeout, and 21 roles marked playwright-service-flag over the tor
block this branch added to them.
make quality green on the merge result.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An adapter-fronted provider holds two unrelated credentials, and handing
the upstream token to the contract probe produces a 401 the log blames on
the adapter. The rule binds the probe include, the vars definition reading
credentials.mcp_bearer and the ADAPTER_BEARER the sidecar starts with to
one variable, so the three cannot drift apart.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both tests reached for the filesystem themselves: one built the project root
from Path(__file__).parents[4], which silently points at the wrong directory the
moment the file moves, and both called Path.read_text directly, bypassing the
cache every other reader shares. The role-defaults path was spelled out by hand
where ROLE_FILE_DEFAULTS_MAIN already names it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SSO over a plaintext onion is where this platform actually breaks, and none of
it shows on clearnet: a framework's Secure session-cookie default makes Chromium
drop the cookie, an https identity provider cannot be embedded in an http page,
and crypto.subtle is undefined because an onion is no secure context. Pixelfed,
Taiga, Joomla and WordPress each cost a red job over exactly that. The
combination is only exercised if a variant deploys both, so the lint requires
it: services.sso.enabled true implies services.tor.enabled true.
69 variants enable SSO; 59 already complied. Of the ten that did not, seven are
flipped on because the mechanism is proven there - matrix and nextcloud both run
SSO behind the onion in variant 0 already (20ddda8c1), and their later variants
were pinned false only by the blanket rule from 1d9d355bc. Nextcloud variant 1
stays false: SSO is off there, and it is what gives the coverage test its
false-pinning variant.
Three roles opt out, each for a structural reason named in the marker. jitsi and
bigbluebutton pin tor.enabled false in meta/services.yml because the conference
needs getUserMedia, which no plaintext onion can offer. seaweedfs gates its own
sso flag on 'svc-net-tor not in group_names', so on a tor node there is no login
flow to test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tools.allowlist meant two things at once: the surface a consumer may reach,
which is a decision of ours, and the surface the upstream offers, which the
vendor changes without asking. Conflating them made a version bump read as a
contract violation, and left no place to record what was actually measured.
Split them. allowlist stays policy; upstream_serves records the observation at
supported_version. State the second only where it DIFFERS, because a provider
that serves exactly what is allowed would otherwise carry the same list twice
and the copies would drift apart. Absence means identical.
The difference between the two measures unenforced exposure: mattermost 103,
gitlab 2, and zero wherever the fields collapse. A non-empty difference is what
tells us a provider needs a gateway in front of it.
Require a non-empty allowlist for an enabled, non-adapter, server-capable
provider. An empty one used to make the probe's comparison vacuous, which read
as a pass while every tool the upstream serves stayed reachable. Nextcloud was
in exactly that state.
Three exemptions, all semantic: an adapter authors its tools here against a
REST upstream, so there is nothing to observe; a provider declared
enabled: false is never admitted; and upstream_serves: dynamic covers an
upstream composing its surface at runtime, which nextcloud does by proxying
MCP servers an administrator configures.
Reject an allowlist naming a tool the upstream does not serve. That found
mattermost's add_user_to_channel, gone since 11.10.0 renamed it
add_channel_member, which would have been advertised and never callable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Under swarm an unpinned role's volumes are rewritten onto the shared NFS
export while its services run one task per node. A role that declares no
RDBMS keeps its state in-process, so several replicas then write the same
SQLite file, Mnesia store or search index over NFS and corrupt it. This
already cost a deploy: openclaw ran three replicas against one shared
export and its state directory answered "file is not a database".
The existing unpinned-no-engine-volume lint states the right rule but
detects state through a hardcoded path list, so app-specific paths slip
through and each case gets pinned reactively after it fails. Add a lint
that joins three facts the roles already declare: the volume is NFS
backed (asked of the production swarm_nfs_backed, not re-derived), it is
mounted read-write, and the role runs no RDBMS. Share-safe payloads carry
a justification marker.
Replicas resolve service-first and fall back to the primary entity, and
a role whose swarm mode is disabled is skipped; both cases exist in tree.
Also correct the requirement and the design doc, which still described
the per-volume opt-in model that b09598dc8 replaced with placement
derived backing. Requirement 023 contradicted itself: line 76 already
described the implemented behaviour.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each of these guards a failure the work already hit. An adapter path
declared in two files drifts silently and surfaces as a 404 on the first
tool call. A provider borrowing the administrator token hands every
client that account's rights, and the read-back never matched the store,
so every deploy minted a fresh secret. A task that both carries a secret
and produces printed output puts it in the deploy log verbatim.
Provider probes, consumer transports, the schema and variant coverage
are checked the same way.
Every MCP role keeps its task tree under tasks/utils/ without numeric
prefixes, so a caller can reach any of them through include_role
tasks_from. utils/roles/mapping.py holds that path as the single source.
Ansible resolves a relative include against the including file's own
directory when that file was reached by path, but against tasks/ when it
was reached through tasks_from. The same file therefore loaded from its
own role and failed from a caller using tasks_from. Every nested include
now names {{ role_path }}/tasks/utils/..., which holds under both bases,
and a lint walks each tasks_from entrypoint transitively to keep it so.
The exec-gating lint learns the same form, otherwise it loses the include
chain and reports inherited IS_STACK_HOST gates as missing.
The expression resolving a provider's stored token appeared eighteen
times. plugins/lookup/mcp_credential.py replaces it, and mcp_probe_owner
disappears from eight roles: it was only ever a step towards the bearer
and no probe read it.
The Hermes and OpenClaw probes stop aborting the play while it is still
running. Roles deploy in dependency order, so a client probing during its
own role can run before a provider exists at all, and failing there
prevents that provider from ever deploying. The verdict moves to the
reconciliation pass, gated on MCP_RECONCILE_STRICT, which the reconciler
sets and group_vars declares so the readers need no default.
The sys-svc-mcp-reconcile role is added here; wiring it into a stage is a
separate change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The workflow directory mixed entry points, reusable stages and scheduled jobs into one flat list sorted by topic, so the first question about any file - what starts it - was the one the name did not answer. Every workflow now carries one of three prefixes, derived from its on: block: cron- for a schedule trigger, call- for one only reachable through workflow_call, entry- for everything a repository event or a human starts. Scheduled workflows additionally carry an alarm clock as the first character of their name.
Two lints keep it that way. One derives the expected prefix from the triggers and compares it against the file name; it flagged all 17 offenders before the rename and none after. The other resolves every .github/workflows/<file> string in the repository against the directory, which is what a rename silently breaks in Markdown links, Python constants and shell API URLs alike.
Renaming pulled 69 pre-existing comments into the comment lint's changed-file scope. The vendored CodeQL template got the nocheck marker its scorecard sibling already carried, two workflow headers moved above name: where they count as file headers, and the remaining banners were deleted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Blocking a persona removes the only end-to-end proof that a role's auth chain
works. docs/contributing/artefact/files/role/playwright.specs.js.md:146 has
always required each flag to carry a one-line nocheck stating the role contract
that justifies it, plus a matching paragraph in the role's README or TODO.
Nothing enforced it, so 89 flags across 46 roles carried neither.
The lint mirrors test_has_spec's shape and uses is_suppressed_at with
mode="line-above", so the marker must sit directly above its flag -- a blank
line between them breaks the association, which is what the persona helper's
reader will look at. A literal =false blocks nothing and is exempt; the
sso-conditional ternary is not, because it blocks in every variant that pins
services.sso.enabled false.
Writing 89 reasons turned out to be the point. Five roles audited in one pass
blocked for five entirely different mechanisms, all of which read as the same
shrug behind a bare =true: pgAdmin's login id is the account e-mail rather than
the Keycloak username; phpMyAdmin authenticates as the MariaDB root account;
GitLab's seeded password belongs to the built-in root, not the administrator;
Mastodon's tootctl accounts create never sets a password; phpLDAPadmin binds
with an LDAP DN and a different secret. Further out: MediaWiki removes the form
that would accept the Keycloak secret ($wgPluggableAuth_EnableLocalLogin=false)
and bounces anonymous requests back into the IdP, so the unauthenticated
landing the contract asserts can never be observed; Mattermost Team Edition has
no native OIDC, so Keycloak rides the GitLab OAuth slot behind a relabelled
link; MinIO's console returns redirectRules: null and renders no SSO button at
all; XWiki's administrator is an XWikiUsers object with no password property.
Two flags turned out to be unjustifiable and are gone rather than annotated.
web-app-mailu gated PERSONA_GUEST_BLOCKED on services.dashboard.enabled, but
runGuestFlow does one anonymous goto and asserts no signed-in marker -- it
never touches the dashboard. A leftover from a helper version that entered
through the dashboard account menu. And web-app-mig's rationale claimed the
role is an in-app module of web-app-dashboard; it has its own compose stack,
healthcheck and canonical domain, and dashboard is merely a tile consumer.
Nine of the reasons say plainly that the block is not a role property but a gap
in the shared persona helpers -- decidim's sign-out is a data-method link the
generic click cannot activate, fider's SSO entry is a two-step modal, taiga,
wordpress, xwiki, odoo and penpot bind OIDC on a path the personas never visit,
jenkins blocks unconditionally although its OIDC variant looks runnable, and
opentalk's own spec proves both personas work. That list did not exist before;
it is the actual yield here, not the 89 comment lines.
45 pre-existing comments came into the comment lint's scope because this sweep
touched their files -- it only inspects files that differ from HEAD. 28 of them
were the banner "Service gate flags rendered for closure.", every one sitting
above a line that is not a service gate flag at all, orphaned by the
alphabetical re-sort these templates once got.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The block sat under services but never behaved like a service. A service has
exactly one providing role, declared with provides, so the registry can map the
key to that role and consumers bind their flag to its presence. No role declares
provides: mcp. There are seventeen providers and the pairing is done by
allowed_consumers inside the block itself.
Every rule that enforces the service model therefore needed an exemption for it:
a shared-provider suppression in fourteen roles, a playwright-service-flag
suppression, and two freshly added exemptions in the dynamic-flag and
variants-static-override checks. Four independent rules, four escape hatches,
one cause. They are all gone now, together with a bond key that only the
resource budgeter reads and only for entries under services, and a wordpress
addon bridge that pointed at a service block that no longer exists.
The block moves to meta/mcp.yml. The applications loader maps meta/<name>.yml to
applications.<app>.<name> generically, so the move needs no wiring; the file
takes the same shape as meta/rbac.yml. Every config path follows, the variant
overrides are lifted out of their services map, and the vocabulary module leaves
the services package because it no longer describes one. roles_with_service
gains an explicit topic parameter rather than an implicit fallback, so nothing
silently resolves against the wrong container.
Files that are otherwise unrelated to this move are included where the committed
tree would otherwise keep pointing at a path that no longer resolves: thirteen
vars files, eleven playwright env templates and one task file.
The lint shipped in the previous commit had three defects, all reproduced in a
throwaway copy of the tree rather than argued.
Its documented opt-out could not be used. The docstring said to put
`# nocheck: playwright-env-template` on the first line of the role's spec.
Every spec is a CommonJS .js file, where a leading `#` is a hard parse error --
`node --check` refuses it. Following the instruction broke the file the lint
was protecting. It is `//` now, and the marker may sit anywhere in the head
rather than on line 1, which the mandatory `require` already occupies.
Its suppression check was a bare substring match: `if _RULE in
read_text(...).split("\n", 1)[0]`. Any prose containing the rule name disarmed
it -- proven by deleting a template and writing
`// TODO: someone still owes this role a playwright-env-template`, which the
lint then passed. A typo in the rule name disarmed it just as well. It now uses
`is_suppressed_in_head` from utils/annotations/suppress.py, the shared grammar
every other lint here consumes; this was the only one with a hand-rolled parse.
Its scope followed test_has_spec's `web-app-` prefix, but the mechanism it
guards does not: discover_playwright_roles.py globs
`rglob("templates/playwright.env.j2")` with no prefix filter. Six web-svc-*
roles ship specs. All six carry a template today, so the hole was latent, but
consistency with a narrower sibling is not correctness.
Verified in the copy: missing template -> fails; bare mention of the rule name
-> still fails; a real `// nocheck` in the head -> passes, and node --check
accepts the file. In the tree: tests/lint + tests/integration 532 passed,
1548 subtests, and the lint itself green at 82 specs against 82 templates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_has_spec.py forces every roles/web-app-* to ship a Playwright spec and
calls a missing one a hard error, because the persona contract must never be
silently absent from the deploy capstone. Nothing forced the other half. The
runner discovers roles by rglob('templates/playwright.env.j2')
(discover_playwright_roles.py:53), so a spec without that file is staged
nowhere and executed never.
21 of 76 web-app roles were in exactly that state. Verified rather than
inferred: running the discovery filter directly returns 61 roles and none of
the 21 is among them, `make compose-playwright` refuses without a rendered
.env from a prior deploy, and `git log --all` on those paths shows the file
never existed on any branch -- the single hit is web-app-gitlab on a fork
branch. Their specs have never run once. 15 of the 21 deploy in CI today,
reporting green while their only functional verification never fires.
The new lint mirrors test_has_spec and carries a per-role opt-out that is
used nowhere: uniform beats exceptions.
Every one of the 21 now has a template written against its own
meta/services.yml rather than a shared skeleton -- espocrm splits
ADMIN_PASSWORD from ADMIN_NATIVE_PASSWORD because OIDC_ALLOW_ADMIN_USER
leaves both login paths open, the four permanently auth-less roles declare
their personas blocked instead of shipping a knowingly-red spec, and
web-app-mastodon gained a real test-seaweedfs.js because no honest
suppression existed for a role that does route Paperclip to S3.
web-app-magento moves pre-alpha -> alpha here: it now has a spec that runs,
which is what the lifecycle gate is about.
Six meta/services.yml files change only where adding a template falsified an
existing suppression -- gitlab and shopware both claimed the role 'ships no
templates/playwright.env.j2', which stopped being true in this commit.
Not resolved here, and the adversarial review says so plainly: the specs are
enabled, not proven. None has executed against a live stack. The review found
three classes of defect that land in follow-up commits rather than being
smuggled in silently -- persona blocks that are missing or should be tied to
sso.enabled, eleven suppressions naming a sys-svc-mail test suite that does
not exist, and eight claiming a CSP assertion that cannot fire because no
consumer role renders CSS_BASE_URL.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Defines what a services.mcp block may declare and enforces it: classification,
transport, auth, implementation and the provenance fields that make an upstream
claim checkable. The lint refuses an unknown key anywhere in the block and
refuses a provider whose allowed consumers cannot speak its transport or its
auth scheme.
Discovery resolves each provider's credential and endpoint per consumer and
per deployment, rejecting a server with a named reason. Transport, auth and
endpoint faults are fatal; a missing credential is not, because it also occurs
when the provider is simply not provisioned yet in the same play.
The filters render the Authorization header from the declared scheme, build
Open WebUI's tool-server connections with their group grants, and derive the
members of those groups from the RBAC assignments. Role scoping gains
application-scoped roles, so an mcp role only counts when it is granted on a
specific application.
roles_with_service gains a scope and a direction filter, and drops the canonical
domain requirement for direction-filtered entries: an MCP URL is built from the
service key and the internal port, so a provider without a public vhost stays
valid.
The role shipped a complete probe script at files/test.sh and no
templates/test.env.j2. discover_cli_roles globs the env template alone,
so the role was never discovered and the script had never run — and the
script hard-requires TOR_SOCKS and NGINX_SERVERS_DIR via ${VAR:?}, so it
could not have run without one either. Its own usage hint pointed at a
path that does not exist.
Moving the script to the discovered path and adding the env template
makes both halves line up. Verified against the live stack: the onion
answers through the Tor SOCKS proxy, 1 probed, 0 failed.
The new lint keeps the pair together and rejects a script left at
files/test.sh. The two halves fail differently and only one is loud: an
env template without a script aborts the play, a script without an env
template is never discovered and its silence reads like a pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
autoformat joined the two operands of the message concatenation during
make quality; 1408c6770 had them on separate lines. Formatting only, same
expression and same output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rule checked whether both keys contain the token `omit`. That catches the
Ollama shape, where async carried no omit at all, but it does not enforce what
the failure message claims: two keys can both carry omit and still hang on
different switches, and the old check passed that.
_gate() now reads the switch each key states inline - the test of an
`if ... else omit`, or the left side of a `| ternary(..., omit)` - and the two
must match. A key that names no gate matches another that names none, which is
the shape sys-service and the canonical ASYNC_TIME/ASYNC_POLL pair use; both
then inherit whatever their variables do. An unrecognised omit-bearing form
falls into a single bucket, so it only matches another unrecognised form.
Checked against all five shapes in the tree: the pre-fix Ollama pair is an
offender, the fixed pair and the ternary and bare-variable pairs are not, and
two different conditions are now an offender where the token check passed them.
The rule still does not catch two ungated keys whose variables diverge. That is
deliberate: group_vars makes ASYNC_TIME evaluate to omit while ASYNC_POLL stays
numeric, and a poll without async is ignored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pre-pull task set `async` unconditionally while gating only `poll`. With
async disabled, `poll` became `omit` and ansible fell back to its built-in 15s
poll while `async` still routed the task through async_wrapper: ansible
collected the job itself and then ran `async_status mode=cleanup`, deleting the
job file that tasks/utils/async/reap.yml expects to read. The reap reported a
successful `ollama pull` as a failed async job, failing web-app-nextcloud 2 in
run 30781723102.
Both keys are now switched off together, matching all six peer call sites. On
the resulting synchronous path the reap's `async_reap_tolerate` list no longer
applies, so the tolerated "up to date" answer is carved out in `failed_when`
instead. It keys on `failed` rather than `rc`, so a module-level failure that
carries no rc is not swallowed.
async_failures._which() now honours loop_control.loop_var: the fired result
records the chosen name under `ansible_loop_var`, so a batch failure names
`smollm2:135m` instead of the bare job id.
A second lint case asserts that async and poll carry `omit` together - the rule
this call site was the only one of seven to break.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A service without a healthcheck.flavor or an explicit test only breaks
while its compose file renders, one role at a time during a deploy.
Replay the lookup's own validation against the static config so the
failure surfaces in CI, and reject declarations the lookup would refuse
anyway: an unknown flavor, or a flavor and a test fighting over the
same entry.
Both tests share _probe_defect and import PROBES from the same SPOT the
lookup uses, so the known flavors cannot drift apart. 49 of the 56 call
sites resolve statically; the rest set service_name in an included base
template and are skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The applications registry is built before any role-vars binding, so a
`{{ SOME_ROLE_VAR }}` in meta/*.yml stays an unrendered Jinja string and
silently resolves to nothing. The sibling rule already covers
`application_id`; this extends it to every key declared under
`roles/*/vars/`.
String literals, keyword arguments and filter or attribute names are
stripped before matching, so `lookup('domain', ...)` and
`lookup('rbac_group_path', application_id=...)` are not mistaken for
variable reads. Scope stays at meta/*.yml: files under meta/addons/ are
rendered inside the role's own play, where role vars are bound.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`shell` populates stdout on every path where changed_when is evaluated,
so the default filter guarded nothing: a non-zero exit and even a chdir
failure both carry stdout, a skipped task never evaluates the condition,
and on a module failure Ansible records the conditional error in
changed_when_result while still reporting the real cause.
The async lint walked the tree itself instead of going through the
shared cache. Both selections cover the same 1079 files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An async task is a deployment wide decision taken in one line. Whether it
holds depends on things the line does not show: in swarm a delegated
command crosses a node boundary and runs far longer than the same command
in compose, so a window that never opens on one node routinely opens
across three, and the playwright run starts the moment the play ends, so a
command still writing in the background surfaces as a flaky spec rather
than a failed task.
Require one sentence per async naming what does not consume the result.
"It is slow" is why async is tempting, never why it is safe, and the
docstring says so. There is no nocheck escape: a task that cannot be given
the sentence has not earned its async.
The comment lint learns `rationale:` as a marker so those lines are not
read as narration. It is deliberately narrower than the exception marker
next to it: a rationale is demanded by another lint that refuses the
construct without it, so it cannot be self granted, which is exactly the
failure mode of comments that explain themselves.
This lands after the change it describes, so every commit stays green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
