Commit Graph
636 Commits
Author SHA1 Message Date
kevinveenbirkenbachandClaude Opus 5 b804060e54 fix(lint): let the two port lints see the edge proxy as the edge
b036f6003 gave svc-prx-openresty the ports it had been binding as literals, including ports.public.http and the onion flag that turns the node onion's port 80 into a declaration instead of a hardcoded torrc line. The matching lint change stayed in the working tree, so that commit does not pass its own gate: the onion-flag lint excludes http wholesale, never finds the category among the declared ones, and reports the flag as out of scope.

The exclusion was right for the 128 roles that declare http, sso or websocket on the loopback and are reached through the proxy on the onion's port 80. It was wrong for the single role that publishes http on the host, because that role is the proxy. The lint now splits on scope rather than on category: exempt under local, decision required under public.

The companion lint gets the same treatment it needed for the dict-form call sites: the ports block is tracked by the indentation of its key rather than by the next line that looks like one, since target, published, protocol and mode are all valid keys and closed the block while it was still being read. Under expose: only a real host:container mapping counts, because there the single-segment form documents a container port and publishes nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 17:03:18 +02:00
kevinveenbirkenbachandClaude Opus 5 46491b1d3a test(lint): check the shell scripts that ship as jinja templates
make lint-shellcheck walks files on disk, so a .sh.j2 is invisible to it. The thing that actually runs on the host is the render, and nothing looked at the render until it failed on a node. That is an unpleasant place to discover a quoting mistake in a script which runs as root and rewrites the host packet filter.

Every roles/*/templates/*.sh.j2 is now rendered and handed to bash -n, plus shellcheck where it is installed. A value the check does not know renders as a placeholder token, an unknown collection renders as empty, an unknown filter passes its value through, and lookup returns a token, so a template written for ansible still renders to whole bash and the check judges the shape of the script rather than the values one deployment happens to supply.

SC2034 is excluded with its reason: emptying a conditional branch orphans the variables that branch used, so a partial render cannot tell a genuinely unused variable from one whose only reader was not rendered. Everything else stays on.

First run, first finding: the backup ssh wrapper substituted with backticks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 17:01:27 +02:00
kevinveenbirkenbachandClaude Opus 5 b71cf359b4 feat(networks): declare the deployment's own address space once
The private blocks were written out wherever something needed to say "this is us": NETWORK_TRUSTED_PROXY_CIDRS listed all three, Tor's SocksPolicy repeated them, unbound's access-control repeated them again. Four copies of a fact, and nothing tying them together, so narrowing one narrows nothing else.

Worse, the block role networks are carved from was never declared at all. The allocator behind 'infinito contributing network address suggest' derives its umbrella from the subnets already in use, so the tree defined the rule instead of the rule defining the tree. Anything consuming "our own address space" had to guess it, and a guess that is one role out of date silently excludes that role. Measured: 106 of 107 role subnets sit in 192.168.0.0/16 and one does not, so a firewall range derived from the pool by eye would have cut BigBlueButton off.

The blocks are now named once and everything else is derived, NETWORK_TRUSTED_PROXY_CIDRS included, which renders byte-identically to before for its four consumers.

BigBlueButton stays outside because its upstream pins both the block and its gateway, which meta/services.yml repeats as upstream_host 10.7.7.1. The exception list therefore carries the role id rather than the CIDR: the block is already declared in that role's meta/networks.yml, and repeating it here would be the same duplication one layer up. The role_subnets lookup resolves the ids and raises on a role that declares no subnet, because a silently dropped entry would narrow whatever firewall or trust list consumes the result.

A lint keeps the declaration honest in both directions: every role subnet must sit in the declared space, and an exception whose block the pool already admits is rejected so it cannot outlive its reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 17:00:17 +02:00
kevinveenbirkenbachandClaude Opus 5 b036f60039 refactor(container_ports): express the remaining published ports through the lookup
The list form could publish a host:container pair and nothing else, so thirteen blocks stayed hand-written: the ones needing a protocol suffix, a port range, the swarm long syntax with mode host, or a container-port-only publish. Each of those duplicated numbers the role already declares in meta/services.yml, which is the drift the lookup exists to remove.

The lookup keeps the list form unchanged for its 93 existing call sites and gains a dict form: transport emits one item per protocol, container 'same' serves the services that cannot remap (STUN/TURN, media relays), mode 'host' emits the long block an edge role needs in swarm so the client address survives the hop, publish 'ephemeral' emits the container port alone, and key names the block for Discourse's launcher file, whose expose: list is what becomes docker run -p there. A declared {start, end} mapping renders as start-end on both sides, so a relay range needs no term of its own.

svc-prx-openresty had no ports block at all and bound 80 and 443 as literals in its template, while torrc.j2 carried a third literal 80 for the hidden service. None of the three referenced the others. openresty now declares the ports, the compose template reads them, and the onion's port 80 comes from ports.onion.http through TOR_ONION_EXTRA_PORTS.

Three blocks stay raw, each with the reason it cannot route through one call: keycloak's sso_proxy maps a published port owned by the consuming role to a container port owned by web-app-keycloak, and matrix's ansible flavor takes element on 8080 and federation on 8048 from the upstream playbook rather than from this repo's ports.internal.

The enforcing lint scanned only files with compose in the name and only double-quoted list items, so nine of the thirteen blocks were invisible to it. It now reads every role template that can carry a ports block, catches unquoted items, protocol suffixes, ranges, flow style and the long syntax, and tracks the block by the indentation of its key: keying on the next line closed the block on anything shaped like a key, which target, published, protocol and mode all are. Under expose: only a real mapping counts, because there the single-segment form documents a container port and publishes nothing.

Verified by rendering all 110 container_ports call sites through the real templar, the real lookup and the real merged applications view.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 14:55:25 +02:00
kevinveenbirkenbachandClaude Opus 5 dd05caa34f feat(services): require an explicit onion decision per host-bound port
A port under ports.local or ports.public binds an address on the stack host, which is what a HiddenServicePort can forward to. Whether it should be forwarded has a cost in both directions: silently off leaves a service unreachable that the operator believes is on the onion, and silently on publishes a database, an admin interface or a SOCKS proxy to everyone holding the onion address. Neither belongs in a default, so the flag is now mandatory and every false carries the reason it is false.

The first run produced 165 findings, of which 131 were one architectural fact repeated rather than 131 decisions. Four groups are therefore settled once in the lint instead: the proxy-fronted http, sso and websocket, because the node onion already forwards port 80 to the reverse proxy that routes every vhost, oauth2 hop and socket upgrade behind it; the implicit-TLS variants, because Tor is already the transport encryption and container_ports drops those listeners when TLS is off, which an onion deployment always is; the categories a hidden service cannot carry, listed once in plugins.lookup.tor_ports.UDP_ONLY_CATEGORIES, which the lint imports rather than restates; and any entity that already answers the question through exposed:.

That leaves 23 real per-port decisions, each with a reason naming its own consumer. svc-net-tor keeps its own socks, trans and dns off the onion: a published SOCKS port is an open proxy, and SocksPolicy cannot refuse it because the forward arrives from the node's own address.

Mailu turns smtp, submission, imap and pop3 on, which is what makes sending and receiving work over the onion, and leaves sieve off because ManageSieve manages filters rather than mail. Measured end to end through the merged applications view, across all 105 roles the deploy now forwards exactly 25, 110, 143 and 587.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 14:51:59 +02:00
kevinveenbirkenbachandClaude Opus 5 3d3defcaae style(lint): wrap the no_log failure message the way autoformat does
e90cac4ee2 staged the lint but not what the gate rewrote after it. Whitespace only; the join, the fix hint and the assertion are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 03:37:21 +02:00
kevinveenbirkenbachandClaude Opus 5 15a6251d72 fix(api): resolve credentials against the declaration an inventory override hides
API is a plain mapping, so an inventory that configures three providers replaces the whole declaration from group_vars/all/18_api.yml instead of merging into it. inventories/development/default.yml declares openai, anthropic and openrouter, so github, google and jira did not exist in CI at all and lookup('api', 'github.client_id') raised.

A raising lookup inside a value the applications renderer is templating does not fail the render: _templar_render_best_effort returns the string untouched and, for a value carrying a non-env lookup(, refuses the fallback as well. The raw template text reached plugin_value.enabled | bool, which coerced it to False and emitted the deprecation warning that fails the deploy gate. Run 34893363518, job 104153194109 lost web-app-nextcloud to exactly that, for three integrations at once. The configvalue entries would have stored template text as the client id.

utils/api.py merges the scope's override over the declaration, so an override narrows values and never the provider set, and holds the enable rule: every credential a provider declares must carry a value. The api lookup reads through it; the new api_enabled lookup gates on it and returns a real boolean instead of a string a filter has to coerce.

The same class sat in group_vars/all/16_ai.yml and roles/svc-ai-litellm/meta/secrets.yml as bare API.<provider>.<key> dereferences; both now go through the lookups.

Two lints keep it shut. test_addon_enabled_renders_to_bool renders the whole applications map against a scope with API blanked, the most hostile override an inventory may legally write, and requires every addon gate to come out a boolean. test_api_accessed_via_lookup forbids a bare API dereference in Jinja under roles/, group_vars/ and inventories/.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 03:35:47 +02:00
kevinveenbirkenbachandClaude Opus 5 e90cac4ee2 test(lint): forbid no_log in the test roles
roles/test-e2e-cli and roles/test-e2e-playwright run only in CI, against a
throwaway cluster whose credentials are generated per deploy and discarded with
it. no_log there protects nothing and costs the one thing a red test job exists
for: a masked task reports that its output was hidden, and the whole matrix
round has to run again to learn what a single assertion saw.

The rule is absolute, so it ships without a nocheck escape hatch. A test task
whose output looks too sensitive to print calls for extracting the field the
assertion reads, not for blinding the task.

A second case asserts the glob still reaches the roles. A path rule that
matches nothing passes forever, which is how test_gateway_backend_registered
nearly went quiet earlier today.

Both roles are clean right now, so this starts green. Reproduced against an
injected no_log in run_one.yml to confirm it is not vacuous.

Note for whoever hits it: test_credential_tasks_are_masked scans roles/*/tasks
and roles/*/handlers without excluding these two, and requires no_log on a task
whose printed arguments name a password, secret, token, api_key or private_key.
Nothing in the test roles trips it today, but a future task that does would
deadlock the two rules. The exemption belongs in that lint's file selection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 02:25:53 +02:00
kevinveenbirkenbachandClaude Opus 5 4ce588fea1 test(lint): forbid a JavaScript default for an unset environment variable
`process.env.X || <value>` is a second source for something the deployment
already renders, and the two drift without a sound. playwright.config.js read
`process.env.PLAYWRIGHT_ACTION_TIMEOUT || 30_000` while nothing ever set that
variable, so every run used the unscaled 30 s and no failure said so.

Normalising an absent variable stays allowed, because it substitutes nothing:
`|| ""`, `|| '[]'`, `|| false`, `|| 0` and falling through to another
`process.env` entry only let the caller test absence itself.

The six sites the rule found:

  n8n and flowise built a Postgres port from `|| '5432'` while env.j2 renders it
  from `lookup('database', application_id, 'port')`. Node throws
  ERR_SOCKET_BAD_PORT on the resulting NaN, so an unfed port now names itself
  instead of connecting somewhere plausible.

  The bluesky broker did the same with BROKER_PORT, which compose.yml.j2 renders
  unconditionally. Its INSECURE_TLS default was `"false"`, which is what `""`
  already evaluates to.

  matomo defaulted MATOMO_TRACKING_SCOPE to "sub"; the only comparison is against
  "root", so `""` takes the identical branch.

  moodle and simpleicons defaulted a name their playwright.env.j2 always renders.
  simpleicons already asserts the value is truthy; moodle now fails on the scope
  it cannot find rather than on a name nobody configured.

Four values the code itself owns keep a default under `// nocheck: env-default`:
PDS_INTERNAL_URL (which falls through to requireEnv, not to a literal),
HANDOFF_COOKIE_NAME, HANDOFF_COOKIE_MAX_AGE and PASSWORD_SALT_HASH_ROUNDS. No
role renders any of them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-12 12:07:57 +02:00
kevinveenbirkenbachandClaude Opus 5 e76e3d5b41 fix(nextcloud): compute the sociallogin group mapping key
Ansible renders the values of a structure loaded through include_vars and leaves its keys as written, so the mapping key a564385068 wrote as a Jinja expression reached Nextcloud as its own source text. occ config:app:get sociallogin custom_providers on a live variant-1 instance returns "groupMapping": {"/{{ lookup('rbac_group_path', application_id=application_id, role='administrator') }}": "admin"} while every neighbouring value of the same provider is rendered, scope among them.

Keycloak emits /roles/web-app-nextcloud/administrator, so the mapping matched nothing and the account sociallogin provisions on first login stayed out of admin. occ group:list on that instance shows admin holding only the local administrator while keycloak-ac165525-e9e8-4230-9381-13b1acd83fc0 exists and belongs to no group, and every Playwright spec that needs an admin settings section answered 'Access forbidden'.

An independent reproduction outside this repo showed the same split on ansible-core 2.21.4 and 2.20.6: in a payload of the same shape the sibling value rendered and the key did not, so it is not a version artifact. Nextcloud 34.0.3 settles the other half: Manager.php hands an admin the unfiltered settings of a section and ai is a core section of the always-enabled settings app, so an admin cannot receive 403 there whatever is installed.

Writing the whole mapping as one expression makes jinja2_native, which this deployment has on, return a real mapping with the key computed. The lint takes the class: no meta payload may carry Jinja in a mapping key. It found exactly this one case in the whole meta tree, and it ignores a line that itself opens with the expression so the corrected form does not trip it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-12 10:29:49 +02:00
kevinveenbirkenbachandClaude Opus 5 7ccd2c185c fix(n8n): resolve the hosting node before the condition that uses it
Ansible templates delegate_to before it evaluates the task's when, so a task delegating to N8N_HOST_NODE kills the play with 'N8N_HOST_NODE' is undefined whenever the resolver that sets it was skipped, even though the delegating task would have been skipped too. 43e09ebf0d put the resolver inside the same conditional block. Run 34653754175 shows the pair directly above the failure: the include skipped on the false condition while the setup call had returned status 200, and the play ended at 01_bootstrap.yml:92.

The resolver now runs unconditionally, before the block. It costs one lookup per deploy and changes no state.

The new lint reads the class rather than this instance: a conditional resolve_host_cid include whose PREFIX_HOST_NODE a delegate_to in the same role reads, unless an unconditional resolver or set_fact defines that prefix role-wide. The walker carries the condition through block, rescue and always, so an include nested in a block does not read as unconditional. Against the pre-fix file it reports would flag: ['N8N'].

It found one more: web-app-yourls/tasks/install.yml wrapped its tasks in a block whose when repeated the IS_STACK_HOST condition that main.yml already carries on the include. The wrapper is gone and the tasks are dedented; nothing had broken there because the condition always held.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-12 09:29:37 +02:00
kevinveenbirkenbachandClaude Opus 5 b3ce43a596 fix(security): require no_log on every task that carries a credential
At -vvv Ansible prints each module argument and result, so a password in
a command line, a request body, a header or a fact lands in the deploy
log. Nothing enforced masking outside the MCP task files, and no check
looked at parents at all.

A new lint, test_credential_tasks_are_masked, finds tasks whose printed
parts (module arguments, environment, loop) reference an identifier ending
in password, secret, token, api_key or private_key, or
secrets.credentials. A task passes when it or an enclosing block sets
no_log, when every file that pulls it in does so through include_tasks
with apply.no_log or import_tasks with no_log (transitively), when the
secret only reaches a parameter the module masks itself, or when the
module masks its values. no_log on an include_tasks task itself does not
count, because it does not reach the included tasks. The rule can be
suppressed per task with `# nocheck: credential-masking`, now listed in
the suppression catalog, and the lint tests its parent and suppression
logic on synthetic cases.

It found 39 unmasked tasks in 19 roles, all real: join tokens, client
secrets, API tokens in Cloudflare headers, database passwords in
command lines, the ChatGPT bot login body and more. Each now sets
no_log from MASK_CREDENTIALS_IN_LOGS. The sys-dns-hetzner-rdns alias
hetzner_no_log, which only restated that switch, is gone.

Not covered yet: template and copy tasks whose rendered file carries a
secret print it as a diff under --diff.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-11 21:52:18 +02:00
kevinveenbirkenbachandClaude Opus 5 59d45ba665 style: apply autoformat normalisation
What `make quality-high` rewrote on its own, kept apart from the changes it ran
against so neither has to be read through the other.

The matomo healthcheck test moves from a flow sequence to a block one, which is
the formatter's threshold for a long line and not a change to the command: both
revisions parse to the same two-element list, escapes included. The healthcheck
timing lint loses two blank lines between methods.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-10 00:34:23 +02:00
kevinveenbirkenbachandClaude Opus 5 57f90fb731 test(lint): an upper bound must scale even when the literal hides behind a name
The rule read the character after `timeout:` and objected to a digit, so a
duration bypassed the helper the moment it was given a name. That was already
reachable before, and it became easy to reach now that a poll interval is
allowed to stay unscaled: the two look identical at the call site and only one
of them is correct.

They are opposites, and the rule now says so. An upper bound must scale, because
the answer over a Tor circuit arrives later and a fixed cap expires before it
does. A poll interval must not, because scaling one makes a slow target notice
its element later rather than sooner, which is the reverse of the tolerance the
scaling exists to buy. So `timeout:` rejects a CONSTANT_CASE name as well as a
digit, while `waitForTimeout` and `setTimeout` keep matching on a leading digit
only and accept a named interval.

Across the 1307 `timeout:` sites in scope the sharpened rule finds exactly one,
and it is a true positive that wanted saying out loud. The n8n LiteLLM spec
passes a completion timeout into an `n8n-nodes-base.httpRequest` node: not a
Playwright wait at all, but a parameter for a request n8n makes to LiteLLM
inside the deployment, which never crosses a circuit. Leaving it unscaled is
right and was invisible; it now carries the sanctioned marker and its reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-10 00:32:36 +02:00
kevinveenbirkenbachandClaude Opus 5 4ac07edaea refactor(meta): move the info-media exemption to the file it speaks about
The marker names the reason a role has no upstream homepage or demo video, and
it sat in meta/main.yml because a comment-only info.yml parses to an empty
document, which the schema lint rejected outright. So the exemption lived one
file away from the two fields it explains, and a reader looking for the video
found neither the field nor the reason it was missing.

The schema lint now accepts an empty document exactly when it carries the
marker, and that is what allows the move. The media lint reads the marker from
info.yml rather than main.yml, and its failure message names that same file, so
the fix it suggests lands where the absent field would have gone.

A hundred and two roles follow. Every occurrence was a standalone comment on
the second line of main.yml, directly under the document start, so the move is
a line delete and an insert at the same offset. Seventy-three roles already had
an info.yml and gain the line; twenty-nine get one whose whole content is the
document start and the marker. All hundred and two reasons carry over verbatim,
including the thirty-five bespoke ones.

Nothing downstream sees a change. Of the four readers of meta/info.yml, three
coerce a null parse through `or {}` behind an isinstance guard, and
utils/cache/applications.py skips a falsy parse before assignment, so
applications.<role>.info stays absent for the twenty-nine exactly as it was
while the file did not exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 22:26:38 +02:00
kevinveenbirkenbachandClaude Opus 5 96408f23a0 perf(healthcheck): let the start window outlast a real boot
start_interval is the cadence while start_period runs, and only while it runs.
Once the window closes the probe falls back to interval, so a service that
becomes ready just after it pays a whole tick anyway and the cadence never
reached it. Six of the seven flavors were shaped that way, Curl worst: a thirty
second window against a one minute interval.

Measured across 151 converge waits in four job logs, 63 sit exactly on a tick
boundary, 36 in the thirty second step and 27 in the sixty second one.
Attributed by application, web-app-seaweedfs is the largest at 1501s over
sixteen waits with a median of 57.7s: it probes with curl and fell straight into
the gap. web-app-zammad probes with connect, whose window is fifteen minutes,
and its 154s median is real boot time with nothing to recover.

So the window moves to ten minutes and the steady state interval to two, and the
flavor overrides that sat below them go: Curl, Wget and Connect lose interval
1m, Http its twenty second window, Nc its ten. Connect keeps fifteen minutes.

Neither number costs anything while a container works. start_period delays
nothing, it only stops failures counting, and interval no longer touches
convergence at all since that hangs on the window alone; it is steady state
load, which drops from 120 probes an hour per service to 30.

Forty six role files follow. Twenty seven declarations go that repeat the new
default or undercut it, an undercut window being the defect itself. Eight
windows longer than ten minutes keep their value and now say why. The literal
blocks and the three Dockerfiles this repository builds spell every timing out,
because outside the lookup an omission takes docker's value and not the
platform's.

A fourth lint rule keeps the shape: start_period must outlast one interval,
checked against the effective values, so raising interval without the window
cannot quietly bring the cliff back.

Two faults in the lint surfaced while building it and are fixed here. It
compared timings as strings, so 600s and 10m read as different values, and it
matched the healthcheck key exactly, so a line carrying a trailing nocheck was
skipped whole, which is how the two swarm-init partials went unexamined.

One regression, bounded. A container that starts and never turns healthy is
neither rejected nor errored, so stack_ready.sh only calls it fatal after three
killed tasks, and each cycle now spans the longer window. That reaches twenty
minutes rather than six, capped there by swarm_stack_converge_retries at 600
with a two second delay, and it lands only on runs that were failing anyway.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 21:54:26 +02:00
kevinveenbirkenbachandClaude Opus 5 e2cca6b120 test(lint): a healthcheck timing matches its default or says why
Three checks over the two places a timing can be written.

In meta/services.yml the lookup supplies the defaults, so a declaration is
either noise or a decision. A value equal to what the service would have
inherited is deleted rather than justified: it changes nothing and hides the day
the default moves. A value that differs is kept and carries its reason.

That reason is required for interval, timeout and start_interval only. Those
three cost something: steady state load for the life of the deployment, the
threshold at which a slow probe counts as failed, and the only timing that moves
the moment a stack reports healthy. start_period and retries buy patience with a
container that is already broken and cost nothing while it works. Demanding a
defence for them would have produced sixty one sentences nobody had evidence
for.

Outside the lookup, in a literal healthcheck block and in a role's own
Dockerfile, an omission is a finding too. Nothing there inherits from the SPOT,
and docker has no daemon-wide default to fall back on, so a missing key lands on
docker's value rather than the platform's. They disagree on start_period, zero
against thirty seconds, which is exactly what left start_interval without a
window in every hand-written block.

Comparison is against the effective default, resolved through the same compose
the lookup uses. Curl probes every minute, Connect grants a fifteen minute start
window, and a prefix raises timeout, retries and start_period further; measuring
against Probe alone would have called out every service that simply agrees with
its flavor.

Negative control for each check. Before the three commits that precede this one
the rule named 47 restatements, 20 unexplained deviations and 34 definitions
outside the lookup. It names none now.

Two limits worth knowing. The line number for a meta timing is found by
indentation and service name rather than by a parser, so an unusually formatted
file makes the rule ask for a reason it cannot anchor; it errs towards asking,
never towards silence. And the Dockerfile reader follows a backslash
continuation but not a comment inside one, which no Dockerfile in the tree uses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 21:16:56 +02:00
kevinveenbirkenbachandClaude Opus 5 b7ce27d40d test(lint): a variable-free healthcheck belongs in meta/services.yml
utils/docker/healthcheck owns the probe timings, and among them start_interval,
the cadence docker uses while start_period is still running. A block written out
in a template bypasses that owner and never gains the key. Nothing stopped it,
which is how forty six blocks ended up without the cadence and a stack converged
no faster than its coarsest interval.

The rule walks every .j2 under roles/, takes each healthcheck: key with the
lines nested under it, and reports the block when its body contains neither {{
nor {%. Such a block can move verbatim, because the lookup writes
overrides.get(key, default) for every timing key and a declaration repeating the
literal values renders the same block plus the cadence.

A block that interpolates is exempt rather than tolerated. Its value resolves in
the template's own scope, which meta/services.yml does not share, so moving it
would be a rewrite and the check would be asking for a different probe.

The opt-out is a nocheck on the healthcheck line or the one above it, resolved
through is_suppressed_at in same-or-above mode, the idiom both sibling rules
already use. It is meant for a block that interpolates nothing yet still cannot
move: a partial rendered for another role's application_id, or timings the
flavor defaults would make worse.

With the two existing rules this closes the triangle. One requires every call to
find a declaration, the second requires every declaration to reach a call, and
this one requires a declaration to exist wherever one is possible.

Negative control: run before the four nocheck markers were added, it named
exactly those four blocks and nothing else; run after, it is green.

One known limit. The body ends at the first blank line, so a block a future
author splits with one would be read only as far as the gap. No block in the
tree is written that way today, checked across all twenty one that remain
literal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 20:33:14 +02:00
kevinveenbirkenbachandClaude Opus 5 d611131423 fix(lint): enumerate docs/ without asking git
The check listed docs/ through git ls-files so untracked build artefacts stayed out of scope. The exclusion was right and the instrument was not: the gate runs the lints in the debian container, which owns the checkout under a different uid, and git refuses such a repository outright. git ls-files docs exited 128 there while the same test passed 483 times on the host, so the failure was visible only in the environment the gate uses.

iter_non_ignored_files walks the tree and applies .gitignore, which excludes the same artefacts without a git invocation. test_urls_reachable already falls back to it for this reason; using it directly drops the fallback branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 01:48:00 +02:00
kevinveenbirkenbachandClaude Opus 5 32698fc7ab test(playwright): fail a helper no spec and no sibling reaches
playwright.config.js collects **/*.@(spec|test).js and nothing else, so a .js file that no sibling requires never runs while still reading as part of the suite. It survives refactors, gets maintained by mistake, and a merge can resurrect one long after the spec that used it was renamed away: web-app-openwebui/files/playwright/test-ollama-chat.js came back exactly that way.

A file now earns its place by matching the collector or by being required through a relative path from somewhere else under the same role's files/playwright/. Resolution follows Node, including the bare and /index.js forms, and stays inside one role because that is the unit Playwright is staged in. .test.js counts alongside .spec.js so the lint cannot drift from the collector.

The repository already satisfies this: 477 files, 185 collected, 292 helpers, none orphaned. That zero is also the evidence the resolver works, since a broken one would flag hundreds. Checked in both directions besides: a planted orphan is caught, the nocheck marker clears it, and renaming it to .spec.js clears it too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 01:47:19 +02:00
kevinveenbirkenbachandClaude Opus 5 3d6b3900e0 refactor(cli): gather the documentation generators under cli/build/docs
Two of them lived inside docs/ next to the pages they render, which is why both had grown a __file__ upward walk to find the repository root, a sys.path insert to import from utils, and a nocheck for each. A generator parked among documents reads as documentation and drops out of the checks that apply to code.

Moving them surfaced six real violations that the old location had hidden: yaml.safe_load instead of the shared cache, two uncached read_text calls, five hard-coded role paths instead of ROLE_FILE_META_SERVICES and ROLE_FILE_META_MCP, two make targets without an alias, and a requirement reference in code. All six are fixed at the root, none with a suppression, and both generators lose their project-root exemptions because a package supplies PROJECT_ROOT.

cli/build/readme joins them so the three generators that write into docs/ sit together. A new lint keeps docs/ to .md, .html, .rst and .nocheck; it enumerates through git ls-files so untracked __pycache__ is not its business. Checked against HEAD, it names exactly the three .py files that moved.

026-unified-addon-syntax stated that the matrix generator stays under docs/requirements/. The lint makes that impossible, so the requirement now records where the generator actually lives.

Verified: every one of the twelve modules under the new tree imports, no old module path survives anywhere in the repository, 482 lint tests and 1246 cli unit tests pass, and both generators run without drift.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-08 23:17:55 +02:00
Alejandro Roman e6596d51f2 fix(stalwart): add the fix PR into main PR 2026-09-09 00:21:58 +04:00
kevinveenbirkenbachandClaude Opus 5 bbfd82f893 fix(profiling): let the task recap reach past the twenty slowest
ansible.cfg enables profile_tasks, whose output_limit defaults to 20. On a deploy round of this size the recap therefore cut off at 66.07s and printed nothing below, so a role whose cost is spread over many cheap tasks could not be attributed at all: in run 33828309745 that left 22.7 of sys-svc-compose 51.9 minutes unexplained, and the same blind spot covers web-app-nextcloud and web-app-keycloak almost entirely.

The callback accepts all, which disables the cap, but the recap prints one line per task execution and pass 1 alone ran over 7500 of them. Since the Actions worker dies silently on ENOSPC while writing its own logs, which is why utils/tests/swarm/matrix.py aborts a step at DISK_FLOOR_MB, the limit stays bounded at 300: about 600 extra lines across both passes.

Measured with a 30-task play: 30 recap entries with this config, 20 with the upstream default forced through PROFILE_TASKS_TASK_OUTPUT_LIMIT. The lint pins the setting so removing the section cannot silently restore the truncation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-04 16:45:44 +02:00
kevinveenbirkenbachandClaude Opus 5 988d7ef11c style: apply make autoformat
isort moved unittest ahead of pathlib and dropped the blank line between them; ruff wrapped the assertTrue message, the re.compile concatenation and the join generator at the 88 column boundary. No regex, condition or assertion changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-04 16:35:14 +02:00
kevinveenbirkenbachandClaude Opus 5 f5c0b2e572 fix(nextcloud): stop doubling the slash on the onlyoffice document-server URL
plugins/lookup/tls.py:102 builds url.base as "{protocol}://{domain}/", pinned by test_tls.py:59. meta/addons/onlyoffice.yml appended another one, so DocumentServerInternalUrl was stored as https://host//.

The connector treats its two URLs differently: getDocumentServerUrl normalises with rtrim, getDocumentServerInternalUrl returns the stored string raw and concatenates the endpoint onto it. Every server-side call therefore went to //healthcheck and //converter. A swarm vhost forwards $request_uri verbatim, so the document server answered 404; a compose vhost uses proxy_pass with a URI part and nginx collapses the double slash. The 404 makes the connector cron write settings_error, and FilesListener then stops adding the onlyoffice script to the Files view, which is why iframe#onlyofficeFrame never appears and the addon spec times out. Observed in run 33828309745 job 100890535389: "GET //healthcheck HTTP/2.0" 404 in the openresty log, and the docx row offering Download as its default action.

The lint generalises it. This class has been fixed one site at a time at least five times before (efee415b21, 035c1efbf3, b0c7c9601e, a96bdbba38, 83636408c9), so the rule now holds repo-wide: a variable carrying an unstripped tls url.base may never be followed by a slash. Definitions that strip it again are excluded, which is what keeps KEYCLOAK_URL and OPENTALK_KEYCLOAK_BASE_URL out of the finding set; measured, no name is defined both ways.

Not fixed here: settings_error is sticky, so an instance already poisoned stays broken after a redeploy. The obvious remedy, gating the deploy on occ onlyoffice:documentserver --check, is a four-step round trip whose steps never succeeded in either artifact, so it needs its own design.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-04 16:07:43 +02:00
kevinveenbirkenbachandClaude Opus 5 bf20562cec style: apply make autoformat
Line-length reflow in three test modules and a stray blank line in zammad variants. In router.yml the formatter also moved the nsswitch lineinfile regex from single to double quotes with the backslashes doubled to match; single-quoted YAML already passes a backslash through literally, so the string reaching lineinfile is byte-identical and the rule still changes only NOTFOUND.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-04 03:40:17 +02:00
kevinveenbirkenbachandClaude Opus 5 02317cc254 style(lint): state the zip invariant the foreign-core lint relies on
ruff B905 flagged zip(entries, anchors) without strict=. The lines directly above reset anchors to len(entries) whenever the counts disagree, so strict=True can never fire and records that pairing; a future edit that drops the alignment now fails loudly instead of silently truncating and skipping the very offenders this rule exists to catch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-04 03:36:51 +02:00
kevinveenbirkenbachandClaude Opus 5 8a3cc63af6 test(lint): guard a foreign entry into another role's core
roles/<role>/tasks/main.yml gates 0N_core.yml behind
`run_once_<role> is not defined`. Entering that body from a different role
with `include_role: {name: <role>, tasks_from: 0N_core.yml}` bypasses the
gate, because the guard lives in main.yml and tasks_from never reads it, so
the body runs once per calling task instead of once per play.

For sys-svc-compose that means the shared compose pull-lock directory is wiped
again, and files/python/pull.py skips a pull only while its lock file exists,
so every stack deployed so far pulls again; under MODE_RESET it also reaches
01_reset.yml, which brings every project in DIR_COMPOSITIONS down and deletes
the directory holding every application's instance, env and volume paths. For
sys-svc-container it drains whatever topic is pending through the
unconditional meta: flush_handlers at 00_core.yml:53.

svc-runner carried the only two unguarded entries in the tree. This is not a
new convention: sys-svc-container is guarded this way at seven existing call
sites, in svc-opt-ssd-hdd, sys-ctl-hlth-csp, svc-swarm-node, desk-docker, and
twice inside sys-svc-compose itself.

The guard belongs at the call site rather than inside the core file.
sys-svc-compose/tasks/00_core.yml sets its flag as its first task on purpose:
it includes sys-svc-container, and utils/network/routine.yml leads back into
sys-svc-compose, so setting the flag first is what breaks the recursion. The
first-task lint records that rationale. Moving the flag to the end to let the
body self-guard would trade one defect for another.

The rule parses the task structures rather than matching lines: it walks
block, rescue and always, reads include_role and import_role including the
ansible.builtin spellings, and accepts a guard whether `when` is a scalar or a
list. A line anchor is still used for the report and the nocheck, not for the
verdict. A scratch control covers seven shapes; two of them, an inverted key
order and an entry nested in a block, escape a line-based rule entirely.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-04 02:40:29 +02:00
kevinveenbirkenbachandClaude Opus 5 57cee99cf9 test(lint): a sandboxed role cannot be pinned to the manager
roles/sys-svc-container/templates/deploy.yml.j2 appends `node.role == manager`
at :11-13 for any role carrying `placement: manager`, and
`node.labels.<SANDBOX_NODE_LABEL> == true` at :18-20 for any role whose
`services.kata.enabled` is truthy. Both land in the same `_constraints` list,
which swarm evaluates as a conjunction. roles/svc-virt-kata/tasks/00_core.yml
:62-70 adds that label only `when: not (IS_STACK_HOST | bool)`, so the manager
never carries it and the two constraints have an empty intersection.

The pin therefore does not move the workload to the manager, it makes it
unschedulable, and the failure surfaces as a converge timeout rather than as a
placement error. That is why web-app-hermes was proposed for such a pin during
the GitLab#1 investigation and why the proposal was rejected.

The existing default-placement-manager rule cannot catch this: it only checks
that a nocheck marker is present, never that the justification holds, so a pin
excused on general grounds still ships. This rule demands its own marker for
the one case that is arithmetically impossible.

It reads YAML rather than matching lines, because `kata` is a top-level
sibling service in meta/services.yml and not a key inside the pinned service
block, so the correlation spans the file. File scope is right: both
constraints are role-scoped, not service-scoped. The line number for the
report and for the nocheck still comes from a match; only the verdict comes
from the parsed document. A templated `enabled` is not statically decidable
and is passed over rather than guessed at.

Green across all roles today, and a scratch control confirms it fires on a
pin plus kata, stays silent on either half alone, on a template, and on the
nocheck.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-04 02:18:11 +02:00
kevinveenbirkenbachandClaude Opus 5 346ad875c6 refactor(mcp): drop the two provenance fields nothing reads
supported_version and minimum_version sat in 24 roles and were read by no template, no task and no lookup. Their only consumer was the audit report version column, which turned false the moment an automated image bump ran: qdrant reported a verified version nobody had verified, two releases long. Both leave MCP_KEYS, so a role that declares them again fails the schema lint instead of carrying a field with no effect.

Requirement 025 mandated minimum_version as a MUST and 035 carried it in an example; both are corrected here, otherwise the repository would contradict its own code.

The gate then surfaced five dead docs.mattermost.com links. Mattermost moved to /administration-guide/ and /deployment-guide/, and every replacement was probed for 200 rather than guessed. The MCP page is gone with no successor, so mattermost now points at mcpserver/README.md in the Agents repository, the source rather than a rendered mirror.

Both concerns share three files - mattermost's meta/mcp.yml, requirement 025, and the generated audit.md, which derives from a source of each - so splitting them would commit an artifact that does not match its sources.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 19:31:08 +02:00
kevinveenbirkenbachandClaude Opus 5 27ba2820f4 refactor: delete five private names nothing reads
Four module-level constants were defined and never read: _OPERATORS in the complexity filter, superseded by the op group of _TOKEN_RE two lines below; _VALID_SWARM_CONDITIONS, which swarm_restart_condition never checks because it maps to one of three literals by construction; _MISSING in services/config.py, where resolution already goes through get(strict=False, default={}); and _DEFAULT_FLAG_VALUES_THAT_ARE_NOT_NAMES in the exec-resolver lint, referenced by no rule in it. The nfs_fstype lookup rebound its variables parameter and returned without reading it.

Every name was private to its module and a repo-wide search over every file type finds no reader. CodeQL flagged them as unused; ruff cannot, because pyflakes has no rule for module globals and treats a rebound parameter as used.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 16:45:19 +02:00
kevinveenbirkenbachandClaude Opus 5 b764b00ba3 fix(keycloak): forward the access token only to consumers that read it
The SSO gate handed X-Forwarded-Access-Token to every oauth2-fronted upstream, so each of the 28 gated roles received a full-scope bearer for the signed-in user while exactly one, n8n, decodes it. The token also grows with the realm's registered origins, and LAM's Apache rejects a header field above 8190 bytes (Moodle#0, run 33689701895).

Consumers now declare services.sso.oauth2.pass_access_token. Without it oauth2-proxy already answers the auth request without the token, and the gate blanks the upstream header rather than dropping the directive, because nginx inherits proxy_set_header as an array and an unset header would let a client-supplied one through. The new lint keeps declaration and use in step in both directions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 09:20:19 +02:00
kevinveenbirkenbachandClaude Opus 5 4164f37981 fix(mcp): give every MCP round an identity provider to sign in with
An MCP client spec signs in through the client's own OIDC button, which
redirects to Keycloak. Every provider put its MCP pairing in the lean variant,
and that variant pinned sso off - so the round that exists to prove the pairing
was the one round without an identity provider. Run 33574056480 shows what that
costs: openwebui's MCP specs reached _shared.js:131 and died on "openwebui OIDC
sign-in button must be visible on the auth page".

The rule now demands two things of the variant that carries the consumers: the
provider pins sso on, and each admitted consumer, in the variant that round
actually pulls in, does not pin it off. The second half matters because a round
deploys every dependency at its own index and falls back to variant 0 only when
the dependency has fewer, so the client shipping alongside is that one and not
the client's variant 0. A role that declares no sso, or pins it to a literal
false because upstream speaks no OIDC, is exempt from a flag it cannot have.

Twenty roles moved to satisfy it. Ten already had a second sso: false pin and
only needed the flip. Six could host the pairing in variant 0, which already
has sso on, and dropped back to two variants. Six kept a third lean variant to
carry the false polarity variants-coverage wants. moodle and mattermost took
swaps instead: moodle's variant 1 is the mandated LDAP-only branch, and
mattermost was already at the three-variant cap with sso on in its variant 2.
nextcloud also turns context_agent on in that round, the ExApp that serves the
endpoint - with it off the round proved nothing it was built to prove.

The rule moves to tests/lint/ansible/services/ because it reads meta YAML and
executes nothing, which is where integration.md says such a check belongs, and
its suppression key joins the catalog it was never listed in.

nextcloud_exapp_reregister becomes nextcloud_exapp_renew: mcp-secret-masking
matches `register:` unanchored, so the substring in the old name read as a task
that prints a secret. register.yml already masks the two tasks that carry one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-02 15:51:24 +02:00
kevinveenbirkenbachandClaude Opus 5 791eee7c9f refactor(mcp)!: prove the endpoint contract with a CLI test, not a deploy probe
The contract check ran as an Ansible task in the middle of every provider deploy, and the two client checks ran the same way. A test that is part of the deploy cannot be run without deploying, cannot be re-run on its own, and reports its verdict as a deploy failure. Twenty-seven task files lose 491 lines to this.

It is now what it always was. Each provider ships files/test/test.sh, templates/test.env.j2 and meta/tests.yml, and test-e2e-cli stages one shared harness beside it: run.sh drives contract.sh in a throwaway container on the provider's own network, then isolation.sh for adapter-fronted providers. The read probe each provider offers moved from the calling task into meta/mcp.yml next to the allowlist it must satisfy, and the schema now rejects a read probe naming a tool outside that allowlist.

The bearer the test presents is the one a client resolves through mcp.credential, via the new mcp_client_bearer lookup over resolve_credential, which moved to utils.roles.applications.mcp so it is the same function mcp_servers renders every client connection with. The probe presented whatever the calling role passed, which for the fifteen adapter-fronted providers is a different secret than any client holds.

Sidecar isolation is read from the engine rather than measured inside the container, so it no longer needs a delegation to the node holding the task: attachment is compared by network name, which drops the gwbridge special case, and a swarm worker says it cannot measure instead of skipping silently.

Retrying is bounded by wall clock against MCP_DEADLINE_SECONDS, which scales through the same lookup('timeout') as the harness budget it has to stay under. A flag that is neither true nor false aborts rather than reading as off.

mcp-provider-probe becomes mcp-provider-cli-test: an enabled provider must ship the test rather than include the task. mcp-probe-bearer becomes mcp-adapter-bearer, tying ADAPTER_BEARER to the declared credential key now that no probe include carries it. A new integration test renders the shared env template for every provider through Ansible's own templar, so a later change to it breaks visibly instead of poisoning twenty deploys quietly.

Hermes and OpenClaw keep their client-side proof as CLI tests of their own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 20:46:26 +02:00
kevinveenbirkenbachandClaude Opus 5 6ac48b7b8a test(services): hold a compose service guard to its services entry
A compose.yml.j2 that wraps a service key in {% if VAR | bool %} has to reach
that service's own meta/services.yml entry, or the template and the
declaration become two switches for one container: the resource model, the
README graph and the network layer read the entry while the container follows
something else.

The reference may sit anywhere in the chain. The resolver walks the role's
vars/ and defaults/ and follows config paths into the resolved application
config, because nextcloud reaches its Talk entry over
addons.spreed.enabled and a resolver that stopped at role variables called
that a bypass. Only guards indented by at most two spaces count: deeper ones
select content inside a service, not the service.

The rule found one real bypass. NEXTCLOUD_MCP_ENABLED read mcp.enabled while
switching the context_agent container, whose entry admitted two peers where
mcp.enabled admits three, so a round carrying only web-app-openwebui ran a
container its own entry declared off. mcp.endpoint.path routes through
app_api/proxy/context_agent/mcp/, so the container is the MCP surface and the
entry was the stale side. Both ends now meet at the entry.

talk_recording carried no enabled at all and fell back to
_looks_like_container, so the resource model billed it unconditionally. It
now reads services.talk.enabled, which is what its guard already computed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 17:55:59 +02:00
kevinveenbirkenbachandClaude Opus 5 30e43cce88 refactor(services): state a refused pairing as off instead of gating it
83c1c729de gave every provider a group_names-gated services entry per MCP
client, including the clients it refuses with mcp_consumer: false. The gate
is what makes the inventory resolver plan provider and client into one round,
so 19 of the repo's 82 co-deploy pairs started a client for a pairing the
provider had already declined. flowise is a live MCP client of qdrant,
homeassistant and prometheus; the other 17 providers were bringing up its
node stack for nothing.

The entries now read enabled: false, shared: false, mcp_consumer: false, and
the variants that pinned them to true are gone. Setting those variants to
false instead is not an option: test_variants_no_static_override forbids any
variant override of a literal base value in either direction.

bond leaves with them, and a new lint holds that line: collect_role_resources
skips a literally disabled entry before it reads any resource key, so a bond
there describes a cost nobody pays. Thirteen entries unrelated to MCP carried
one and are cleared too.

test_mcp_consumer_services_entry demanded the group_names shape for every
client entry, refused ones included, and now demands the opposite there.
dynamic-flag keeps its rule and takes its own documented per-key exemption.

make docs redraws twenty graphs: the edges go, the nodes stay as
'openwebui ❌' and 'flowise ❌', so the refusal is visible rather than read as
an integration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 17:20:24 +02:00
kevinveenbirkenbachandClaude Opus 5 ffdef28da3 refactor(mcp)!: derive a surface's presence from its peers
enabled and shared were hand-maintained in 24 roles and wrong in both
directions. Both now resolve from the mcp_consumer edges already declared
in meta/services.yml: a server takes the consumers it admits, a client
takes the servers that admit it, and the pair renders as "'peer' in
group_names or ...", or False when no peer is admitted at all. setdefault
keeps meta/mcp.yml and meta/variants.yml as the override channel, so
pinning either key still wins.

The resolved block replaces the mapping instead of being written into it.
_build_role_base_config parks the YAML cache's own object under mcp, so an
in-place setdefault injected the derived keys into every later raw read and
made test_mcp_schema pass alone but fail in a directory run.

The derivation keys on peer presence, never on a peer's own value: a
re-entrant read under _RENDER_GUARD returns the unrendered dict.

Four lints now ask the resolver instead of reading meta/mcp.yml, which they
could not do while the key was hand-written. That is what exposed the eleven
missing hostile-path specs the previous commit closed.

Carries one change that belongs to the WordPress probe but shares a file
with this one: the tool allowlist moves to the hyphenated names the adapter
actually serves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 16:13:01 +02:00
kevinveenbirkenbachandClaude Opus 5 83c1c729de refactor(mcp)!: derive admission from the client
allowed_consumers was twenty hand-kept lists of which fourteen were identical, so every new client meant an edit in every provider. A client now declares services.<own-entity>.mcp_consumer: true once and a provider refuses one with mcp_consumer: false, which is also what tells a deliberate exclusion apart from a forgotten entry.

The group_names-gated consumer entries give the inventory resolver the co-deploy edge meta/mcp.yml never had: one of sixty-nine matrix rounds carried an MCP client before, twenty-three role/round pairs do now. bond 0.5 keeps the clients out of round 0 so the host memory budget still holds.

BREAKING CHANGE: meta/mcp.yml no longer accepts allowed_consumers; declare services.<entity>.mcp_consumer on the client instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-31 18:33:22 +02:00
kevinveenbirkenbachandClaude Opus 5 65d226d0d9 fix(container): reach every resolver address through a shell, and lint for it
The container_address lookup does not return a container name in swarm. It
returns a shell fragment, "$(/usr/bin/resolve-container-id <stack> <service>)",
which is only a name once an interpreter evaluates the substitution. The shell
module provides one. The command module does not -- it execs the binary directly,
so the fragment arrives at container exec as literal text and the call dies with
"No such container: $(...)".

Compose hides the entire class: there the same lookup yields a bare name and
command: works. The failure exists only in swarm, and no swarm run had touched
these paths -- both were added by feature commits (b23653adf8, 0421ebeb16) that
wrote the same argv shape independently, which is the argument for a rule rather
than six edits.

Six call sites moved to the shell module: the moodle egress task, the three
moodle AI-gateway tasks, and the two wordpress AI-Engine tasks. The wordpress
pair keeps its stdin, so it uses the dict form with cmd:. Moving off argv exposes
the surrounding values to word splitting, so they now go through quote -- the
address itself must NOT, because single quotes would suppress exactly the
substitution this depends on.

test_argv_exec_needs_shell.py closes the gap that let this through.
container-exec-resolver checks that the resolver is used; it has nothing to say
about whether anything can evaluate the result. The new rule checks that other
half. It reuses the sibling's argv parser, which recognises only container/docker
in first position, so an argv list that does start an interpreter
(argv: [bash, -lc, ...], as joomla does) is correctly not matched.

Run against the tree the rule reported exactly the five sites found by hand plus
bigbluebutton, and passed over joomla and the discourse task, whose address is a
config lookup rather than the resolver. Bigbluebutton is pinned to compose via
compose_mode_force and can only ever see a bare name, so it takes a nocheck.

Five container-exec-resolver markers disappear with the argv form: under shell
the resolver is the ordinary path, not an exception.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-31 11:39:38 +02:00
Alejandro Roman 333079cc2a feat(UI): fix migration, errors happening in local deploy and swarm integraton + merged main 2026-08-30 21:39:52 +04:00
kevinveenbirkenbachandClaude Opus 5 4a0d6b9f5d style(lint): unwrap the stack-var pattern the way autoformat wants it
880bec33bb wrote the regex across three lines; it fits on one under the line
limit, so `make quality` rewrites it on every run. The pattern is unchanged -
identical once whitespace is stripped from both revisions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-30 15:25:20 +02:00
kevinveenbirkenbachandClaude Opus 5 5f7ff5a1c1 test(lint): let the host-fact rule see what its name promises
The rule read one of the two ways a fact is written, and it read only
templates. Both gaps hid the same kind of value.

Bracket subscripts were invisible: ansible_facts.default_ipv4 was matched,
ansible_facts['default_ipv4'] was not. Both forms are now generated from one
key list rather than spelled out.

Role vars and defaults were out of scan. A fact assigned to a variable there
reaches the template exactly as if it had been written in the template, and the
only real violation in the tree was sitting in that blind spot, which is how it
survived the rule that exists to catch it. The scan covers vars/main.yml and
defaults/main.yml through the mapping constants, 839 files where it saw only
templates before.

Interface addresses were not a pattern at all, only the named default_ipv4. A
resolver reading the docker bridge off ansible_facts['docker0'] cost thirteen
jobs in one run and was removed by hand; the rule that should have caught it did
not know the shape. Any ansible_facts[<iface>]['ipv4'] is now matched, and the
legacy ansible_<iface>.ipv4 with it.

What stays permitted is the distinction the rule turns on. A fact that ADDRESSES
a host travels wrongly. A fact that describes its PLATFORM does not: os_family,
architecture and processor_vcpus choose behaviour for the node the rendered file
is consumed on, which is the node it was rendered for. All seven ansible_facts
reads in the tree were read individually; five are of that second kind, and a
rule needing a suppression on five correct lines out of seven is not a rule.

svc-net-tor is the sixth and seventh. It reads both the bridge address and
default_ipv4, and meta/services.yml pins the role to the manager, so the fact
describes the node the hidden service forwards on. Declared rather than left
looking like an oversight.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-30 14:24:32 +02:00
kevinveenbirkenbachandClaude Opus 5 880bec33bb fix(roles): name the application in values handed to the stack include
`vars:` on an `include_role` is in scope for everything the include pulls in,
and the expression is evaluated where it is read rather than where it is
written. `sys-stk-full` pulls in the proxy and the database, and each of those
roles sets `application_id` to its own id, so a value written with
`application_id` answers a different question by the time anyone asks for it.

It surfaced as a missing key in a role that never declared the value: a
repository url written by one app was looked up on `svc-prx-openresty`, and the
git task died on an unresolvable argument rather than on anything to do with
the url.

The seven definitions here are the ones the included chain actually reads, all
of them `compose_repository_url` / `compose_repository_ref` sources. Naming the
application literally makes each expression answer the same in every scope it
travels through. Values that stay inside their own role keep reading
`application_id`; nothing downstream evaluates them.

`4ce4236585` fixed the other half of the same mechanism a while back, in
`meta/services.yml`, where a role-var was referenced from a value the compose
role resolves. Same failure, opposite end of the lookup.

The lint fixes the form. It judges only the `compose_*` keys, because matching
every handed-over key reported thirteen values, six of which nobody downstream
reads.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-30 12:56:56 +02:00
Alejandro Roman 026b28f9a7 fix(swar-stalwart): migration process 2026-08-30 14:43:00 +04:00
kevinveenbirkenbachandClaude Opus 5 f233d0c30b style(lint): split the role path with the path type
ruff's PTH206 rejects .split(os.sep) on a path. parts[0] is the same first
component here, since the string always has its roles/ prefix removed and so
never starts with a separator.

It reached the branch because the file was rewritten after its ruff run, to
satisfy the raw-walk rule, and the green check belonged to the earlier version.
quality-high's autoformat step caught it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-30 01:00:26 +02:00
kevinveenbirkenbachandClaude Opus 5 c9298033e6 fix(roles): define the entity_name these five roles read
Role vars do not cross role boundaries, so flowise, hermes, homeassistant,
openclaw and openwebui read nothing where they template entity_name into
resolve_service_key. Seventy-seven other roles set it on the line below
application_id.

Ansible does not stop for it. In compose the undefined value renders into task
names as "<< error 1 - 'entity_name' is undefined >>" and the play carries on,
because that path resolves through resolve_compose_cid and never reads the key.
Swarm templates it into a real argument and dies on the first task, and with
any_errors_fatal that ends the play. Reproduced both ways against a replica of
resolve_host_cid.yml: compose ok=4 failed=0, swarm "'entity_name' is undefined".

Nothing could have supplied it either - role vars reach neither a sibling
include nor the parent, so the value was always undefined rather than sometimes
borrowed from whichever role ran before.

The rule that comes with it compares consumption against definition, which no
existing check does; the four scanners under tests/lint/ansible/variables all
run the other way. It skips a file another role pulls in by absolute path,
since that file executes in the including role's scope and reads that role's
value, and it matches the variable rather than get_entity_name, the filter whose
name ends in it. Both exemptions cost a false finding each while I was writing
it. It caught two roles the review had missed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-30 00:37:24 +02:00
kevinveenbirkenbachandClaude Opus 5 87c1ba0371 test(lint): scan the nested task files too, and say what is not scanned
The glob stopped at roles/*/tasks/*.yml while the docstring claimed every
task file of every role. It missed 226 of 916 files - anything under a
nested directory such as tasks/flavor/compose/ or tasks/utils/. The rule
passed and read as full coverage.

The vacuum guard could not catch that. It fails when no file pairs a
docker call with a stack include at all, which protects against an empty
scan, not against a scan that is merely a quarter short.

Widening it surfaces no new finding, so nothing was hiding down there.

The other limit stays and is now written down instead of implied: the
comparison is per file, so splitting the docker call and the stack include
across two task files escapes the rule. Resolving that needs the include
graph rather than two line numbers, and every current caller keeps both in
one file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 20:59:49 +02:00
kevinveenbirkenbach ac02ed134d fix(homeassistant): create the MCP network after docker exists
The image ships docker-ce-cli and no daemon; sys-svc-container installs
and starts that, and a role reaches it through a sys-stk-* stack. This
role pre-created its overlay network before that include, so on the
compose path the branch that probes the bridge ran against a socket that
was not there yet:

  Error connecting: ... FileNotFoundError(2, 'No such file or directory')

It fails late and unrecognisably. The play spends three minutes in the
constructor and the base roles first, the container reports healthy
throughout because its probe cannot ask about a daemon that is not due
yet, and the error surfaces as a network probe rather than as 'docker is
not running'. Swarm hides it entirely, since those nodes have docker
before the play reaches the role.

minio, discourse, openldap, seaweedfs and prometheus all order it the
other way. minio is the closest model: load the stack, create the
network, and let the later flush_handlers start the stack, so the network
exists before anything needs it and docker exists before anything
creates it.

The rule compares the two line numbers in each task file. It caught this
one at line 5 against line 11.
2026-08-29 20:06:43 +02:00
kevinveenbirkenbach 8c4ed6be46 fix(lint-playwright): derive the helper set here too
87f762bd7 replaced the named helper lists in the deploy task and
rerun-spec.sh with a glob. There was a third stager. lint-playwright
keeps its own staging tree, so it kept its own list, and every MCP spec
failed to parse there: 8 of 87 roles, the same 'Cannot find module
./mcp-endpoint' the CI lint job reports.

All three lists held different sets - the deploy task four helpers,
rerun-spec.sh three, this one four again but not the same four - and none
of them the one the specs actually needed.

f14b1ab3f6 already fixed this drift once, by adding addon-gating here to
match the deploy stager. Adding a name to a list that exists three times
is what made a third occurrence certain; the list is gone now.

The rule learns about the third stager as well, since one that goes back
to naming helpers is exactly what nobody notices.
2026-08-29 18:37:20 +02:00
kevinveenbirkenbach 42c7df9ed9 fix(proxy): give the vhost overrides back the concerns they drop
sys-stk-front-proxy prefers a role's own templates/proxy.conf.j2 over a
shared flavour, and an override replaces the flavour whole - including
every cross-cutting include living inside it. Expressing your own
locations should not cost you the MCP policy.

It did. f5272fa85f added that policy to both flavours and to nextcloud,
and the one provider whose native path most needed hiding renders from
its own template, so /mcp kept reaching baserow. The deploy was
failed=0 the whole time: the template produced no line at all, and a
green recap says nothing about a line that never existed. Reading the
rendered vhost is what found it.

Two overrides also lacked the prometheus locations, so nextcloud and
checkmk had no /healthz/live and no per-request metrics. That include
guards itself and emits nothing where prometheus is absent.

The required set is the intersection of the two flavours, so a concern
added to both is demanded of the overrides on the next run without
anyone maintaining a list. Location snippets are excluded, being what an
override exists to replace, and the set is only asked of roles that
actually serve MCP - demanding it of the rest would report noise and
teach the reader to skip the report.

A second rule covers a latent case the intersection cannot: a gated
override needs the oauth2 endpoint and the raised header buffers, both
of which live in one flavour only. All six carry them today, and nothing
made them.
2026-08-29 15:56:13 +02:00