fix(deploy): code-sync updates the install instead of discarding it (#14275)

[mrveiss]

Thinking Path

#14273 fixed the ai-stack's constraint path on the ansible side. This is the code-sync side —
the path an operator actually reaches from the maintenance UI, and the one CLAUDE.md designates as
the only way updates may reach a host.

Chasing the same defect there turned up a worse one. Code-sync is an update procedure: downtime is
expected, discarding the installation is not. It was discarding it.

Problem

1. A sync of ai-stack deleted the live install.

role_registry.py  ai-stack:  source_paths = ["autobot-ai-stack/"]   → repo dir holds ONE file: README.md
                             target_path  = /opt/autobot/autobot-ai-stack
sync_orchestrator.py:129     rsync in delete mode, excluding only __pycache__ and *.pyc

ai_install_dir is that same directory. The ansible role builds the venv at <dir>/venv, creates
src/ module symlinks, and deploys ai_api_server.py, ai_container_main.py,
requirements-ai.txt there. The real sources live under
autobot-infrastructure/shared/docker/ai-stack/ — not where source_paths pointed.

So the sync copied one README over the live install in delete mode. Data was never at risk
(ChromaDB is /var/lib/autobot/chromadb; postgres has source_paths: [] and never syncs at all),
but the AI stack needed a full re-provision to come back.

2. A missing source path reported success. _rsync_source_path returned True, "skipped",
so a role pointing at a directory absent from the checkout reported a clean sync having copied
nothing — an update that silently did not happen.

3. Three components installed requirements raw. ai-stack, npu-worker and tts-worker ran
a bare pip install -r requirements.txt; both worker files carry a relative -c constraint
include. Only backend delegated to scripts/build-filtered-requirements.sh — and its own comment
at :124 says why: "A bare pip install -r requirements.txt would error on the unresolvable
include".

What Changed

• Delete mode is gone from this path. Its sources do not carry the full tree, so the cost of a
  stale leftover file is far below the cost of deleting a live installation. The ansible syncs keep
  it, because theirs do carry the full tree (bug(deploy): resync refusal is all-or-nothing — four host-only paths are not in _PROTECTED_EXCLUDES, so the only way to finish a sync is to delete them #14231).
• Host state is excluded anyway, reusing HOST_STATE_EXCLUDES and the canonical artifact set
  from services/deploy_artifacts.py — the same vocabulary api/code_sync.py uses. This was the
  third implementation of the sync and the only one consulting neither.
• A missing source path fails instead of reporting success.
• All three post_sync_cmds route through the shared rewrite, and ai-stack's now names
  requirements-ai.txt — the file that is actually deployed — and installs into its own venv.
• source_paths for ai-stack points at the real sources.

Deliberately unchanged: the two other rsyncs in this file (_build_rsync_command,
_build_local_rsync_command) keep delete mode. They write the git checkout into the sync cache,
not onto an install, and already exclude .git, venv, node_modules and friends. I checked
them before assuming they shared the defect; they do not.

Verification

9 new tests; 446 passed across tests/services/. Mutation-checked:

mutation	result
restore delete mode	1 failed
return True, "skipped" again	1 failed
point ai-stack back at the placeholder dir	1 failed
revert npu-worker to a bare pip install	2 failed
drop the excludes from the argv	1 failed
(restored)	9 passed

That last row needed a second attempt, and it is the useful one: my first version asserted the
module text contained HOST_STATE_EXCLUDES, which stayed true after the excludes were deleted
from the argv, because the import line still named them. It now parses the rsync_cmd list and
asserts on what is actually built.

test_a_source_path_carries_more_than_a_readme exists because an existence check would not have
caught this: the placeholder directory was there.

Risks

Without delete mode, a file removed from the repo lingers on the host until the next provisioning
run. That is the deliberate trade — the alternative deleted installations.

Model Used

Opus 5 (1M context).

Closes #14275

[mrveiss]

Self-review before the reviewer landed found a regression I had introduced, and fixed it.

The host-state excludes are SOURCE on this path, not host state. I reused HOST_STATE_EXCLUDES (data, logs, config/, .env*) from deploy_artifacts.py because it is the canonical vocabulary api/code_sync.py protects. It is canonical for that layout. Here the same names are tracked directories a sync must deliver:

autobot-backend/      data/  config/
autobot-frontend/     config/  .env.example
autobot-slm-backend/  data/   .env.example

Applying them would have made the update silently incomplete — precisely the failure this PR exists to remove, reintroduced by the fix for it.

Now only the artifact excludes (venv, node_modules, __pycache__, dist, …) are applied. Safe to omit the rest because there is no delete flag: this sync only adds and overwrites, so host-only files are untouched whether excluded or not. The artifact set stays because it stops a repo directory clobbering the venv.

test_no_exclude_blocks_a_directory_that_is_source_for_some_role now asserts the rule, deciding "is this source?" with git ls-files rather than by what is on disk — __pycache__ exists on disk and is exactly what the artifact excludes are for.

And that test needed two attempts, for the same reason as the last one. Its first version read rsync_artifact_excludes() directly, so reintroducing a second exclude set into the argv did not change what it examined — mutation reintroduce-host-state-excludes passed. It now resolves the patterns from the rsync_cmd argv itself:

mutation	result
reintroduce the host-state excludes	1 failed
drop all excludes	2 failed
(restored)	10 passed

447 passed across tests/services/. Rebased onto the branch's auto-format commit.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[mrveiss]

Review found two more real defects, both confirmed against the branch. Fixed in 5ea9a2740.

1. Three roles installed into the wrong interpreter. I fixed ai-stack's to venv/bin/pip and
left the others bare — one of four, again.

role	unit's ExecStart	was	now
npu-worker	{{ npu_install_dir }}/venv/bin/python	bare pip	venv/bin/pip
tts-worker	{{ tts_install_dir }}/venv/bin/python	bare pip	venv/bin/pip
slm-backend	{{ slm_backend_dir }}/venv/bin/uvicorn	bare pip and bare alembic	both venv/bin/

A bare pip installs into whatever the SSH session's PATH resolves — system Python — so the
service restarts on new code against an unchanged dependency set, and slm-backend ran its
migration with a different interpreter than the service. Exactly the "did not come back running"
outcome, reported as success.

slm-agent is deliberately left alone: its unit runs /usr/bin/python3, so a bare pip is
correct there. The rule is match the unit, not always venv — and the test now derives it from
each role's .service.j2 rather than assuming.

2. A failed post-sync was discarded. _run_post_sync_command never inspected
proc.returncode; only an exception was logged. sync_node_role ignored its result, restarted the
service, and recorded the role as synced. So a pip install failing on permissions, a missing
venv, or an unresolvable constraint produced a green sync with no dependencies installed.

It now returns success, and a failure stops the sync before the restart and before the DB
record is written — restarting into a half-installed dependency set is worse than leaving the
running process alone with new files on disk.

Mutation-checked, 14 tests, 451 across tests/services/:

mutation	result
npu-worker back to bare pip	1 failed
slm-backend back to bare pip	1 failed
ignore the post-sync exit code	1 failed
continue after a failed post-sync	1 failed
(restored)	14 passed

On that third row — this is the fourth time in this PR I asserted on text rather than
behaviour. if proc.returncode != 0: → if False: leaves both "returncode" and "return False"
in the source, so the substring check passed. Executing the function is not available here: this
suite's conftest stubs services.*, so the class is a MagicMock and cannot be awaited. The test
now parses the function and requires an if whose test actually reads .returncode and whose
body returns a falsy first element — structure, which the mutation removes.

[mrveiss]

code-quality was red on pre-commit-hardcoded-values:

VIOLATION autobot-slm-backend/services/sync_orchestrator.py:218
1 violation(s) on lines this PR added.

Line 218 is await asyncio.wait_for(proc.communicate(), timeout=300). The literal predates this PR, but my edit touched that line to capture stdout, which reclassified it from "pre-existing" to "added" — the check is changed-lines-only, so it is right to flag it.

Hoisted to POST_SYNC_TIMEOUT_S = env_int("AUTOBOT_SYNC_POST_CMD_TIMEOUT_S", 300) and applied at both call sites (the other one at :444 was the same literal). Registered in env_registry.py and docs regenerated — and worth noting it needed registering at all only because #14266 taught the checker to see env_int; before today it would have passed unregistered.

The 4 remaining violations in that file are on lines this PR does not touch and the job explicitly does not fail on them (#13950 backlog).

451 passed in tests/services/, 20 in the registry-checker suite.