Skip to content

feat(update): venv reconciliation needs install-provenance, not a name diff, to protect operator packages #15067

Closed

Description

@mrveiss

What

#15063's venv reconciliation (autobot-slm-backend/api/venv_reconcile.py) removes a
package only when it was in the tool's own prior-declared lock and is no longer
declared now. That protects a name the tool's history never declared, but it
has one narrow, proven hole:

A package an operator installs under a name this venv's history once
declared — after that name left requirements, before the next reconcile — is
indistinguishable from tool debris and IS removed.

Reproduced live in autobot-slm-backend/tests/test_venv_reconcile.py,
test_reconcile_removes_a_name_an_operator_reinstalled_after_it_was_declared:
install pkg-stale-collide into a real venv, put that name in the lock's
previous set (a prior reconcile once declared it), leave it out of current
requirements → the package is removed, full stop, regardless of who put it
there this time.

This is inherent to a name-diff design with no install-provenance marker —
not a bug in the current implementation, and #15063 was explicit that closing
it there would mean approximating rather than a sound fix. This issue is that
follow-up.

Why the obvious fix doesn't work

pip writes INSTALLER = pip in a distribution's dist-info regardless of
who ran pip install — the tool's own reconcile and an operator typing
pip install foo by hand produce the identical INSTALLER value. That field
alone cannot discriminate "this tool put it here" from "something else did".

Worth investigating

  • Whether direct_url.json (present when pip has provenance info, e.g. a
    -e or URL install) carries anything usable — likely not for a plain
    pip install -r requirements.txt entry, but worth checking rather than
    assuming.
  • Recording something at install time that only this tool's invocation
    writes — e.g. a marker file, a install-batch id, or a hash of the exact
    reconcile run — so a later reconcile can tell "I put this specific
    installation here" apart from "this name is merely declared again".
  • Whether that marker needs to live per-package (dist-info-adjacent) or
    whether the existing per-venv lock file can carry enough (e.g. an
    install epoch/session id keyed by name) without becoming its own source
    of drift.
  • Whatever the mechanism, it must survive pip install -r requirements.txt
    upgrading a package in place — an upgrade should keep the provenance
    marker, not reset the venv into looking "unclaimed" again.

Acceptance criteria

Related

Activity

  1. github-actions commented on Sep 3, 2026

    @github-actions
    Contributor

    PR #15572 (merged to Dev_new_gui) references this issue with a close keyword.

    feat(update): gate venv removal on install provenance, not a name diff (#15067)

    If this issue is fully resolved, close it manually. If work remains, no action is needed.

  2. mrveiss commented on Sep 3, 2026

    @mrveiss
    OwnerAuthor

    Closed on code evidence, verified against merged Dev_new_gui (PR #15572, 488817ad).

    AC Evidence in base
    Removal decided by provenance, not a name diff venv_provenance.split_by_provenance splits removal candidates into verified and unverified; _apply_removals removes only verified
    Provenance survives an operator's manual reinstall The marker lives in the package's own dist-info. pip fully replaces that directory on install or uninstall, so marker-absence is positive evidence something touched the install since this tool last confirmed it — which is precisely what pip's INSTALLER field cannot express
    The KNOWN LIMITATION paragraph replaced api/venv_reconcile.py's docstring rewritten; the limitation it described is the thing this closes
    An operator package is not removed test_reconcile_survives_a_name_an_operator_reinstalled_after_it_was_declared — an inverted test, not a new one. It previously asserted the reinstall is removed. That old assertion encoded the defect, so the fix had to change it rather than add a sibling

    The first-run question is answered explicitly rather than left to fall out. Every deployed host has a venv with no markers, including packages this tool did install. Treating absence as "ours" destroys operator packages on the first run; treating it as "never ours" leaves #15063 dead on every existing host. Neither is the default: unverified candidates are refused and reported, and removing them takes an env opt-in an operator can review the reported list against and then unset. Coverage grows on its own as each run re-marks still-declared packages — no migration step, no flag day.

    38 tests pass; each of the three load-bearing behaviours was mutation-proven, including inverting the verified/unverified split and forcing has_tool_provenance true.

    #15063 stays open deliberately. Its reconciler already existed; what this adds is the gate that makes removal safe. Confirming the first run on a real host with a pre-existing lock and zero markers reports its candidates rather than silently dropping them needs host evidence, and today's self-update is a reminder why that distinction matters — the process performing it predated the code it deployed by 181 seconds.

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