Reinstall module deps when they're missing from the environment - #3338
Conversation
The setup status cache recorded a module's deps as installed and never rechecked them, so a rebuilt virtualenv (e.g. `uv sync`) silently left modules broken until ~/.bbot was wiped by hand. Verify a module's pip packages are actually present before honoring a cache hit. Also promote `packaging` to a direct dependency; it was only present transitively via ansible.
The else branch in install()'s dependency loop was unreachable structure; the preceding branch always continues. Collapsing it also flattens the nested if/else into elif. pip_install() overwrote custom constraints with bbot's own instead of using them as a fallback. No module sets deps_pip_constraints today, so this changes no current behavior.
📊 Performance Benchmark Report
📈 Detailed Results (All Benchmarks)
🎯 Performance Summary! 1 regression ⚠️
30 unchanged ✅🔍 Significant Changes (>10%)
🐍 Python Version 3.11.15 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #3338 +/- ##
=====================================
+ Coverage 90% 90% +1%
=====================================
Files 450 450
Lines 46308 46407 +99
=====================================
+ Hits 41565 41660 +95
- Misses 4743 4747 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Verified both fixes.
Constraints inversion is real and dates to cec3cb4 (Oct 2024). Unnoticed because no module sets deps_pip_constraints, so only the falsy path was reachable. Reverting that one line fails the new test, so it gates properly.
Stale cache check: ran it against all 18 real module pip deps, zero false reinstalls. Edge cases correct. ~4ms per module at startup. The install() de-indent is behavior preserving.
Two non-blocking notes: pip-only, so it is a no-op for the 12 modules with only non-pip deps (dnsbrute, portscan, jadx, trufflehog, etc). And extras are parsed but unused, so pkg[extra] reads as satisfied. No module uses extras today.
Problem
BBOT pip-installs module deps into the active environment and records success in
~/.bbot/cache/depsinstaller/setup_status.json, keyed by a hash of the module's deps + venv path + bbot home + hostname + version. That hash never changes when the contents of the venv change.So when the venv gets rebuilt out from under BBOT (
uv syncprunes anything not in the lockfile, which includes every moduledeps_pip), the packages are gone but the cache still says "installed". BBOT skips the reinstall, reports the module as set up, and the module then fails to import.baddnsis a frequent casualty, but this hits every module with pip deps (wafw00f,markdown,neo4j,pymongo,sqlmodel, ...).The only known workaround was wiping
~/.bbotby hand.Reproducer against
dev:Fix
Before honoring a cache hit, check that the module's pip requirements are actually satisfied in the current environment (
importlib.metadata+packaging.requirements). If a package is missing or at a non-satisfying version, the cache entry is treated as a miss and deps are reinstalled.This mirrors how
install_core_depsalready works: it checks whether each binary is present rather than trusting a flag.Requirements that can't be parsed (VCS URLs) are assumed satisfied, and requirements whose environment markers don't apply are skipped, so neither causes a reinstall loop.
Same reproducer after the fix:
Also in here
packagingpromoted to a direct dependency. It was already present transitively viaansible-core/ansible-runner, but core code shouldn't rely on a transitive dep. Lockfile diff is 2 lines, no version churn.elsebranch removed frominstall()'s dependency loop. The preceding branch alwayscontinues, so theelsewas unreachable structure; collapsing it flattens the nestedif/elseintoelif. This is why the installer diff looks larger than the fix itself.pip_installnow honors custom constraints. The condition was inverted (if constraints is not None: constraints = get_python_constraints()), which replaced a caller's custom constraints with bbot's own instead of using bbot's as the fallback. No module setsdeps_pip_constraintstoday, so this changes no current behavior, but the code contradicted its own comment.Tests
test_depsinstaller_stale_pip_cachefails ondev(assert [] == [['bbot-nonexistent-package']]) and passes here. It also asserts the cache is still honored for deps that are genuinely installed, so the fix can't degrade into reinstalling on every scan.test_depsinstaller_pip_constraintslikewise fails against the old inverted condition. Plus unit coverage of the requirement check itself (missing, version mismatch, non-applicable marker, unparseable).Note on sudo prompts
A module with both
deps_aptanddeps_pip(e.g.sslcert) now re-runs its whole install when only the pip half went missing, which can prompt for a sudo password. That's identical to post-rm -rf ~/.bbotbehavior, so it's a return to correct behavior rather than a regression, but users who never saw a prompt after auv syncmay now see one.