Skip to content

fix(ansible): stage the SLM frontend publish and repair three backend path defects (#15557, #15560) - #15602

Merged
mrveiss merged 6 commits into
Dev_new_guifrom
issue-15557-15560-ansible-deploy-paths
Sep 4, 2026
Merged

mrveiss merged 6 commits into
Dev_new_guifrom
issue-15557-15560-ansible-deploy-paths

Conversation

@mrveiss

@mrveiss mrveiss commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Closes #15557, #15560

Thinking Path

Both issues are the same defect, found in the same layer, at sites that sit
beside each other: an Ansible deploy path whose value or file location is
computed correctly in one place and restated — wrongly, or with no gate — in the
others. #15557 is four entry points that build the SLM frontend, one of which
received the staged-publish fix and three of which did not. #15560 is a symlink
target that does not depend on where the link lives, a systemd setting with two
writers, and a variable defined under a name no consumer reads. Every one of
them is "the same fact spelled out more than once, and the copies drifted".

That makes them one review, not two. The remedy is also shared: give each fact a
single writer, make every caller delegate to it, and put a guard over the
delegation with a floor under its own counts — because a fix applied to one of
four copies is exactly what produced both issues.

What Changed

#15557 — staged publish and a failure gate on every SLM-frontend entry point

New autobot-slm-backend/ansible/roles/_shared/tasks/build_publish_slm_frontend.yml
holds the build-then-swap once: build into dist.staging, fail the play on a
non-zero rc with the build's own stderr, refuse to publish a bundle whose
index.html is missing or zero-length, rotate the live bundle to
dist.previous, then promote. New
roles/_shared/tasks/record_slm_deployed_commit.yml holds the .deployed_commit
write once, and fails loudly when the commit cannot be identified rather than
finishing a deploy anonymously.

The seam is an Ansible task file rather than a call into
services/slm_frontend_build.py: that module runs inside the SLM backend's own
venv and resolves its paths from SLM_DEPLOYED_ROOT, while three of the four
callers are plays that run against an arbitrary target node before — or without
— that backend being able to serve a request. update-node.yml in particular
deploys to a node that has no SLM backend at all. A task file is the only seam
all four can reach. The Python path keeps its own copy of the same mechanism and
is recorded as such in the shared file's sweep note.

Entry point Was Now
playbooks/update-all-nodes.yml:499 the staged publish, spelled out inline delegates (7 tasks → 1 include)
roles/slm_manager/tasks/main.yml:806 npm run build:slm into the served dist/, no gate delegates
playbooks/update-node.yml:189 npm run build:slm into the served dist/, no gate delegates
playbooks/provision-fleet-roles.yml:523 plain npm run build into the served dist/, no gate delegates — also fixes the wrong build script, which bakes the wrong API base

.deployed_commit now lands on all four. update-all-nodes.yml:358 and
roles/slm_manager/tasks/main.yml:229 had a copy each (and update-all-nodes
needed a second task to repair the ownership its shell redirect got wrong);
update-node.yml:200 and provision-fleet-roles.yml:539 wrote no marker at all.
update-node.yml deploys from the manager's cache to a node with no checkout of
its own, so the commit is resolved on the manager (update-node.yml:98) and
handed to the play that writes it.

Sweep (#15557 AC 6), recorded in the shared file's header: the four Ansible entry
points above plus services/slm_frontend_build.py are every SLM-frontend build
site in the tree. The user-frontend build sites carry the same unstaged shape but
are a different component behind a different nginx site — filed separately rather
than folded in silently, along with two metadata files that describe an unstaged
build and one installer script that swallows a build failure outright.

#15560/1 — ai-stack symlink targets resolve relative to the link

roles/ai-stack/tasks/main.yml:259 built each src: as an absolute path under
the backend install directory, so the link re-anchored to the live tree under any
deploy-root relocation. Now ../../{{ backend_install_dir | basename }}/{{ item }}
— the basename comes from the variable rather than being spelled out, so an
install that renames the backend component directory still resolves.

#15560/2 — one writer for the backend unit's PYTHONPATH

playbooks/fix-backend-environment.yml rendered
roles/backend/templates/autobot-backend.service.j2 and then rewrote the unit's
PYTHONPATH and EnvironmentFile with lineinfile a few lines later. Both
hand-written lines are leftovers from before the template covered those settings
— that is the answer to the issue's first criterion. Removed; the template is now
the only writer. The play reads the value back out of the rendered unit
(fix-backend-environment.yml:112-131) and the import test exports that
(:162), so the two cannot disagree. The play fails loudly if the rendered unit
carries no PYTHONPATH rather than papering over it with a writer of its own.
The .env location checks now report against the location the template names,
which is what the deployed unit on a live node actually carries.

#15560/3 — backend_shared_dir wired in, not deleted

setup-user-backend.yml:46. The value was right and the name was wrong: roles
read backend_shared_standalone_dir, whose default is a hardcoded path that does
not follow the backend_deployed_root this play declares — so the play's deploy
root never reached the standalone autobot_shared copy. Renamed to the consumed
name. The omission was the bug.

Verification

Two new guards in repo_tests/ — CI's shard command passes an explicit path list
that autobot-slm-backend/ansible is not on, so a test beside the playbooks is
collected by a bare local pytest and by nothing that gates a merge.

repo_tests/slm_frontend_staged_publish_15557_test.py (12 tests) resolves each
entry point's include the way Ansible resolves it — a renamed shared file breaks
the check, which a substring match would not — and then sweeps every Ansible YAML
file for a task that still builds the SLM frontend itself, which is what catches
a fifth entry point arriving with a fresh copy. Floors: the entry-point set must
name 4 (a shrinking set is how a caller stops being checked), the file sweep must
reach 60 files, and the shared file must carry both fail gates, each with a
when:. test_the_inline_build_detector_discriminates re-runs the detector over
the pre-#15557 task shape so a clean sweep means something.

Mutation-checked: reverting update-node.yml to an inline npm run build:slm
fails 2 of the 12 — the delegation check and the inline-build sweep — with the
entry point named.

repo_tests/ansible_backend_path_defects_15560_test.py (27 tests) proves the
symlink property with real symlinks under a tmp_path root that is guaranteed
not to be the live tree, resolved with os.path.realpath, in the style of
autobot_shared_symlink_relative_13539_test.py, plus the contrast mutation
showing the historical absolute form escapes that root. It asserts the backend
unit has exactly one PYTHONPATH writer and that the play still derives a value
from the rendered unit — the negative half alone would pass on a play that
exports nothing. The dead-variable check is general: every variable
setup-user-backend.yml defines must be referenced somewhere in the Ansible
tree, so the next dead one fails too. Floors on the loop item count, the play's
variable count, and the file sweep.

repo_tests/update_deps_detection_window_test.py — the #15430 guard asserted the
staged publish against update-all-nodes.yml's own text. It now follows the
include to wherever the logic lives and asserts the property there, with a floor
on the shared file's task count.

Local: 45 tests across the three guard modules pass; 83 tests in the seven other
repo_tests modules that parse these playbooks and 63 in the six SLM-backend
test modules that do the same are unchanged and green. Full pre-commit over the
commit range passes with no findings. No Ansible was executed and nothing was
installed — CI is the authority on the suite.

Model Used

Claude Opus 5 (1M context)

@mrveiss

mrveiss commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Out-of-scope findings from this work, filed rather than folded in:

@mrveiss

mrveiss commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Pushed fc96f369 fixing two CI reds, both of them the same shape: an extraction that moved code out from under a guard that was watching the old file.

1. Six new hardcoded path literals (SSOT Configuration Compliance)

The new shared task files and the two playbooks carried default('<literal>') fallbacks copied from the sites they replaced. Copying a baselined literal into a new file is still a new violation — the baseline records where the value already is, it does not license another copy.

Fixed by deriving each fallback from the SSOT (autobot.base_dir, published in inventory/group_vars/all.yml) instead of restating the path:

file count
roles/_shared/tasks/record_slm_deployed_commit.yml 3
playbooks/update-node.yml 2
playbooks/provision-fleet-roles.yml 1

An override of code_source_dir / slm_backend_dir is still honoured; only the default changed. One deliberate behaviour change: if autobot were ever undefined the template now errors instead of silently falling back to a path that may not exist on that host. That is the better failure — a wrong provisioning path surfaces as a missing constraints file mid-pip-run, which is the shape #14272 records.

The pre-existing git_repo_root literal at provision-fleet-roles.yml:39 is untouched and stays baselined — it was not part of this change and is tracked in #15601 along with the other two.

2. The extraction hid a guarded git command from its checker

check_git_safe_directory_test.py::test_the_repository_guarded_sites_stay_visible pins three files that must each show the checker a real, guarded git -C <code_source>. Moving the marker write into record_slm_deployed_commit.yml emptied update-all-nodes.yml of its command, so the pin failed with "the code_source_dir command is no longer visible to the checker".

The pin now names the shared include, where the command actually lives. The distinction matters: the pin exists so that some file still demonstrates a guarded invocation to the structural walk — keeping it on update-all-nodes.yml would have asserted that this extraction never happens, which is not a property anyone wants to hold.

This is the second guard in this PR that had to follow the behaviour rather than the text; update_deps_detection_window_test.py was the first and was already handled. Worth noting that a text-anchored guard is invisible to an extraction until CI says otherwise — neither of these announced itself at authoring time.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

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

@mrveiss

mrveiss commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Review pass — approved. No blocking findings; one informational, filed as #15610.

Checked the five things most likely to be wrong in an extraction that consolidates four call sites.

Property Result
Bundle validation is real Checks index.html exists and stat.size != 0 before promoting — an empty or partial bundle cannot pass
All four entry points delegate, none left inline update-all-nodes.yml, slm_manager/tasks/main.yml, update-node.yml, provision-fleet-roles.yml all include_tasks the shared file and pass all three required vars
The wrong npm script is gone provision-fleet-roles.yml no longer runs npm run build; the shared file always uses build:slm, so the wrong API base cannot be baked in
The ai-stack symlink math ai_install_dir and backend_install_dir are true siblings, so ../../ lands exactly on backend_install_dir/item
The two removed lineinfile writers Safe. The unit template is confirmed the sole EnvironmentFile=/PYTHONPATH= writer, its single-line form matches the read-back regex, and every downstream consumer was moved to backend_unit_pythonpath in the same change — no stale reference left
backend_shared_dir → backend_shared_standalone_dir The renamed variable is the one both consuming roles actually declare, so the play's override now reaches them
The dropped safe-directory pin slm_manager/tasks/main.yml genuinely carries no other git -c safe.directory= invocation after the extraction, so removing its pin hides nothing
Guard quality Includes are resolved the way Ansible resolves them rather than by substring, floors bind to real counts, and both detectors carry a contrast case proving they discriminate. A fifth entry point with a fresh inline build would be caught

The one finding — #15610

The promotion is two sequential renames (dist → dist.previous, then dist.staging → dist), so dist/ does not exist between them and a request landing in that window gets a 404 rather than either bundle.

Not a regression from this PR — it is the pre-existing code lifted verbatim, and this PR is a behaviour-preserving extraction. But the exposure changed: the window existed at one entry point and now exists at four, precisely because making all four consistent is what this PR did. That is the cost of having got the consistency, and it should be recorded rather than left in a review comment.

It is also much smaller than the defect that was fixed: #15557 was about a failed build leaving a broken site serving indefinitely, and the staging directory plus the non-empty-index.html gate genuinely close that. #15610 is milliseconds on a successful publish, and the fix is a symlink flip — one atomic rename(2) — which is now a single edit rather than four.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant