fix(provision): gate the setup-wizard ansible-group builder on declared roles (#14638)

[mrveiss]

Closes #14638

Thinking Path

Read _ansible_groups_for_nodes (the setup-wizard's own ansible-group
builder, api/setup_wizard.py) and traced everything that feeds it, per
#14638's ask. It builds group membership from the NodeRole assignment
table with no gate at all — unlike services/inventory_builder.py's
build_registry_inventory, which has called
_strip_undeclared_privileged_groups on the group branch since #14513/#14552.
This is the second half of the shape #14594/#14637 fixed for the hostvar
branch (node_roles_declared): same defect class, different code path
(_ansible_groups_for_nodes vs _apply_role_host_vars), which is why it was
filed and fixed separately.

Root cause: _ansible_groups_for_nodes(node_roles, node_id_to_inv_name) in
api/setup_wizard.py:113 (pre-fix) unions two group vocabularies
(services/inventory_builder.groups_for_role_tokens and the legacy
services/role_registry.ROLE_ANSIBLE_GROUPS) for every NodeRole row it is
given, with nothing filtering the privileged groups
(slm_server/backend/main/frontend/aiml/npu/browser,
inventory_builder._DECLARED_ONLY_GROUPS) the way the dynamic-inventory
builder's own group branch already does.

What Changed

• api/setup_wizard.py::_ansible_groups_for_nodes now takes db_nodes (Node
  ORM rows) and strips privileged groups a node's Node.roles does not
  justify — reading declared intent straight off Node.roles
  (inventory_builder._declared_roles), the same source fix(provision): close node_roles_declared's setup-wizard fallback dependency (#14594) #14637 uses for
  node_roles_declared, rather than trusting the NodeRole table to stay
  declared-only. _build_inventory_children and _generate_dynamic_inventory
  thread db_nodes through.
• Added _groups_for_role_tokens_both_vocabularies: both the "raw" and
  "declared" group sets are computed through the same union of the canonical
  map and the legacy ROLE_ANSIBLE_GROUPS map. _strip_undeclared_privileged_groups
  itself is deliberately NOT reused as-is — it only consults the canonical map,
  and the wizard's raw side also unions in ROLE_ANSIBLE_GROUPS (e.g. vnc
  reaches backend only through that legacy map). Reusing it unmodified
  would have stripped a genuinely declared vnc node's backend membership —
  caught by mutation-testing my own first draft against the existing
  test_no_group_this_path_already_emitted_disappears parity test.
• An unresolved node (a NodeRole row whose node_id has no matching
  db_nodes entry) fails closed: its privileged groups strip to nothing
  rather than passing through unfiltered.
• Extended tests/services/test_wizard_inventory_parity_14286.py (already the
  colocated, correctly-real-loaded home for this builder's tests) with 8 new
  tests covering the detected-only, declared, legacy-vocabulary, unresolved-node
  and end-to-end-via-_build_inventory_children cases.
• Updated the existing _children_for test helper to mark every role as
  declared (matching the file's existing "vocabulary parity" scope), and
  _build_inventory_children's only other test call site.

Verification

Full enumeration of every path from a detected role to a privileged group
or privileged activation fact, across both the dynamic-inventory builder and
the setup wizard (the deliverable #14638 asked for):

#	Path	Gate	Verdict
1	services/inventory_builder.py:438 build_registry_inventory GROUP branch	_strip_undeclared_privileged_groups (#14513/#14552)	SAFE
2	services/inventory_builder.py:327 node_roles hostvar (raw union)	privileged facts don't read this branch any more (#14560/#14589 repointed them)	SAFE — non-privileged facts (tts-worker/redis/vnc/xrdp/llm) deliberately still do (#9965)
3	services/inventory_builder.py:334 node_roles_declared hostvar	_declared_roles(node) = Node.roles only	SAFE (#14560/#14589)
4	api/setup_wizard.py _ansible_groups_for_nodes GROUP branch	was ungated	FIXED by this PR (#14638)
5	api/setup_wizard.py:394 node_roles hostvar (raw union from NodeRole table)	same reasoning as #2	SAFE
6	api/setup_wizard.py node_roles_declared hostvar	_declared_roles(node) = Node.roles directly	SAFE (#14594/#14637, e8517c3e6)
7	api/setup_wizard.py _inject_co_located_ai_stack (#3461)	stamps both node_roles and node_roles_declared; never reaches the GROUP builder at all (only the in-memory hostvar dict)	SAFE / deliberate, unaffected by this PR — confirmed by regression test
8	ansible/playbooks/vars/role_active_facts.yml five privileged facts	both branches (group + node_roles_declared) now gated on both builders	SAFE
9	Static inventories (ansible/inventory/slm-nodes.yml, CI fixtures)	operator-authored, definitionally declared; node_roles_declared fallback intentionally preserved	SAFE by design
10	Every other caller of groups_for_role_tokens / ROLE_ANSIBLE_GROUPS	grepped — none exist outside the two builders above	N/A, no third path

Also audited every NodeRole(...) / Node.roles = write site in
api/, services/, main.py (the thing both builders' "declared" side
ultimately trusts): SLM-manager bootstrap and node_seeder.sync_slm_node_roles
are bounded by a hardcoded/existing declared set; compose_fleet.py's
_ensure_compose_nodes derives from a hardcoded topology table (declared by
construction, not agent-probed — a producer not previously named in #14594's
provenance table, documented here for completeness); sync_orchestrator.py's
role-sync only touches an operator-selected role already on the node;
api/nodes.py, api/npu.py are authenticated admin endpoints;
deployment.py/reconciler.py/blue_green.py only ever subtract roles or
move already-declared roles between paired nodes. The one place
Node.detected_roles (the untrusted agent heartbeat probe) is written
(api/nodes.py:1872) never reaches Node.roles or the NodeRole table —
detection stays correctly firewalled from declared intent everywhere.

Test run (local design-aid only — CI is authoritative):
pytest tests/services/test_wizard_inventory_parity_14286.py — 16/16 passed
after the fix.

Mutation: reverted the gate in _ansible_groups_for_nodes (dropped the
_DECLARED_ONLY_GROUPS stripping) → 11 passed / 5 failed, all 5 failures the
new detected-only/legacy-vocabulary/unresolved-node/end-to-end tests, exactly
as expected. Restored the fix → back to 16/16.

Also ran the full existing coverage for this area unmodified —
setup_wizard_privileged_node_roles_test.py,
inventory_privileged_node_roles_test.py, inventory_detected_roles_test.py,
node_roles_declared_producers_test.py, inventory_deploy_groups_test.py:
43/43 passed. A broader autobot-slm-backend/api autobot-slm-backend/services autobot-slm-backend/tests run: 2189 passed, 3 skipped, 5 failed — the 5
failures are services/hf_token_validator_test.py, pre-existing and
unrelated (fail identically in complete isolation, before touching any file
this PR changes — a local httpx-import gap in this environment).

Model Used

Claude Opus 5 (1M context)

[mrveiss]

Review — APPROVE

The enumeration is the deliverable I asked for and it is the reason this round should be the last one. Three previous rounds each closed the door someone happened to be looking at; ten paths audited with a verdict each, only #4 open, is what ends that pattern.

The catch on _strip_undeclared_privileged_groups is the part that would have made a naive fix wrong, and I verified it:

services/role_registry.py:508      "vnc": "backend"      # legacy vocabulary
inventory_builder.py _ROLE_TO_GROUPS   (no vnc entry)   # canonical map

The wizard's raw side unions both vocabularies; the canonical stripper consults only one. So reusing it as-is would have computed the declared side from the canonical map while the raw side included the legacy grants — and a vnc node's legitimately-granted backend group would have been stripped as "undeclared".

That is an over-tight gate: a node silently dropped from a deploy it needs. Quieter and worse than the crash being fixed, and the exact failure direction I have been asking each round of this chain to check. Confirmed the fix routes both sides through _groups_for_role_tokens_both_vocabularies, so they are compared like for like.

Worth noting how close that came: vnc → backend is the same mapping behind #14513/#14552 in the first place — a vnc node running npx vite. This vocabulary is load-bearing in both the original bug and its fix, in opposite directions.

And it was found by mutation against the existing parity suite, not by inspection. That is the difference between a test suite that documents behaviour and one that catches you. Reverting the gate takes 16/16 to 11 passed / 5 failed — exactly the new tests, so they fail for the reason they exist.

The trace is properly specific: _ansible_groups_for_nodes built group membership from every NodeRole row with no gate at all, unlike build_registry_inventory, which has called the stripper since #14513/#14552. The fix reads Node.roles — declared intent — the same way #14637 does, removing the dependency on the assignment table rather than pinning its semantics with a test. Consistent with the fix one path over, which matters for whoever maintains these next.

Reporting the previously-undocumented producer (services/compose_fleet.py) as safe-but-noted is the right level of thoroughness: a producer nobody knew about is exactly how this chain kept reopening, and recording it costs nothing now.

Separating the 5 pre-existing failures by isolated re-run before attributing them is also correct — I would rather see that check than a claim they are unrelated.

Merging once the required contexts and all twelve shards confirm green.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!