Skip to content

deploy(update): the venv husk repair is unreachable from ansible provisioning, so a husked host stays broken #17339

Description

@mrveiss

#17332 stops the provenance marker creating dist-info husks and clears the existing ones —
but only on the path that runs api/venv_reconcile.py. A host reached only by ansible
provisioning keeps its husks, and its pip step keeps aborting.

The gap, stated exactly

#17332 wires clear_provenance_husks into _run_pip_install
(autobot-slm-backend/api/venv_reconcile.py), which the builtin code-sync / self-update path
calls (api/code_sync.py). That is the right home: updates go through the builtin updater, and
the tool that wrote the marker owns cleaning up after it.

Ansible never calls it:

$ grep -rn 'venv_reconcile\|clear_provenance_husks' autobot-slm-backend/ansible/
(no matches)

So roles/backend's own pip: task — the one that failed on the manager in the run that found
this — 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_husk is deliberately
narrow — 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 the
one place a second implementation is least acceptable.

Invoking the module on the target is not free either: it lives in code_source, which is the
controller's checkout and is not on a fleet node — the #17242/#17331 invariant.

Options

  1. Run it from the venv being repaired. The module is stdlib-only; stage it beside the venv
    (the #17331 staging task is already there) and invoke it with that venv's own interpreter.
    One definition, runs anywhere, costs one staged file.
  2. Call the SLM API from the play. Keeps one implementation and no staging, but makes a node's
    provisioning depend on the manager's HTTP surface being up, which first provisioning cannot
    assume.
  3. Accept the gap and document it: husks are repaired at the next builtin update, and
    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 #17332 in the first place ("a reinstall replaces the directory").

Acceptance criteria

  • A husked venv is repaired on a host whose only entry point is ansible provisioning, using
    the same is_provenance_husk classification and not a second copy of the rule.
  • Verified against a venv carrying a husk, asserting the pip step that previously aborted with
    uninstall-no-record-file completes.
  • The narrow-deletion property is preserved: a real installation, an operator's files and
    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 is
    tool-authored debris carrying this tool's own marker, which is why it is classifiable at all

Activity

  1. mrveiss commented on Sep 24, 2026

    @mrveiss
    OwnerAuthor

    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-stack reaches reconcile_component through code_sync._WORKER_COMPONENT_PIP (:1686) and is not in venv_reconcile.EXPLICIT_LIST_COMPONENTS (:94), so it takes the else-branch and its venv gets stamped. roles/ai-stack/tasks/main.yml has its own pip steps at :145 and :242 against {{ 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 reach reconcile_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 are pkg-1.0.dist-info. So ansible.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 passed
    

    My 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_husk follows symlinks, so a *.dist-info symlink pointing at a directory whose sole entry is a file named AUTOBOT_PROVENANCE would 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.

  2. mrveiss commented on Sep 24, 2026

    @mrveiss
    OwnerAuthor

    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_PIP makes autobot-ai-stack marker-capable because it is absent from EXPLICIT_LIST_COMPONENTS. The set is derived from those constants by test_the_role_map_covers_every_marker_capable_component rather 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_step located the first pip step as rendered.index("ansible.builtin.pip:"), which cannot see ansible.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:348 describes 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:136 is_provenance_husk remains 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 -rf or shell, and only invokes the Python module, gated on the venv existing. test_the_shared_task_does_not_re_express_the_classification fails 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_record rewrites pip's RECORD with 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.

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions