Repository navigation
guard(deploy): the provision-python playbook path is stated in ansible and Python, and nothing checks they agree #15707
Description
Found while closing out #15705. One path is stated in two languages and must stay byte-identical, and nothing checks that it does.
The ansible side — autobot-slm-backend/ansible/playbooks/configure-python-provision-permissions.yml:
provision_playbook: "/opt/autobot/code_source/autobot-slm-backend/ansible/playbooks/provision-local-python.yml"That value is written into a NOPASSWD sudoers rule.
The Python side — autobot-slm-backend/api/code_sync.py:
_ANSIBLE_DIR = Path(DEFAULT_REPO_PATH) / "autobot-slm-backend" / "ansible" # :1622
_PROVISION_PYTHON_PLAYBOOK = _ANSIBLE_DIR / "playbooks" / "provision-local-python.yml" # :1623
cmd = ["sudo", "ansible-playbook", str(_PROVISION_PYTHON_PLAYBOOK)] # :1896Sudoers matches that argv exactly. code_sync.py:1628 already records the constraint in a comment: "changing the command args would break the exact-match NOPASSWD sudoers rule", which is why the runner passes cwd= instead of adding arguments.
Why the failure mode is worse than a wrong path
The two values agree today. If either moves — DEFAULT_REPO_PATH changes, or the install root relocates and the playbook literal is updated to match — the other does not follow, and the symptom is sudo refusing the command on the provisioning path. Not a missing directory, not a clear "path not found": a permission failure whose cause is a string mismatch two files apart, in different languages, neither of which mentions the other.
#15705 deliberately left the ansible literal alone for exactly this reason: deriving it from autobot.base_dir would have made it a third independent source rather than closing the gap. The gap is the coupling itself, and it needs a check rather than a refactor.
Acceptance criteria
- A guard asserts the two spellings resolve to the same string — reading the literal out of the playbook and the constructed path out of
code_sync.py, so it fails when either side moves alone - The guard fails for the right reason: prove it by mutation, changing one side and confirming red
- Both sites carry a pointer to the other, so someone editing either is told the constraint exists before they discover it through sudo
- If a single source is genuinely reachable — the playbook templating its value from something the backend also reads — that is preferred over a guard, and the guard becomes unnecessary rather than additional
Related
- tech-debt(ansible): four playbook sites still state the code-source root as a literal after #15632 #15705 — recorded the constraint at the ansible site; this is the check it still lacks
- tech-debt(ansible): two pip constraint paths restate the code_source literal instead of deriving it from base_dir #15601 — its "only sites that genuinely need it" criterion is what surfaced this one as genuine
Activity
fix(ansible): make the code_source override authoritative for the who…
mrveiss commentedon Sep 5, 2026
Strengthened by review on #15706 — the drift is one env var away, not a relocation.
I filed this saying the two spellings "agree today by coincidence" and would diverge if someone moved the install root. That understated it. Verified:
api/code_sync.py:98imports that symbol. So any deployment that setsSLM_REPO_PATHmoves the Python side immediately, while the playbook literal and the NOPASSWD rule it writes stay at/opt/autobot/code_source. No relocation, no edit, no migration — one environment variable, andsudostops matching the argv on the provisioning path.That changes the severity and it changes the criteria. Added:
SLM_REPO_PATHthat differs from the playbook's value must fail loudly, at a point where the operator can see it, rather than at the moment sudo refuses a commandA second defect found on the way, filed separately
services/code_distributor.py:33binds the same environment variable to a different default:services/git_tracker.py:33/opt/autobot/code_sourceservices/code_distributor.py:33/opt/autobotSame name (
DEFAULT_REPO_PATH), same env var (SLM_REPO_PATH), two different meanings — a repository checkout in one, an install root in the other. Unset they disagree by a path component; set, one of the two is necessarily wrong, because a single value cannot be both. Filed as its own issue since it is a distinct defect from this coupling.Related
SLM_REPO_PATHin ittech-debt(ansible): derive the git_repo_root aliases, keep the sudoer…
Merge Dev_new_gui for #15707
guard(deploy): assert the two spellings of the provision playbook agr…
docs(tests): record why default_repo_path() is safe without comment s…
guard(deploy): four deploy facts stated twice, now stated once and ch…
github-actions commentedon Sep 7, 2026
PR #15901 (merged to
Dev_new_gui) references this issue with a close keyword.If this issue is fully resolved, close it manually. If work remains, no action is needed.
mrveiss commentedon Sep 7, 2026
Closed by #15901, merged as
539873f75. Verified against merged base (git show origin/Dev_new_gui:<path>).code_sync.pyrepo_tests/provision_playbook_path_agrees_15707_test.pyconfigure-python-provision-permissions.yml:45— "api/code_sync.py builds_PROVISION_PYTHON_PLAYBOOKfromDEFAULT_REPO_PATH… the NOPASSWD rule written below exact-matches that argv".code_sync.py:1621— "These paths MUST match the sudoers grant created by ansible/playbooks/configure-python-provision-permissions.yml"autobot.base_dir, the Python side fromDEFAULT_REPO_PATH, and the sudoers grant exact-matches an argv built on the Python side. The playbook's own comment records why deriving from the ansible SSOT does not work hereNote on what this cost, since the scheduling was the interesting part
This was held behind #15880 on a predicted file-size ratchet collision:
code_sync.pysat exactly at its ceiling, so adding a pointer comment would have forced a ceiling change in two mirror files while another PR was moving the same numbers.The collision never arose — the pointer comments already existed, added by #15705, so no
code_sync.pyedit was needed at all. The ordering was still correct: neither session could have known without looking, and looking was cheaper after #15880 landed than during it.A precaution that turns out unnecessary is not a precaution that was wrong. It was decided on the information available, and the information that dissolved it only existed later.
fix(repo_tests): make a docstring raw so parsing it stops warning (#1…