refactor(slm): derive the role list's names, units, health and ports from the role manifests (#16025) by mrveiss · Pull Request #16441 · mrveiss/AutoBot-AI

mrveiss · 2026-09-12T07:59:49Z

Single-issue rationale: this is phase 1 of #16025, delivering the owner's decision that the role manifests are the system of record. Phase 2 (#16435) and the missing manifests (#16436) are filed as sub-issues. #16025 stays open until they land.

Thinking Path

The SLM described role shape twice: role_registry.DEFAULT_ROLES, read by the deploy and inventory path, and autobot-infrastructure/<role>/manifest.yml, read by the reconcile and health path. The owner decided on #16025 that the manifests are canonical and DEFAULT_ROLES a derived projection.

Mapping each consumer's fields to a manifest source showed that nine registry fields have no manifest equivalent yet: display_name, sync_type, auto_restart, source_paths, target_path, post_sync_cmd, required, degraded_without and ansible_playbook. Deriving those would mean inventing values. So this phase derives the four fields the manifests do carry, and pins the rest in an allowlist that can only shrink.

What Changed

New services/role_manifest_shape.py: default_roles_for derives each role's canonical name, its systemd units (as a sequence), its health endpoint and its ports from the loaded manifests. REGISTRY_ONLY_FIELDS names the nine fields still declared by hand.
Multi-unit roles (AC4):
- Role.systemd_service becomes a JSON list, with migration migrations/widen_systemd_service_to_sequence.py.
- Every consumer handles the list: services/role_units.py, services/sync_orchestrator.py (restarts all units), api/roles.py, api/nodes.py, and slm/agent/role_detector.py plus its Ansible mirror (a role is running only when all its units are active).
- The backend role now exposes both autobot-backend and autobot-celery.
The canonical name (AC3) is the prefixed form, e.g. autobot-backend, exposed as canonical_name. The existing name stays, for now: renaming it touches NodeRole rows, the Ansible group keys and the frontend, which is phase 2.
Manifests: celery-beat unit added; slm_roles tags so one manifest can narrow to or fan out across registry roles; the ai-stack deploy source fixed.
Divergences resolved against the SSOT:
- browser-service health 3000 → 9001 (the browser role's playwright_port, bug(playwright): Chromium binary missing — browser automation broken after Ansible deploy #4662).
- chromadb 8000/v1 → 8100/v2 (ssot_config's chromadb field, Knowledge base unavailable after single-host deployment — ChromaDB connected but KB not initialized #3094).
SLM frontend: useRoles.ts, RolesView.vue and OrchestrationView.vue read a role's units as a list. They use the new joinSystemdUnits and parseSystemdUnits helpers, which have tests.

Verification

tests/services/test_role_manifest_shape_16025.py (16 tests):
- The AC5 divergence test: a derivation reflects the manifest it's given, not a frozen copy.
- The registry-only allowlist can only shrink.
- Two end-to-end tests against the real autobot-backend/manifest.yml, checking both units.
slm/agent/role_detector_16025_test.py (4 tests) covers multi-unit detection. useRoles.test.ts covers the list helpers.
test_role_registry.py: three string pins on systemd_service become list equality.
Collisions: no line of api/code_sync.py changed, which is the file fix(code-sync): delete files removed from source through the updater, and check drift across every file (#16310, #16322) #16351 and feat(pricing): live cross-checked prices, refreshed on every install, at first boot and on a set cadence (#16229, #16231) #16236 edit.
Checked statically only (py_compile, ast.parse); repo code isn't run locally, so CI is the runtime check.

Follow-ups, filed as sub-issues of #16025:

SLM role shape phase 2: move registry-only fields into the manifest schema #16435 (phase 2): the registry-only fields, deploy source/destination, and the name rename.
Add autobot-infrastructure manifests for tts-worker, vnc and docker roles #16436: manifests for tts-worker, vnc and docker.

Model Used

Claude Opus 5 (coordinator) with a Claude Sonnet 5 subagent for the implementation.

🤖 Generated with Claude Code

coderabbitai · 2026-09-12T07:59:57Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d19174f1-d6ca-4ba2-a2ae-94d63b575d71

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-09-12T08:25:43Z

Notice: 29 open PRs — past the runaway threshold (25)

There is no PR queue limit, and this is not a request to defer this PR. Work proceeds one issue at a time without a cap on open PRs; review capacity is the constraint.

This notice only means the count is high enough to be worth a glance for a runaway — something opening PRs in a loop, or a merge pipeline that has stalled so nothing is draining.

Currently open:

security(secrets): admin-gate infrastructure hosts, fix vault-backed template names (#16426, #16427) #16442 security(secrets): admin-gate infrastructure hosts, fix vault-backed template names (security(secrets): any logged-in user can list and delete infrastructure hosts through /api/infrastructure/hosts #16426, bug(secrets): the OpenAI and Anthropic quick-add templates save names the vault mirror never recognizes, so the key never reaches the runtime #16427)
fix(infra): map detect-environment.sh onto files that exist, and drop CONFIGURATION_GUIDE.md's dead env vars (#15143, #15151) #16440 fix(infra): map detect-environment.sh onto files that exist, and drop CONFIGURATION_GUIDE.md's dead env vars (bug(scripts): detect-environment.sh selects three .env files that do not exist #15143, bug(config): AUTOBOT_PLAYWRIGHT_HOST and AUTOBOT_PLAYWRIGHT_API_PORT are documented but read by nothing #15151)
chore(ci): merge train 2026-09-12-b, stacked test vehicle, do not merge (#15937) #16438 chore(ci): merge train 2026-09-12-b, stacked test vehicle, do not merge (Enable the merge queue — no PR merges against a stale base #15937)
fix(frontend): point artifact-cells/CodeCell.vue at the shared --codecell-syntax-* tokens (#14853) #16437 fix(frontend): point artifact-cells/CodeCell.vue at the shared --codecell-syntax-* tokens (style(frontend): the two CodeCells now tokenise their syntax palette differently #14853)
fix(knowledge): wire real grounding:stats counters, replacing the hardcoded claim_sources split (#14981) #16434 fix(knowledge): wire real grounding:stats counters, replacing the hardcoded claim_sources split (bug(knowledge): kb-stats reports a hardcoded claim_sources split — 65/22/13 on an empty KB with zero claims extracted #14981)
security(auth): fail closed when the password-epoch check cannot reach Redis (#16411) #16433 security(auth): fail closed when the password-epoch check cannot reach Redis (security(backend-auth): password-epoch revocation check fails open when Redis errors (backend equivalent of #16387) #16411)
chore(slm-frontend): retire getApiUrl(), route its display-only callers through getSlmApiBase() (#15761) #16432 chore(slm-frontend): retire getApiUrl(), route its display-only callers through getSlmApiBase() (tech-debt(slm-frontend): getApiUrl() survived the #13140 migration and still diverges from getSlmApiBase() in DEV #15761)
chore(frontend): retire useApi.ts, the deprecated useApiWithState composable family (#15025) #16424 chore(frontend): retire useApi.ts, the deprecated useApiWithState composable family (tech-debt(frontend): useApi.ts still exports the deprecated useApiWithState with zero importers — the #7600 migration stopped short of the shim #15025)
chore(ci): merge train 2026-09-12-a2, test vehicle, do not merge (#15937) #16423 chore(ci): merge train 2026-09-12-a2, test vehicle, do not merge (Enable the merge queue — no PR merges against a stale base #15937)
test(voice): judge first audio by chunks released, not a wall-clock race (#16407) #16420 test(voice): judge first audio by chunks released, not a wall-clock race (test(voice): voice_latency_test's streaming-vs-blocking TTFB race looks flaky under CI load (found triaging #16367) #16407)
fix(security): require sign-in on seven ungated routers, and admin where they change shared state or reach outside (#16375) #16418 fix(security): require sign-in on seven ungated routers, and admin where they change shared state or reach outside (security(api): 44 config-registered routers declare no auth gate, triaged by exposure (found by widening the router-auth guard, #16368) #16375)
feat(preflight): run every reproducible required check through the workflow's own command (#15933) #16416 feat(preflight): run every reproducible required check through the workflow's own command (Local preflight must cover the 10 required checks #15933)
fix(guards): fold shell continuations in the hooksPath guard via one shared helper (#15961) #16414 fix(guards): fold shell continuations in the hooksPath guard via one shared helper (core.hooksPath override silently disables every hook in a worktree #15961)
fix(security): deny an SLM token when its revocation check cannot run, HS256 and RS256 (#16387, #16412) #16413 fix(security): deny an SLM token when its revocation check cannot run, HS256 and RS256 (security(slm-auth): token revocation checks fail open when Redis errors, so a revoked token is accepted #16387, security(slm-auth): RS256 authority-token revocation check fails open when Redis errors #16412)
fix(security): generate the vault root key from 32 random bytes, not alphanumeric characters (#16405) #16410 fix(security): generate the vault root key from 32 random bytes, not alphanumeric characters (security(secrets): the vault root key is base64 of 32 alphanumeric chars (~190 bits), not 32 random bytes #16405)
chore(docker): retire the four agent Dockerfiles whose modules no longer exist; agents run in-process (#16361) #16409 chore(docker): retire the four agent Dockerfiles whose modules no longer exist; agents run in-process (chore(docker): four agent Dockerfiles under autobot-infrastructure/autobot-backend/docker/ are built by nothing: wire in or retire #16361)
chore(deps): carry dependabot's npm security bumps onto Dev_new_gui (hono 4.13.7 and transitive; supersedes #16171) #16408 chore(deps): carry dependabot's npm security bumps onto Dev_new_gui (hono 4.13.7 and transitive; supersedes chore(deps): bump the npm_and_yarn group across 6 directories with 7 updates #16171)
fix(tests): stop gating a disk-bound model load on a CPU work budget, and prove it stays offline and import-free (#15055, #15235) #16404 fix(tests): stop gating a disk-bound model load on a CPU work budget, and prove it stays offline and import-free (test(perf): startup benchmarks assert absolute wall-clock budgets and fail on runner load — passed and failed on the same code minutes apart #15055, test(perf): three calibrated budgets are drifting or disk-bound — the startup margin is 1.4x, not the 1.9x its note records #15235)
fix(ci): keep the drift-checked SLM agent mirror out of the duplication scan (#16401) #16402 fix(ci): keep the drift-checked SLM agent mirror out of the duplication scan (ci(duplication-guard): the SLM scope counts the drift-checked agent mirror, so every agent change fails the absolute pin #16401)
fix(ci): bring CI's pytest environment up to its declared floors, and gate on them (#16264) #16391 fix(ci): bring CI's pytest environment up to its declared floors, and gate on them (ci: the pytest environment sits below two declared floors, and the floor banner tells CI readers the opposite #16264)
fix(security): the dev proxy stops granting admin without a login, and the session check endpoint is tested (#16382, #16385) #16390 fix(security): the dev proxy stops granting admin without a login, and the session check endpoint is tested (SLM Vite dev-server proxy injects X-Internal-API-Key on /autobot-api with no session check #16382, SLM backend: add direct test coverage for GET /api/auth/me #16385)
fix(security): the SLM proxy checks the caller's SLM session before it adds the backend key (#16374) #16386 fix(security): the SLM proxy checks the caller's SLM session before it adds the backend key (security(slm): the SLM nginx /autobot-api/ proxy adds the internal API key for every caller, so anyone reaching the SLM host is a backend admin #16374)
fix(ci): retire the CodeQL suite file nothing loads, and record the real guard behind alert #1030 (#16283, #16302) #16377 fix(ci): retire the CodeQL suite file nothing loads, and record the real guard behind alert feat: Voice conversation mode — Hands-free with VAD (Tier 2) #1030 (ci(security): codeql.yml is disabled, so the reviewed codeql-config.yml exclusions apply to nothing #16283, ci(security): codeql-config.yml's file-scoped query-filters exclude nothing, under either setup #16302)
fix(security): write the service-key export only to its configured directory, never into a checkout, and keep one (#16348) #16372 fix(security): write the service-key export only to its configured directory, never into a checkout, and keep one (security(service-auth): generate_service_keys.py writes its live-key export relative to the working directory, so a run inside a checkout drops keys into the work tree #16348)
fix(security): authenticate every skills route, and require admin to change, fetch or execute (#16368) #16370 fix(security): authenticate every skills route, and require admin to change, fetch or execute (security(api): every route in api/skills.py (catalog fetch/install, enable, execute) is unauthenticated, and the router-auth guards can't see feature routers #16368)
fix(ci): let the system-package retry recover from its own timeout, and fetch before unpacking (#15515) #16367 fix(ci): let the system-package retry recover from its own timeout, and fetch before unpacking (ci(runner): apt provisioning times out (exit 124) three attempts in a row and fails the shard before any test runs #15515)
fix(guards): tell a baseline entry that matches nothing apart from one that under-matches (#16334) #16365 fix(guards): tell a baseline entry that matches nothing apart from one that under-matches (bug(guards): --audit-baseline says "no longer match anything" for an entry that still matches, so the reader deletes a live exemption #16334)
fix(ci): stop the regen bot's approved runs cancelling the job that approved them (#16360) #16363 fix(ci): stop the regen bot's approved runs cancelling the job that approved them (ci: the regen bot's own push cancels the job that approves its parked runs, so a PR's CI can silently never run #16360)
fix(ci): keep line numbers out of the secrets baseline, so a line move no longer rewrites it (#16353) #16362 fix(ci): keep line numbers out of the secrets baseline, so a line move no longer rewrites it (ci(secrets): .secrets.baseline stores a line number per finding, so every PR that shifts lines in a file with findings rewrites it and conflicts with every other such PR #16353)

If the queue is draining normally, ignore this. Otherwise:

Check whether CI is dispatching at all — see the ci-dispatch-watchdog status on these PRs
Merge the ones whose CI has finished and review has passed: gh pr merge <number> --squash --delete-branch
Look for a loop opening near-identical PRs

Warn-only runaway detector — .github/workflows/pr-queue-gate.yml. It never blocks a merge.

github-actions · 2026-09-12T08:35:31Z

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

…st changes (#16441) Review (37): four reds, three fixed here. - runner_exit_code_test.py's reachability regression still named add_role_permission_audit_log_timestamps as MIGRATIONS' last entry. #16025 appended widen_systemd_service_to_sequence after it; the test named the wrong migration as the one seed_agents must stay reachable before. - test_role_manifest_shape_16025.py's real-manifest test expected ["autobot-backend", "autobot-celery"] for the "backend" role. Per find_role_manifest's own docstring, direct ownership (role_name == the manifest's own role) wins every systemd service the manifest declares, slm_roles tags or not -- so adding autobot-celery-beat to autobot-backend/manifest.yml means "backend" now owns it too, same as it already owns autobot-celery despite that service's own narrower slm_roles: [celery] tag. Renamed the test off "both" now that there are three. - conftest.py stubs models.database wholesale, so role_registry.Role is a bare MagicMock; role_registry.seed_default_roles's new Role.__table__.columns filter auto-vivified through a MagicMock's own __contains__/__bool__, which never rejects a field. Gave the stub the real Role model's actual column names instead, so the filter is exercised against something that behaves like the real table would. Not touched here: the drift-mirror finding (role_detector_16025_test.py) -- couldn't locate the specific check it refers to despite looking at drift_checker.py's *_test.py exclusion pattern (generic, already covers new test files by suffix) and repo_tests/collection_coverage_test.py (no reference to this file at all). Flagging rather than guessing at a fix for a mechanism I haven't found.

…-set (#16441) Previous commit crashed exactly the tests it was meant to fix: AttributeError: __table__ autobot-slm-backend/conftest.py:177: in <module> sys.modules["models.database"].Role.__table__.columns = frozenset(...) Mock's __getattr__ refuses to auto-vivify a dunder-shaped name it does not implement as a real magic method -- it raises AttributeError rather than fake one, and __table__ matches that shape. Reading Role.__table__ (to then set .columns on whatever came back) never got past that raise. Fixed by assigning __table__ itself, a plain attribute set that does not go through __getattr__ at all, to a types.SimpleNamespace carrying the real columns.

mrveiss · 2026-09-12T23:14:04Z

37's review addressed, pushed at d490c68 (two rounds — the pre-push hook caught a bug in my own first fix):

runner_exit_code_test.py:118: updated to expect widen_systemd_service_to_sequence as MIGRATIONS's last entry (was still add_role_permission_audit_log_timestamps).
manifest.yml:31 / test_role_manifest_shape_16025.py:233: the test was wrong per the PR's own documented intent — find_role_manifest's docstring says direct ownership wins every systemd service the manifest declares, slm_roles tags or not. Updated the real-manifest test to expect autobot-celery-beat alongside the other two, renamed off "both" now that there are three.
Role stand-in has no __table__: confirmed and fixed in conftest.py — Role.__table__.columns is now a real frozenset of the actual model's column names. First attempt (read-then-set) crashed with AttributeError: __table__ — Mock's __getattr__ refuses to auto-vivify a dunder-shaped name it doesn't implement. Fixed by assigning __table__ directly.
Drift mirror missing role_detector_16025_test.py: not fixed — I couldn't locate the specific check this refers to. drift_checker.py's _SOURCE_ONLY_PATTERNS["autobot-slm-agent"] = ("*_test.py",) is a generic suffix pattern that already covers any new *_test.py file without per-file registration, and repo_tests/collection_coverage_test.py has no reference to this file. Could you point me to the exact check/file? Happy to fix it once I know where it lives.

…st changes (#16441) Review (37): four reds, three fixed here. - runner_exit_code_test.py's reachability regression still named add_role_permission_audit_log_timestamps as MIGRATIONS' last entry. #16025 appended widen_systemd_service_to_sequence after it; the test named the wrong migration as the one seed_agents must stay reachable before. - test_role_manifest_shape_16025.py's real-manifest test expected ["autobot-backend", "autobot-celery"] for the "backend" role. Per find_role_manifest's own docstring, direct ownership (role_name == the manifest's own role) wins every systemd service the manifest declares, slm_roles tags or not -- so adding autobot-celery-beat to autobot-backend/manifest.yml means "backend" now owns it too, same as it already owns autobot-celery despite that service's own narrower slm_roles: [celery] tag. Renamed the test off "both" now that there are three. - conftest.py stubs models.database wholesale, so role_registry.Role is a bare MagicMock; role_registry.seed_default_roles's new Role.__table__.columns filter auto-vivified through a MagicMock's own __contains__/__bool__, which never rejects a field. Gave the stub the real Role model's actual column names instead, so the filter is exercised against something that behaves like the real table would. Not touched here: the drift-mirror finding (role_detector_16025_test.py) -- couldn't locate the specific check it refers to despite looking at drift_checker.py's *_test.py exclusion pattern (generic, already covers new test files by suffix) and repo_tests/collection_coverage_test.py (no reference to this file at all). Flagging rather than guessing at a fix for a mechanism I haven't found.

…-set (#16441) Previous commit crashed exactly the tests it was meant to fix: AttributeError: __table__ autobot-slm-backend/conftest.py:177: in <module> sys.modules["models.database"].Role.__table__.columns = frozenset(...) Mock's __getattr__ refuses to auto-vivify a dunder-shaped name it does not implement as a real magic method -- it raises AttributeError rather than fake one, and __table__ matches that shape. Reading Role.__table__ (to then set .columns on whatever came back) never got past that raise. Fixed by assigning __table__ itself, a plain attribute set that does not go through __getattr__ at all, to a types.SimpleNamespace carrying the real columns.

…d copy (#16441) 37's 4th finding: pipeline-scripts/detect-agent-code-drift.sh requires autobot-slm-backend/slm/agent/ (canonical) and autobot-slm-backend/ansible/roles/slm_agent/files/slm/agent/ (mirror) to stay byte-identical, *_test.py files included since #14584 removed the blanket test exclude -- the mirror copies exist so CI can run the test suite against the exact bytes the role ships, not just diff them. role_detector_16025_test.py existed only on the canonical side. Added a byte-identical copy at the mirror path (verified with cmp); role_detector.py itself was already identical on both sides.

mrveiss · 2026-09-13T01:55:27Z

4th finding fixed and pushed at d9103dd: added a byte-identical copy at autobot-slm-backend/ansible/roles/slm_agent/files/slm/agent/role_detector_16025_test.py (verified with cmp). All 4 of 37's findings are now addressed on this PR.

mrveiss · 2026-09-13T01:56:52Z

Delta on the four CI reds from the red-PR sweep, at d9103dd6d: approve. All four are fixed at the root.

Red	Fix at this head
The drift mirror was missing `role_detector_16025_test.py`	`d9103dd6d` adds only `autobot-slm-backend/ansible/roles/slm_agent/files/slm/agent/role_detector_16025_test.py`. Its git blob (`24aebdfd6e9b`) is identical to the source's.
`test_seed_default_roles_*` raised `AttributeError: __table__`	`e1411ea2c`: the SLM `conftest.py` stub now sets `Role.__table__` directly, to a `SimpleNamespace`, instead of reading an attribute the stub lacked. That is what `services/role_registry.py:610`'s `Role.__table__.columns` reads.
`runner_exit_code_test.py:118` expected the old last migration	`8bad7ccab`: it now expects `widen_systemd_service_to_sequence`, both last and after `seed_agents`.
The manifest test did not expect `autobot-celery-beat`	`8bad7ccab`: `test_the_real_backend_manifest_yields_all_its_systemd_units` expects every backend unit, celery-beat included.

Scope: this covers these four fixes only; the PR's main content is 37's review. I did not check that the stub's column set matches the real Role model's. If it lists a column the model lacks, the seeding test could pass while the real code filters differently, so that is worth a one-line comparison. The branch is 0 behind main, and CI has 24 checks pending with none red.

mrveiss · 2026-09-13T01:57:31Z

Verified 37's open point: diffed the stub's 17 column names in conftest.py against models/database.py's real Role class's 17 Column(...) assignments — identical sets, no mismatch. The seed test and the real filter would behave the same way.

mrveiss · 2026-09-13T01:58:02Z

Follow-up on my open point: resolved. I checked it independently at d9103dd6d, parsing both files rather than running anything. The real Role model in models/database.py has 17 Column(...) fields, and the Role.__table__ stub in conftest.py lists 17 names. The two sets are identical, with none on either side alone. The seeding test's column filter therefore matches the real model.

…m the role manifests (#16025) Phase 1 of #16025 (a full single-PR fix is not honest -- see the recorded gate decision): role_registry.DEFAULT_ROLES now derives its identifier (canonical_name), systemd units (a sequence, AC4), health check and ports from autobot-infrastructure/*/manifest.yml via the new services/role_manifest_shape.py, instead of hand-declaring them a second time. display_name, sync_type, auto_restart, source_paths, target_path, post_sync_cmd, required, degraded_without and ansible_playbook stay registry-only, pinned in REGISTRY_ONLY_FIELDS (a shrink-only ratchet) -- none has a manifest field today, and two roles' manifests were found to disagree with a registry value role_registry.py's own comments prove correct, so deriving them blind would have shipped a regression under the banner of removing one. Resolved two divergences from on-base SSOT, not by picking a side blind: - browser-service health 3000/status -> 9001/health (ansible/roles/browser's own defaults declare 9001, list 3000 as legacy, #4662) - chromadb health 8000/v1 -> 8100/v2 (autobot_shared's ssot_config, #3094, and the ai-stack ansible role's own defaults) systemd_service becomes a sequence end to end: models/database.py's Role.systemd_service column (JSON, migrations/widen_systemd_service_to_sequence.py converts an existing deployed column), services/role_units.py, services/sync_orchestrator.py, api/roles.py, api/nodes.py, and slm/agent/role_detector.py (+ its ansible-deployed mirror copy) all updated to restart/detect every unit a role owns, not just one. Also fixed: autobot-ai-stack/manifest.yml's deploy.source pointed at a placeholder README dir (#14275) -- corrected to match what role_registry.py and the ansible role already deploy from. Not done here, and reported rather than guessed: the autobot-slm-agent manifest vs. registry source/destination divergence (tracked in the phase 2 issue), manifests for tts-worker/vnc/docker (#16436), and the SLM frontend's RoleResponse.systemd_service type (str -> list[str], noted in #16435). Refs #16025. Phase 2 tracked as #16435 and #16436, filed as its sub-issues.

api/roles.py's RoleResponse/RoleCreate/RoleUpdate.systemd_service became list[str] | None in the #16025 phase-1 commit; the SLM frontend still read it as a string. Every consumer under autobot-slm-frontend/src now handles the list: - useRoles.ts: Role.systemd_service -> string[] | null, plus shared joinSystemdUnits/parseSystemdUnits helpers so RolesView.vue and OrchestrationView.vue's comma-separated-text-field <-> array conversion (same pattern already used for source_paths/degraded_without) is not reimplemented, and diverging, in each view. - RolesView.vue, OrchestrationView.vue: local RoleDefinition/Role field type, edit-form population, submit payload, and every display site (table cells, role cards, migration summary) now join the units instead of interpolating an array. useOrchestration.ts's ServiceDefinition.systemd_service and the generated api.ts are untouched -- they back api/orchestration.py's unrelated service_orchestrator.py catalog (always one service per definition), not role_registry. api.ts is bot-regenerated on the PR and checked by verify-generated-types, so it is not hand-edited here. Added: joinSystemdUnits/parseSystemdUnits unit tests in useRoles.test.ts (join, single-unit parity, null/empty, split with ragged whitespace, blank-input null, round-trip) -- fixtures built at run time, no snapshot. Updates #16435 to drop the frontend item (now fixed here) from its list of what phase 2 still owes. Refs #16025.

Auto-regenerated (py3.14) to match the backend and/or SLM backend schema so the required verify-generated-types gate(s) pass. Triggered by auto-fix-generated-types.yml.

…st changes (#16441) Review (37): four reds, three fixed here. - runner_exit_code_test.py's reachability regression still named add_role_permission_audit_log_timestamps as MIGRATIONS' last entry. #16025 appended widen_systemd_service_to_sequence after it; the test named the wrong migration as the one seed_agents must stay reachable before. - test_role_manifest_shape_16025.py's real-manifest test expected ["autobot-backend", "autobot-celery"] for the "backend" role. Per find_role_manifest's own docstring, direct ownership (role_name == the manifest's own role) wins every systemd service the manifest declares, slm_roles tags or not -- so adding autobot-celery-beat to autobot-backend/manifest.yml means "backend" now owns it too, same as it already owns autobot-celery despite that service's own narrower slm_roles: [celery] tag. Renamed the test off "both" now that there are three. - conftest.py stubs models.database wholesale, so role_registry.Role is a bare MagicMock; role_registry.seed_default_roles's new Role.__table__.columns filter auto-vivified through a MagicMock's own __contains__/__bool__, which never rejects a field. Gave the stub the real Role model's actual column names instead, so the filter is exercised against something that behaves like the real table would. Not touched here: the drift-mirror finding (role_detector_16025_test.py) -- couldn't locate the specific check it refers to despite looking at drift_checker.py's *_test.py exclusion pattern (generic, already covers new test files by suffix) and repo_tests/collection_coverage_test.py (no reference to this file at all). Flagging rather than guessing at a fix for a mechanism I haven't found.

…-set (#16441) Previous commit crashed exactly the tests it was meant to fix: AttributeError: __table__ autobot-slm-backend/conftest.py:177: in <module> sys.modules["models.database"].Role.__table__.columns = frozenset(...) Mock's __getattr__ refuses to auto-vivify a dunder-shaped name it does not implement as a real magic method -- it raises AttributeError rather than fake one, and __table__ matches that shape. Reading Role.__table__ (to then set .columns on whatever came back) never got past that raise. Fixed by assigning __table__ itself, a plain attribute set that does not go through __getattr__ at all, to a types.SimpleNamespace carrying the real columns.

…d copy (#16441) 37's 4th finding: pipeline-scripts/detect-agent-code-drift.sh requires autobot-slm-backend/slm/agent/ (canonical) and autobot-slm-backend/ansible/roles/slm_agent/files/slm/agent/ (mirror) to stay byte-identical, *_test.py files included since #14584 removed the blanket test exclude -- the mirror copies exist so CI can run the test suite against the exact bytes the role ships, not just diff them. role_detector_16025_test.py existed only on the canonical side. Added a byte-identical copy at the mirror path (verified with cmp); role_detector.py itself was already identical on both sides.

…#16441) This file removes the conftest's models.database stub and builds a fresh one before importing the real services.role_registry (see its own module docstring), so the earlier conftest.py fix for Role.__table__ never reaches seed_default_roles() here -- it reads this file's own separately-constructed Role mock instead. Second pass on this finding: give this file's own stub the same __table__ columns directly (not read-then-set, since Mock's __getattr__ rejects dunder-shaped names). Bumps both file-size ratchet baselines by the 28 net new lines (846 -> 874).

mrveiss · 2026-09-13T07:02:52Z

Delta d9103dd6d..7772bd52d: REQUEST CHANGES, on one point: the raised size ceiling. The stub fix itself is correct.

Verified:

Root cause and fix: tests/services/test_role_registry.py builds its own models.database.Role mock before importing services.role_registry, so the conftest.py stub never reached seed_default_roles() there. The file now assigns its own Role.__table__ = SimpleNamespace(columns=frozenset({...})). Its 17 names match the real Role model's 17 Column(...) fields exactly, with none on either side alone; I parsed both files at this head.
Scope: only test_role_registry.py and the two ratchet files differ from the approved net diff. The branch is 0 behind main.

Severity	Where	Finding	Fix
Blocking	`scripts/python_file_size_known_large.py`, `repo_tests/python_file_size_ratchet_baseline.py`	`test_role_registry.py` is raised from 846 to 874 in both files. `repo_tests/python_file_size_ratchet_test.py:17` states the rule: "no ceiling may be raised". The baseline file exists so that a raise cannot be made by editing one file alone. CI will not catch this. `test_recorded_ceilings_match_the_files_today` only requires both copies to equal `wc -l`, which 874 does, and `ratchet-base-guard.yml` audits the base without diffing against it (its line 68). So a coordinated two-file raise passes, which leaves it to review.	Drop both bumps and keep the file at 846 lines or fewer.
Medium	`test_role_registry.py` vs `autobot-slm-backend/conftest.py`	The same 17-name column set is now written out in two places. When the model gains a column, one copy can be updated and the other left stale, and the stale copy's seed test then filters differently from the real code.	Put the set in one small shared test helper, for example `ROLE_TABLE_STUB = SimpleNamespace(columns=frozenset({...}))`, imported by both `conftest.py` and this test. That also removes most of the 28 added lines. If the file still ends up above 846, trim elsewhere in it; do not raise the ceiling.

CI had 26 checks pending when I posted.

mrveiss · 2026-09-13T07:04:25Z

For reference on "CI will not catch this": the gap is tracked in #16393 (open), "nothing rejects a raised ratchet pin". I added this PR's 846→874 there as a concrete case. #14498's two-copy lock rejects a raise in one file alone, but a coordinated raise of both copies to the file's new size satisfies every current check. Until #16393 lands, keeping ceilings from rising is a review rule, which is why it blocks here.

…#16441) 37's review on 7772bd5: raising test_role_registry.py's file-size ceiling 846->874 to fit a second copy of the Role column set violates the ratchet's own rule (no ceiling may ever rise) and defeats the two-copy-baseline design that exists to catch exactly that. Root fix: conftest.py now registers the column set once, under sys.modules["_role_cols"], instead of only setting it as a local attribute. test_role_registry.py's own separately-built Role stub reads that same value back (via a MagicMock __table__ constructor kwarg, which -- like a direct attribute assignment -- bypasses Mock.__getattr__'s dunder-name rejection) instead of carrying its own literal copy. One source of truth, and the file lands back at exactly 846 lines -- the ceiling is restored in both baseline copies rather than raised.

mrveiss · 2026-09-13T07:21:19Z

Delta 7772bd52d..6726f3619: approve. Both points from my last review are fixed.

Ceiling: tests/services/test_role_registry.py is exactly 846 lines. scripts/python_file_size_known_large.py and repo_tests/python_file_size_ratchet_baseline.py are byte-identical to main's; nothing was raised.
One column set: autobot-slm-backend/conftest.py:183-205 builds a real types.ModuleType("_role_cols") whose COLUMNS frozenset lists exactly the real Role model's 17 Column(...) names. I extracted the literal with ast; there are none on either side alone. It is registered once as sys.modules["_role_cols"]. test_role_registry.py:88 reads that same set back (MagicMock(__table__=SimpleNamespace(columns=sys.modules["_role_cols"].COLUMNS))), so a new column cannot update one copy and leave the other stale.
The sys.modules leak guard: repo_tests/sys_modules_leak_guard.py, loaded from the root conftest.py and therefore active in the separate slm-backend invocation too, fires only when a stub installed under directory D is still live while pytest touches something outside D. The slm-backend shard collects only autobot-slm-backend, so this conftest's owner directory covers every node in that run. The key cannot escape, and the guard should not fire, although this conftest is not on sys_modules_leak_baseline.txt.

	Note
Nit	Using a synthetic entry in `sys.modules` as a shared constant is unusual. A small importable helper, such as `autobot-slm-backend/tests/_role_columns.py`, would read more conventionally. This does not block.

The branch is 0 behind main. CI had 24 checks pending when I posted.

mrveiss added this to the v0.9.0 milestone Sep 12, 2026

github-actions Bot mentioned this pull request Sep 12, 2026

security(secrets): admin-gate infrastructure hosts, fix vault-backed template names (#16426, #16427) #16442

Merged

mrveiss self-assigned this Sep 12, 2026

mrveiss added area: config-ssot Wave 0 · cluster R — Config, paths & SSOT area: deploy-lifecycle Wave 1 · cluster O — Deploy, provision & lifecycle backend bug Something isn't working infrastructure priority: high labels Sep 12, 2026

mrveiss linked an issue Sep 12, 2026 that may be closed by this pull request

SLM role shape has two sources of truth: role_registry.DEFAULT_ROLES vs autobot-infrastructure/*/manifest.yml #16025

Open

5 tasks

This was referenced Sep 12, 2026

feat(collaboration): persist activity/notification history, add invitation list+respond endpoints (#16460) #16471

Merged

chore(deps): bump the npm_and_yarn group across 4 directories with 2 updates #16474

Merged

mrveiss force-pushed the issue-16025-role-shape-ssot branch from e3bebb2 to d490c68 Compare September 12, 2026 23:13

mrveiss force-pushed the issue-16025-role-shape-ssot branch from d490c68 to d9103dd Compare September 13, 2026 01:55

mrveiss and others added 7 commits September 13, 2026 09:57

chore(types): regenerate generated API types from OpenAPI schema(s)

8bc9da4

Auto-regenerated (py3.14) to match the backend and/or SLM backend schema so the required verify-generated-types gate(s) pass. Triggered by auto-fix-generated-types.yml.

mrveiss force-pushed the issue-16025-role-shape-ssot branch from d9103dd to 7772bd5 Compare September 13, 2026 07:00

mrveiss mentioned this pull request Sep 13, 2026

ci(ratchets): nothing rejects a raised ratchet pin; only the file-size ratchet has a guard #16393

Open

4 tasks

This was referenced Sep 13, 2026

chore(train): batch 2c CI vehicle — 25 PRs onto batch-2b's tip [no-merge] #16608

Closed

chore(train): batch 2c CI vehicle v2 — 24 PRs onto post-2b main [no-merge] #16614

Closed

mrveiss merged commit 10afa05 into main Sep 13, 2026
83 checks passed

mrveiss deleted the issue-16025-role-shape-ssot branch September 13, 2026 13:42

mrveiss mentioned this pull request Sep 13, 2026

SLM role shape has two sources of truth: role_registry.DEFAULT_ROLES vs autobot-infrastructure/*/manifest.yml #16025

Open

5 tasks

Navigation Menu

Repository navigation

refactor(slm): derive the role list's names, units, health and ports from the role manifests (#16025) - #16441

refactor(slm): derive the role list's names, units, health and ports from the role manifests (#16025)#16441
mrveiss merged 8 commits into
mainfrom
issue-16025-role-shape-ssot

mrveiss commented Sep 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 12, 2026

Uh oh!

github-actions Bot commented Sep 12, 2026

Uh oh!

mrveiss commented Sep 12, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

mrveiss commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Thinking Path

What Changed

Verification

Model Used

Uh oh!

coderabbitai Bot commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 12, 2026

Notice: 29 open PRs — past the runaway threshold (25)

Uh oh!

github-actions Bot commented Sep 12, 2026

✅ SSOT Configuration Compliance: Passing

Uh oh!

mrveiss commented Sep 12, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

mrveiss commented Sep 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mrveiss commented Sep 12, 2026 •

edited

Loading

coderabbitai Bot commented Sep 12, 2026 •

edited

Loading