feat(slm): node-major post-sync plan with a durable per-action record, sudo -n restarts and a node sync endpoint (#17455)

[mrveiss]

Thinking Path

The owner decided #17455 on 2026-10-10 (all four yes): node-major post-sync order across roles, with slm-backend last and chromadb before ai-stack, and the rule living once in _post_sync_plan; ollama restarted once per node; the orchestrator restart using sudo -n with failures recorded; and a new post_sync_action_runs table. The survey showed that nothing synced several roles on one node in one run. SyncOrchestrator.sync_node_role handled one (node, role) pair, its only caller was POST /roles/{role}/sync, and update-all's fleet stage uses Ansible, not the orchestrator. So the coordinator chose a node-level entry point (option A), which keeps the owner's ordering from being inert in the automated path. It builds on #18186's single SSH restart executor (merged as #18352, together with the #18359 command-injection fix) and is rebased onto that main.

What Changed

• One plan (api/_post_sync_plan.py). ordered_post_sync_plan keeps install → schema → build → restart and adds, by unit name (_RESTART_PRECEDENCE, _RESTART_LAST_UNITS, both citing feat(sync): post-sync actions need a durable record and a node-level plan in the orchestrator #17455):

  • autobot-chromadb restarts before autobot-ai-stack;
  • autobot-slm-backend goes last, even when mixed into another restart group or named by a non-restart action;
  • ollama, shared by the cpu and gpu LLM roles, restarts once through the existing unit dedupe.
  • The unit names come from code_sync._COMPONENT_SERVICES and _SELF_SERVICE_NAME.

• POST /api/nodes/{node_id}/sync (new, api/node_post_sync.py).

  • It syncs every ACTIVE role's files, then runs ONE node plan through services/post_sync_plan_runner.execute_plan.
  • A failed install, schema or build step aborts the rest. No restart runs and no role is marked synced, as sync_node_role did on a post-sync failure.
  • A role whose file sync fails is left out of the plan, and the node reports success=false.

• POST /api/code-sync/roles/{role_name}/sync now goes through the same plan path. It uses one run_id per call, shared across nodes, and its response keeps every field and adds run_id and actions.

  • The old per-role execution path is removed: sync_node_role, _run_post_sync_command, _restart_systemd_service and _sync_nodes_for_role. sync_role_files and record_role_synced remain.

• GET /api/code-sync/post-sync-runs/{run_id} (new) returns the run's rows in order, or 404.

• Self-restart safety. On the SLM's own node (code_sync._get_local_slm_node_id):

  • role versions are recorded and an "issued" row (success = null) is committed first;
  • the response is returned, carrying run_id and every row;
  • only then is the restart sent, via fire_and_forget after service_restart.wait_for_response_flush(), the same flush-then-restart mechanism the Ansible self-update uses (Bug: 3 SLM frontend/backend code bugs from console log analysis #816);
  • role sync processes the SLM host last;
  • if the deferred restart returns (refused or timed out), a follow-up row records the real outcome.

• Decision 3 (services/post_sync_runner.py). The restart runs through tech-debt(slm): restarting units over SSH is implemented three times with different sudo, timeout and failure handling #18186's run_systemctl_over_ssh with use_noninteractive_sudo=True and a BatchMode=yes ssh base (build_service_ssh_base_cmd). The returncode is checked. A non-zero exit, a timeout or a spawn error becomes a recorded failure instead of logging "Restarted". Timeouts are env-backed (AUTOBOT_SYNC_RESTART_TIMEOUT_S, default 60, registered in env_registry_slm.py and in ENV_VARS.md).

• Decision 4. models/post_sync_action_run.py adds PostSyncActionRun (post_sync_action_runs): run_id, node_id, role_name, category, label, command, units, started_at, finished_at, exit_code, success (nullable, for "issued"), output_tail and error.

  • command, output_tail and error go through autobot_shared.secret_redaction.redact_text (REDACTION_BOUNDARY.md:125), and the tail is capped at 800 characters.
  • The migration migrations/add_post_sync_action_runs_table.py is idempotent (table_exists) and additive only: nothing is dropped or altered, so no data can be lost. It is appended last in MIGRATIONS.
  • services/database.py registers the model for create_all.
  • Recording never fails a sync: an error is caught and logged.

• Rebase onto fix(slm): one SSH systemctl executor with quoted, validated units and gated service routes (#18359) #18352 (tech-debt(slm): restarting units over SSH is implemented three times with different sudo, timeout and failure handling #18186 + security(slm): remote command injection — service_name from the URL path is interpolated unquoted into an ssh 'sudo -n systemctl' shell command (reachable by admin/operator users) #18359): where the two overlap, main's hardening wins.

  • Every unit feat(sync): post-sync actions need a durable record and a node-level plan in the orchestrator #17455 restarts goes through main's systemctl_remote_command (validate_unit_name + shlex.quote + --) via run_systemctl_over_ssh. feat(sync): post-sync actions need a durable record and a node-level plan in the orchestrator #17455's own earlier unquoted builder and shell_suffix are gone (grep finds neither in the feat(sync): post-sync actions need a durable record and a node-level plan in the orchestrator #17455 modules).
  • A role unit that fails validation becomes a recorded failure: nothing is spawned, the row says systemctl restart (rejected: invalid unit name), and it is never deferred as an "issued" self-restart.

• Test-isolation fix (found during the rebase): test_post_sync_recording_17455.py run in one process with test_colocated_managed_update_11605.py made 11605 fail at collection (a MagicMock response model). The fix that matters is in 11605: its dev-host stub now also applies when a real models package carries the conftest's MagicMock schemas. A contrast with only that change, and the helper reverted, passes. Additionally, import_modules_with_real_orm now drops modules first imported during its swap. That is defence in depth, not the cause.

Verification

• After the rebase: plan order + node route 39; recording 12; migrations/*_test.py 66; test_ssh_restart_*_18186.py 60; injection 18359 64; code-sync-preserves-install 15; root guards (remote-command quoting, one-executor, reach ×2, size ratchet) 633. A single batched run of all 9 import_modules_with_real_orm users + 11605 + the 17455 route and plan tests: 218 passed.
• autobot-slm-backend targeted set before the rebase: 144 passed. That covers the plan order, the node route, recording, the 18186 equivalence file, the 14275 structural test and migrations/. The implementer's wider run gave 170 passed, and repo guards (router auth, mount, duplicate-route, size ratchet, one-executor) gave 130 passed.
• Intended test deltas:
  • test_orchestrator_argv_has_no_dash_n_one_call_all_units_and_literal_60 → …has_sudo_n_batchmode_one_call_all_units_env_backed_60_17455. tech-debt(slm): restarting units over SSH is implemented three times with different sudo, timeout and failure handling #18186 pinned the old sudo without -n, pending this decision. Decision 3 changes it on purpose.
  • test_orchestrator_swallows_a_nonzero_exit_and_logs_restarted → …records_a_nonzero_exit_as_a_failure_not_restarted_17455. The swallowed exit was the defect decision 3 removes.
  • The two test_code_sync_preserves_install_14275.py structural pins follow the code into post_sync_runner.py. The AST guard requires code != 0 (Compare against 0), and the plan-runner abort is pinned.
• Opus gate: CHANGES REQUIRED. All findings are fixed:
  • HIGH: versions were recorded after the self-restart, losing the version and the response;
  • MEDIUM: the self-restart outcome was discarded;
  • LOW: a weaker AST pin, a 34-line migrate, stale docstrings.
  • Auth, migration, ordering, sudo -n, redaction, removed code and ratchets were confirmed correct.
• Auth: all three routes take Depends(get_current_user) and mount on nodes_router / code_sync_router, which main.py includes with dependencies=_SM_OR_AGENT. A test asserts the dependency and the mounts.
• Ratchets: services/sync_orchestrator.py goes 649 → 561 (under 600, entry deleted), and api/code_sync.py goes 5860 → 5829, in both baselines. No ceiling is raised, and no new noqa is added.

Risks

• Deploy-visible. The orchestrator restart now needs passwordless sudo -n for systemctl. A host where only interactive sudo works will record a failed restart instead of restarting silently. The other two executor callers already use -n.
• The migration adds a table only.
• Generated types: the new routes make autobot-slm-frontend/src/types/generated/api.ts stale. The generator's exact diff is taken from the verify-generated-types-slm job log and committed as a follow-up push, per the reviewer, never hand-written.
• Local tests/api has 4 environmental failures, identical on the base tree, filed as test(slm): four tests/api tests fail outside CI — pricing reads the deployed .env, admin-socket fails even alone #18234.

Model Used

Anthropic Claude Opus 5.5 (claude-opus-5-5), coordinating and gate review. Anthropic Claude Sonnet 5.5 (claude-sonnet-5-5) subagents for the survey and implementation.

Issue Link

Refs #17455

Not closed by this PR: the AC "the record is reachable from wherever the sync run is shown" is met at the API (the sync responses and GET /post-sync-runs/{run_id}). The SLM fleet-view UI that shows it is the child #18235.

Single-issue rationale: this is one owner decision on #17455 that spans the plan, the orchestrator, routes and the schema. Its only remaining part, the SLM UI, is its own child issue, #18235.

Changelog fragment

• N/A (operator-facing API and record; the UI surface lands with feat(slm-ui): fleet-view node sync and the post-sync run record (#17455 follow-up) #18235)

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 025bb499-08fb-47e9-80f4-3013ab3c7a14

---

📥 Commits

Reviewing files that changed from the base of the PR and between df211d3 and 57cd4cd.

---

⛔ Files ignored due to path filters (2)

• repo_tests/python_file_size_ratchet_baseline.py is excluded by !repo_tests/python_file_size_ratchet_baseline.py
• scripts/python_file_size_known_large.py is excluded by !scripts/python_file_size_known_large.py

---

📒 Files selected for processing (27)

• autobot-slm-backend/api/__init__.py
• autobot-slm-backend/api/_post_sync_plan.py
• autobot-slm-backend/api/code_sync.py
• autobot-slm-backend/api/node_post_sync.py
• autobot-slm-backend/conftest.py
• autobot-slm-backend/migrations/add_post_sync_action_runs_table.py
• autobot-slm-backend/migrations/add_post_sync_action_runs_table_test.py
• autobot-slm-backend/migrations/runner.py
• autobot-slm-backend/migrations/runner_exit_code_test.py
• autobot-slm-backend/models/post_sync_action_run.py
• autobot-slm-backend/models/post_sync_schemas.py
• autobot-slm-backend/services/database.py
• autobot-slm-backend/services/post_sync_plan_runner.py
• autobot-slm-backend/services/post_sync_recorder.py
• autobot-slm-backend/services/post_sync_runner.py
• autobot-slm-backend/services/post_sync_timeouts.py
• autobot-slm-backend/services/service_restart.py
• autobot-slm-backend/services/sync_orchestrator.py
• autobot-slm-backend/tests/api/_real_orm_import.py
• autobot-slm-backend/tests/api/test_colocated_managed_update_11605.py
• autobot-slm-backend/tests/api/test_node_post_sync_17455.py
• autobot-slm-backend/tests/api/test_post_sync_plan_order.py
• autobot-slm-backend/tests/api/test_ssh_restart_equivalence_18186.py
• autobot-slm-backend/tests/services/test_code_sync_preserves_install_14275.py
• autobot-slm-backend/tests/services/test_post_sync_recording_17455.py
• autobot_shared/env_registry_slm.py
• docs/developer/ENV_VARS.md

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

---

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

[mrveiss]

Security review (Opus) at 57cd4cd: approve.

• Authz: the route lives in the SLM. It is mounted via nodes_router/code_sync_router with _SM_OR_AGENT (require_service_management_or_internal, main.py:655,670,697) plus get_current_user. The node id is checked with a parameterized select, and a missing node gives 404.
• Remote commands: every restart goes through run_systemctl_over_ssh → systemctl_remote_command (action allowlist, validated and quoted units, --). sudo -n is set only through use_noninteractive_sudo. No shell=True, create_subprocess_shell or f-string sudo/ssh lines are added. role.post_sync_cmd is still admin-set DB content sent raw, the same as main.
• Migration: the SLM's own runner, additive and idempotent, with no alembic collision with refactor(audit): audit_logs is the one system of record, Redis its projection — three stores retired, one write path, one retention table (#17225) #18343/feat(approvals): one approval record, shared RiskTier, pending inbox query (#13421 S1) #18404.
• Record: command, tail and error go through redact_text, with the tail capped at 800 chars. Recording failures never fail the sync.
• Invalid unit: nothing is spawned and a failure is recorded, tested at test_post_sync_recording_17455.py:133,149.
• Rebuild: all 29 files are in the feat(sync): post-sync actions need a durable record and a node-level plan in the orchestrator #17455 scope.

LOWs are filed as #18424: retention, the missing downgrade() despite the docstring, the concept declaration, and the narrower module cleanup. Queued for the next merge train.

[github-actions]

✅ 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.

[github-actions]

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

This is not a request to defer this PR. The actual limit is the open-PR cap (AUTOBOT_OPEN_PR_CAP, default 40), enforced at pre-push against NEW branches only -- pushing more commits to this PR's own branch is never blocked by it.

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:

• refactor(rag): de-fork graph retrieval — one neighbour contract, one graph blend, shared config (#18139, #18141) #18421 refactor(rag): de-fork graph retrieval — one neighbour contract, one graph blend, shared config (bug(rag): PPR is wired with the mesh adapter whose get_neighbors signature differs — mesh retrieval raises TypeError on first use #18139, tech-debt(rag): graph-blended retrieval paths fork the score blend, config, reranker weights and stale filter #18141)
• refactor(captcha): one CaptchaType in autobot_shared (#18131) #18418 refactor(captcha): one CaptchaType in autobot_shared (tech-debt(captcha): two captcha_solver modules whose CaptchaType enums disagree — a kind one can classify is UNKNOWN to the other #18131)
• fix(memory): host context calls get_related_entities, the canonical graph traversal (#18140) #18416 fix(memory): host context calls get_related_entities, the canonical graph traversal (bug(memory): security memory integration calls traverse_relations, which the memory graph does not have #18140)
• feat(guard): a kit component cannot be forked in either app, and kit stories render in both Storybooks (#14776) #18409 feat(guard): a kit component cannot be forked in either app, and kit stories render in both Storybooks (Retire the local Base* component forks in autobot-frontend into the shared kit #14776)
• feat(agents): read-only agent-ledger APIs for Company OS and the SLM (#18391) #18408 feat(agents): read-only agent-ledger APIs for Company OS and the SLM (feat(agents): a GUI for the agent ledger — who is working on what, per company and install-wide in the SLM #18391)
• fix(kb): add_document chunks, partial-success ingest, stable upload rejection codes (#17830) #18405 fix(kb): add_document chunks, partial-success ingest, stable upload rejection codes (bug(kb): add_document never chunks — a whole transcript or PDF becomes one embedding, against its own docstring #17830)
• feat(approvals): one approval record, shared RiskTier, pending inbox query (#13421 S1) #18404 feat(approvals): one approval record, shared RiskTier, pending inbox query (ui(approvals): five approval surfaces with no shared record or component — build one approval inbox #13421 S1)
• feat(llm): needs_reauth registry skip + typed error, header headroom harvest, pre-dispatch headroom routing (#15022, #15026, #15028) #18403 feat(llm): needs_reauth registry skip + typed error, header headroom harvest, pre-dispatch headroom routing (llm: degradation store cannot distinguish a dead credential from a transient failure — dead tokens retry every TTL forever #15022, llm: harvest provider rate-limit response headers into a cross-worker quota headroom store (the missing quota monitor) #15026, llm: route on observed headroom before dispatch instead of on a 429 after it; reconcile guessed RPM buckets with observed limits #15028)
• fix(slm-auth): SSO callbacks redirect with a one-time code, not the session JWT (#18394) #18399 fix(slm-auth): SSO callbacks redirect with a one-time code, not the session JWT (security(sso): SSO callbacks redirect with the session JWT in the query string — now live after #18322 #18394)
• refactor(api): the main app's raw fetch tail goes through fetchWithAuth, and ApiClient gains an opt-in circuit breaker for polled dashboards (#12363) #18398 refactor(api): the main app's raw fetch tail goes through fetchWithAuth, and ApiClient gains an opt-in circuit breaker for polled dashboards (canonical(frontend): no single backend contract — 8 independent HTTP clients (2 generic) + 22 raw fetch sites, and generated api.ts is bypassed (4 importers vs 74 hand-typed responses) and stale (2424 vs 2152 paths) #12363)
• fix(guards): mypy resolves autobot_shared so enum aliases stay types; the embedding-literal guard covers namespaced ids and the infra builders (#18345) #18393 fix(guards): mypy resolves autobot_shared so enum aliases stay types; the embedding-literal guard covers namespaced ids and the infra builders (types: an enum alias of the canonical Severity used as an annotation is 'not valid as a type' in the backend mypy run (6 instances) #18345)
• test(tools): differential test of the tool-approval planes (#13416) #18387 test(tools): differential test of the tool-approval planes (test(security): the two tool-approval planes are never compared — add a differential assertion #13416)
• refactor(infra): classify 43 more infra scripts by runtime, repair 3 validators (#18289) #18380 refactor(infra): classify 43 more infra scripts by runtime, repair 3 validators (infra: 98 more infrastructure modules import backend-only packages — classify each as shared, backend tool or dead (#18288 follow-up) #18289)
• feat(shell): Refined UI step 2: top nav and icon cluster, Minimal/Full mode, Appearance settings, one brand mark (#18240) #18373 feat(shell): Refined UI step 2: top nav and icon cluster, Minimal/Full mode, Appearance settings, one brand mark (Refined UI step 2 · shell: top nav, UI mode, profile appearance #18240)
• security(api): RUM split into admin, user and bounded-open routes, and Prometheus metrics behind an SLM-issued scrape token (#16375) #18372 security(api): RUM split into admin, user and bounded-open routes, and Prometheus metrics behind an SLM-issued scrape token (security(api): 44 config-registered routers declare no auth gate, triaged by exposure (found by widening the router-auth guard, #16368) #16375)
• refactor(config): the #18094 de-fork remainder — one key reader, one feature flag, one language detector, and image-gen keys vault-first through an origin-bound resolver (#18094) #18371 refactor(config): the tech-debt: forked env defaults, shadowed constants and same-name functions — the remainder after #17419/#16177/#18093 #18094 de-fork remainder — one key reader, one feature flag, one language detector, and image-gen keys vault-first through an origin-bound resolver (tech-debt: forked env defaults, shadowed constants and same-name functions — the remainder after #17419/#16177/#18093 #18094)
• fix(codebase-analytics): honest scan outcomes, duplication detectors wired, and forks shown from the one CI detector (#18013, #18067, #17982) #18362 fix(codebase-analytics): honest scan outcomes, duplication detectors wired, and forks shown from the one CI detector (bug(codebase-analytics): /duplicates returns a fabricated "timed out after 120s" in 0.2s — the single-flight lock is never released #18013, Codebase analytics reports 18,200 problems and zero duplication, while the fork detector finds 365 clusters in the same tree #18067, bug(codebase-analytics): duplicate detection reports 'nothing scanned' as a completed scan — the one tool the dedup programme needs returns nothing for AutoBot's own codebase #17982)
• refactor(audit): audit_logs is the one system of record, Redis its projection — three stores retired, one write path, one retention table (#17225) #18343 refactor(audit): audit_logs is the one system of record, Redis its projection — three stores retired, one write path, one retention table (tech-debt(audit): #6475 closed on three parallel audit systems — there are now six #17225)
• security(auth): DB role permissions work, one strict-union permission gate for both services, MFA-pending tokens refused, SSO mints from the real user (#18259, #18321, #18322) #18338 security(auth): DB role permissions work, one strict-union permission gate for both services, MFA-pending tokens refused, SSO mints from the real user (security(rbac): UserService.get_user_permissions reads role.Permission, which does not exist — the DB-backed permission path raises and the middleware denies #18259, security(slm): get_current_user accepts MFA-pending temp tokens as full sessions (#17049 fixed only the backend) #18321, fix(slm-sso): SSO login passes a stand-in user without is_platform_admin to create_token_response, which raises #18322)
• chore(deps): bump the all-dependencies group across 1 directory with 15 updates #18273 chore(deps): bump the all-dependencies group across 1 directory with 15 updates
• fix(sandbox): execution and pooled containers run fully hardened, and no caller can widen their reach (#17267) #18232 fix(sandbox): execution and pooled containers run fully hardened, and no caller can widen their reach (refactor(sandbox): container hardening is copy-pasted across three Docker clients, with a docstring as the only thing preventing drift #17267)
• chore(deps): bump the all-dependencies group in /autobot-infrastructure/autobot-npu-worker/docker with 3 updates #18177 chore(deps): bump the all-dependencies group in /autobot-infrastructure/autobot-npu-worker/docker with 3 updates
• chore(deps): bump the all-dependencies group in /autobot-infrastructure/shared/docker/ai-stack with 4 updates #18176 chore(deps): bump the all-dependencies group in /autobot-infrastructure/shared/docker/ai-stack with 4 updates
• chore(deps): bump actions/upload-artifact from 7.0.1 to 7.0.2 in the all-dependencies group #18175 chore(deps): bump actions/upload-artifact from 7.0.1 to 7.0.2 in the all-dependencies group
• chore(deps): update asyncpg requirement from >=0.31.0 to >=0.32.0 in /autobot-slm-backend in the all-dependencies group #18172 chore(deps): update asyncpg requirement from >=0.31.0 to >=0.32.0 in /autobot-slm-backend in the all-dependencies group
• chore(deps): bump the all-dependencies group across 1 directory with 16 updates #18157 chore(deps): bump the all-dependencies group across 1 directory with 16 updates
• chore(deps): update aiohttp requirement from >=3.14.3 to >=3.14.4 in /autobot-backend/code_analysis in the all-dependencies group across 1 directory #18156 chore(deps): update aiohttp requirement from >=3.14.3 to >=3.14.4 in /autobot-backend/code_analysis in the all-dependencies group across 1 directory
• chore(deps-dev): bump the oxlint group across 1 directory with 2 updates #18152 chore(deps-dev): bump the oxlint group across 1 directory with 2 updates
• chore(deps): update transformers requirement from >=5.18.0 to >=5.19.0 in /autobot-tts-worker in the all-dependencies group across 1 directory #18150 chore(deps): update transformers requirement from >=5.18.0 to >=5.19.0 in /autobot-tts-worker in the all-dependencies group across 1 directory

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

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

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