Repository navigation
deploy(update): the venv husk repair is unreachable from ansible provisioning, so a husked host stays broken #17339
Description
Activity
- addedbugSomething isn't workingSomething isn't working
on Sep 23, 2026 - added 5 commits that reference this issue
on Sep 23, 2026 Two review findings from f8 on #17360, both verified in the tree before acting, plus one residual worth recording separately.
1. The marker-capable set was three, not two — and my test could not have told me.
autobot-ai-stackreachesreconcile_componentthroughcode_sync._WORKER_COMPONENT_PIP(:1686) and is not invenv_reconcile.EXPLICIT_LIST_COMPONENTS(:94), so it takes the else-branch and its venv gets stamped.roles/ai-stack/tasks/main.ymlhas its own pip steps at:145and:242against{{ ai_install_dir }}/venv. So a host whose ai-stack was code-synced before #17332 and is later provisioned by ansible has exactly this issue's gap, in a third venv. Fixed: the include now runs before that role's first pip step.The test that missed it is the part worth recording. It was parametrised over the two roles I had asserted, so it checked my claim against itself — a test whose parametrisation encodes the thing it should verify cannot falsify it. The replacement derives the component set from source with
ast(both dicts that reachreconcile_component, minus the explicit-list components) and requires a checked-in component→role map to cover exactly that set, so a new marker-capable component fails the file until someone maps it.2. The single-source-of-truth guard could not catch its own worked example.
It matched
-name '*dist-info— no dot — while real directories arepkg-1.0.dist-info. Soansible.builtin.command: find {{ dir }} -name '*.dist-info' -delete, the exact reimplementation the guard exists to stop, passed all four assertions. The task file being clean made it report a true negative, which is how a matcher that cannot match looks correct. Fixed, with a positive control that feeds the guard that very line.The scan now reads the YAML round-trip rather than raw text, because this file has to be able to explain in prose what it forbids — a raw scan flagged its own documentation, which is the #16750 shape.
Mutation-verified, and the miss is part of the record:
a find -name '*.dist-info' STEP added -> 1 failed ai-stack's include removed -> 2 failed a new marker-capable component in code_sync -> 1 failed clean tree -> 20 passedMy first attempt at the second mutation edited an indentation that does not exist in that file, changed nothing, and 'passed' — proving nothing. Checking that a mutation actually mutates is part of the check.
Residual, not fixed here and not mine originally:
is_provenance_huskfollows symlinks, so a*.dist-infosymlink pointing at a directory whose sole entry is a file namedAUTOBOT_PROVENANCEwould have that file unlinked through the link. Inherited from #17332 and theoretical rather than reachable — but this PR is what newly runs that predicate as root, unconditionally, on every provisioned host, so the blast radius is larger than when it was code-sync-only. Filing separately rather than widening this PR; recorded here so the connection is not lost.- added a commit that references this issue
on Sep 24, 2026 Closing evidence — verified against merged
main(54fec85388)All three criteria checked by reading the merged tree (
git show origin/main:<path>), not the PR diff.AC1 — a husked venv is repaired on a host whose only entry point is ansible provisioning. MET.
All three marker-capable roles include the shared repair:
roles/ai-stack/tasks/main.yml -> clear_venv_husks.yml (1) roles/backend/tasks/main.yml -> clear_venv_husks.yml (1) roles/slm_manager/tasks/main.yml -> clear_venv_husks.yml (1)Three, not the two the original task header claimed:
_WORKER_COMPONENT_PIPmakesautobot-ai-stackmarker-capable because it is absent fromEXPLICIT_LIST_COMPONENTS. The set is derived from those constants bytest_the_role_map_covers_every_marker_capable_componentrather than hand-written, so a fourth marker-capable component reddens that test instead of going quietly unrepaired.AC2 — the pip step that previously aborted now succeeds; the repair runs before it. MET, and this was the pre-merge finding.
:374 ansible.builtin.include_tasks: ../../_shared/tasks/clear_venv_husks.yml :381 cmd: "{{ slm_backend_dir }}/venv/bin/pip install -e {{ slm_base_dir }}/autobot_shared"The repair now precedes the first pip invocation. It did not in the reviewed revision — the include sat at
:389, after the editable install at:372, in the one venv the task's own header calls critical because it runs the builtin updater, so a husk there blocks the only other path to its own repair.The guard did not catch that, and the reason is the part worth recording.
test_the_repair_precedes_the_first_pip_steplocated the first pip step asrendered.index("ansible.builtin.pip:"), which cannot seeansible.builtin.command:invoking.../venv/bin/pip install -e. The assertion was true of what it measured and false of what its name claimed.The widened matcher then had a defect of its own:
"pip install"matched prose —roles/backend/tasks/main.yml:348describes its deploy window as "apt/pip install, code sync, alembic migrations" — and failed a correctly-ordered role for a sentence. Both offsets now come from_task_content(), the YAML round-trip already in that file because a scan that reads comments makes the comment the defect (#16750). Mutation-proved on the real file in both directions, with the mutation confirmed applied before the verdict was read.AC3 — the narrow-deletion property is preserved. MET.
venv_provenance.py:136is_provenance_huskremains strictly conjunctive, and says why in the code:"Recognised narrowly and by evidence, because the remedy is a deletion: the directory must describe no installed distribution (no METADATA, no RECORD) AND carry this tool's marker AND contain nothing else. Anything else — another tool's leftovers, a partial install, a directory with files this module did not write — is not ours to classify and is left alone."
The ansible task adds no second predicate: it contains no
find,with_fileglob,rm -rfor shell, and only invokes the Python module, gated on the venv existing.test_the_shared_task_does_not_re_express_the_classificationfails if a future edit reimplements the rule in YAML.Not claimed
Filed separately from the same review, deliberately not folded in: #17371 —
_record_marker_in_recordrewrites pip'sRECORDwith a truncate-and-rewrite rather than temp-file +os.replace. An interrupted run can truncate the manifest of a working package, which is a worse failure than the husk this repairs. It predates this PR.
#17332stops the provenance marker creatingdist-infohusks and clears the existing ones —but only on the path that runs
api/venv_reconcile.py. A host reached only by ansibleprovisioning keeps its husks, and its pip step keeps aborting.
The gap, stated exactly
#17332wiresclear_provenance_husksinto_run_pip_install(
autobot-slm-backend/api/venv_reconcile.py), which the builtin code-sync / self-update pathcalls (
api/code_sync.py). That is the right home: updates go through the builtin updater, andthe tool that wrote the marker owns cleaning up after it.
Ansible never calls it:
So
roles/backend's ownpip:task — the one that failed on the manager in the run that foundthis — still meets whatever husks are on disk. The prevention half is complete everywhere, since
nothing but the reconciler writes the marker; the repair half is not.
Why this is not simply "add a task"
The repair logic must stay one definition (
venv_provenance.is_provenance_huskis deliberatelynarrow — no METADATA, no RECORD, and the marker as the sole content). Re-expressing that rule
in an ansible
shell:step would fork the classification that decides a deletion, which is theone place a second implementation is least acceptable.
Invoking the module on the target is not free either: it lives in
code_source, which is thecontroller's checkout and is not on a fleet node — the
#17242/#17331invariant.Options
(the
#17331staging task is already there) and invoke it with that venv's own interpreter.One definition, runs anywhere, costs one staged file.
provisioning depend on the manager's HTTP surface being up, which first provisioning cannot
assume.
provisioning is first-install where no husk can exist yet. True for a clean host; false for
every host that already has them — which is how this was found.
Recommendation: option 1, and note that option 3's premise is exactly the assumption that
produced
#17332in the first place ("a reinstall replaces the directory").Acceptance criteria
the same
is_provenance_huskclassification and not a second copy of the rule.uninstall-no-record-filecompletes.another tool's leftovers are all untouched.
Related
#17332— the defect and the prevention + reconciler-side repair#17331— the staging task this could reuse#17038— deletion of stored state is proposed and approved, never silent; a husk istool-authored debris carrying this tool's own marker, which is why it is classifiable at all