feat(plugins): warn when plugin venv was built with a different Python#43
Closed
lukeocodes wants to merge 1 commit intomainfrom
Closed
feat(plugins): warn when plugin venv was built with a different Python#43lukeocodes wants to merge 1 commit intomainfrom
lukeocodes wants to merge 1 commit intomainfrom
Conversation
When a user installs deepctl via Homebrew (or any path that ends up using
IsolatedVenvStrategy), then later upgrades the underlying interpreter
(e.g. brew upgrade python@3.13), the running interpreter no longer
matches the Python that built ~/.deepctl/plugins/venv/. Pure-Python
plugins keep loading because PluginManager bridges them via sys.path,
but C-extension plugins fail with a confusing low-level ImportError that
gives users no obvious remediation.
This adds a one-line warning at plugin-load time that surfaces the
mismatch and tells the user how to rebuild:
Plugin environment was built with Python 3.12 but you're running
Python 3.13. C-extension plugins may fail to load. To rebuild:
rm -rf ~/.deepctl/plugins/venv && deepctl plugin install <plugin>
Implementation:
- New get_venv_python_version() in plugin_env.py reads pyvenv.cfg and
returns (major, minor). Handles both stdlib's 'version' key and uv's
'version_info' key. Returns None on missing/malformed cfg so callers
can treat 'unknown' as 'no warning'.
- New _warn_if_plugin_venv_python_mismatch() in PluginManager fires
before sys.path bridging in _load_plugin_venv_entries, only when
there's a real mismatch (no warning for unknown / matching versions).
Tests:
- 6 cases for get_venv_python_version (missing venv, missing cfg,
stdlib 'version' key, uv 'version_info' key, malformed, no version key)
- 4 cases for the warning logic (silent on unknown, silent on match,
warns on minor diff, warns on major diff)
All 48 plugin tests pass. mypy + ruff clean on changed src files.
Member
Author
|
Folded into #42 so all the Homebrew-tap-related changes for deepgram/cli ship as a single review unit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the last edge case in our "Homebrew installs + plugins" story. When
brew upgrade python@3.13rebuilds deepctl against a newer underlying Python, the user's plugin venv at~/.deepctl/plugins/venv/keeps the old Python it was built with. C-extension plugins then fail to load with a confusing low-levelImportErrorand no breadcrumb to the fix.This PR detects that mismatch and prints a one-line, actionable warning at plugin-load time:
Pure-Python plugins continue to load transparently via
sys.pathbridging — the warning fires alongside normal loading, not as a blocker.Why now
Companion to:
deepgram/homebrew-tap#1— the new Homebrew formuladeepgram/cli#42— release-automation that keeps the formula freshThe plugin system already routes Homebrew installs through
IsolatedVenvStrategy, so plugins survive the install path by design. The Python-version-bump scenario is the only remaining failure mode that's hard to diagnose from a user's perspective. Closing it now means we ship the Homebrew install path with a complete plugin story.What lands
packages/deepctl-core/src/deepctl_core/plugin_env.pyget_venv_python_version()readspyvenv.cfg, returns(major, minor)orNoneon missing/malformed cfgpackages/deepctl-core/src/deepctl_core/plugin_manager.py_warn_if_plugin_venv_python_mismatch()fires inside_load_plugin_venv_entries, before sys.path bridgingpackages/deepctl-core/tests/unit/test_plugin_env.pyget_venv_python_versionpackages/deepctl-core/tests/unit/test_plugin_manager.pyDesign notes
pyvenv.cfgparsing handles bothversion(stdlibpython -m venv) andversion_info(uv-created venvs). Both formats useMAJOR.MINOR.PATCH...semantics so splitting on.and taking[0:2]works for both.Nonefromget_venv_python_versionmeans "unknown, skip the check" — explicitly documented in the function's docstring and respected by_warn_if_plugin_venv_python_mismatch. Avoids noisy false-positive warnings when the cfg is missing on weird/legacy venvs.plugin venv exists+plugins.jsonnon-empty), so users without plugins never see it.Verification
pytest packages/deepctl-core/tests/unit/test_plugin_*.py -vruff check(changed src files)ruff format --check(changed src files)mypy --strict(changed src files)_load_plugin_venv_entriesbehaviorThe 21 ruff warnings and 5 mypy errors visible in a wider check are all pre-existing in files this PR doesn't touch (
auth.py,deepctl_cmd_mcp,test_auth.py, etc.) — out of scope per repo policy.Pre-existing comments / docstrings
A few docstrings in the new code were retained as Category 3 necessary:
get_venv_python_version— public API indeepctl_core, documents the dualversion/version_infokey support and theNonecontract that_warn_if_plugin_venv_python_mismatchdepends on_warn_if_plugin_venv_python_mismatch— documents when it fires (the Homebrew Python-bump scenario) and why we warn instead of block (so future "robustness" PRs don't break the pure-Python plugin case by raising/skipping)-voutputEach describes a behavioral contract that the code alone can't communicate; removing them would predictably lead to wrong-direction refactors.
Out of scope
This PR doesn't try to automatically recreate the plugin venv on mismatch. That would be a more invasive change with its own failure modes (what if the rebuild fails partway through? what about user-installed dev plugins via
--editable?). The single-line warning with explicit remediation is a much smaller blast radius. If users frequently hit this, we can layer auto-recovery on top later.