Repository navigation
chore(ansible): the native-services playbook states the deployed source root as a literal twenty times #15687
Description
Activity
- added 6 commits that reference this issue
on Sep 6, 2026 Closed by #15901, merged as
539873f75. Verified against merged base.AC Evidence check_ansible_file_references.pyresolves a templated deploy-root reference to its repo path, with a test that a wrong path behind the template is still caughtThe #13744 defect was restored behind the template and confirmed still caught — verified end to end rather than by inspection The playbook names the deployed source root once and every one of the 20 sites reads it Single definition, 20 sites templated The count of references the guard reports does not fall Held at 1 — and the criterion turned out to be weaker than it reads. See below ansible_manifest_resolution.pystill derivesautobot-npu-worker/requirements.txtfor the npu-worker venvNPU = "autobot-npu-worker/requirements.txt"at:67, resolved through_existing_suffix; asserted rather than assumedThe third criterion could not have failed, and fixing that is what this issue actually bought
It reads as the coverage guarantee — "the only evidence that coverage survived". Review found two defects in it:
_resolve()returnsNonefor anything unresolvable and the caller doescontinue. A reference that stops resolving leaves the population rather than failing. The floor was>= 1against 218 host patterns: it detected the walk reading nothing, and nothing else.- The measured and templated populations were never the same set. The guard tracks
_FILE_KEYSassignments, of which exactly one is a deployed-src reference, while this change moved roughly twenty literals. An unchanged1is a statement about one reference offered as evidence about twenty. In the author's words: "not a weak number, a number about a different thing."
Both fixed.
_referenced_repo_pathsnow returns(resolved, unresolvable), the summary reports 1 resolves / 168 unresolvable where it previously hid 168 entirely, and the floor is asserted in the test file — a fact about this repository — rather than inmain(), which must work on an arbitrary tree.Two wrong turns on the way, both caught by the existing suite: failing on all 168 was over-fire (
{{ item }}in a loop,{{ cert_file }}at run time, network subnets in group_vars are legitimately unresolvable), and putting the floor inmain()broketest_a_play_resolving_nothing_is_still_a_pass. That test exists to protect a legitimate empty case, and it is what stopped a reach floor becoming a false positive.A criterion that names a single number as its only evidence is worth re-reading before it is ticked. This one was satisfiable by a number that could not move in the direction the failure would move it.
Raised in review of #15686 against one task, declined there as out of scope, and filed here as the whole finding.
What is repeated
autobot-slm-backend/ansible/playbooks/deploy-native-services.ymlstates the deployed source root as a literal 20 times, across five plays, in six different shapes:synchronize:dest:file:path:for the follow-up chownWorkingDirectory=cdpip:requirements:and its-croot,PYTHONPATH=, and one moredest:Each play writes the destination itself with
synchronizebefore reading from it, so there is no correctness bug here today: the path cannot be absent unless the sync failed, and a variable would not change that. This is the SSOT rule applied to a value that is genuinely one value, not a defect report.Why it was not done in #15686
Two reasons, both worth carrying into whoever picks this up.
It is not a local repetition. #15686 touched two of the twenty. Naming an NPU-only variable for those two would leave one play spelling one path two ways while the other four plays kept the literal — worse for a reader than the uniform literal it replaced. The fix is per playbook, not per task.
It costs guard coverage unless that guard is taught first.
scripts/check_ansible_file_references.pyresolves ansible values to repo paths only where the value is literally prefixed/opt/autobot/src/, under_FILE_KEYS = ("requirements", "src", "chdir", "creates"). That guard exists for #13744 — which was this playbook's NPUrequirements:line pointing atdocker/npu-worker/requirements.txt, a path that never existed in the repo, so the documented native NPU deploy installed nothing and still reported success. Templating those values removes them from the guard's population, and its only vacuity floor isplays_read >= 1, so the loss would be silent.So the guard change is a prerequisite, not a follow-up: it must resolve
{{ some_var }}/autobot-npu-worker/requirements.txtbefore the literals move, or #13744 becomes undetectable again.Acceptance criteria
check_ansible_file_references.pyresolves a templated deploy-root reference to its repo path, with a test that a wrong path behind the template is still caught — the fix(ansible): NPU play installs from docker/npu-worker/requirements.txt, which does not exist #13744 case, restated against the templated formrepo_tests/ansible_manifest_resolution.pystill derivesautobot-npu-worker/requirements.txtfor/opt/autobot/autobot-npu-worker/venv(it resolves the templated form through_existing_suffix, so this should hold — assert it rather than assume it)Related
requirements:/-coccurrences it touched