Repository navigation
Skip Hermes source venvs built for another Python version - #19160
Merged
Merged
Conversation
The TUI Python wrapper picked $HERMES_PYTHON_SRC_ROOT/{venv,.venv} before
the launcher's own interpreter (#12996). A Hermes source checkout managed by
`hermes pm` runs on a store interpreter, and its .venv can be an unrelated
test env on another Python minor version, so the gateway crash-looped
loading extensions built for the launcher's version.
Resolve the launcher interpreter first, and skip a source venv whose
pyvenv.cfg version differs from it. Venvs without a readable version, or
when the launcher is unknown, are still preferred as before, which keeps
the symlinked-venv fix for #12948.
Fixes #18680
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No concrete merge-blocking issue was established for the supported Hermes invocation. The change is mergeable after normal checks. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (1 skipped: 1 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (24 passed)
-
Check name Status Explanation Title check The title clearly identifies the main behavior change: skipping Hermes source virtual environments built for a different Python version. Description check The description clearly explains the problem, resulting behavior, implementation, issue linkage, and test command. It uses equivalent headings for Summary and Testing, but it omits the required Change… Linked Issues check The active direct issue is #18680. The wrapper now discovers the Hermes launcher interpreter before source-root virtual environments. It reads the venvpyvenv.cfgversion and compares major.minor wi…Out of Scope Changes check The changes are limited to Resources/bin/cmux-hermes-python-wrapperand its focused test file. The helper functions, launcher-selection logic, version checks, and new tests directly support issue#1…Cmux Cloud Persistent Session And Early Input PASS: The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. The diff changes Hermes Python interpreter selection and adds tests. It does no…Cmux Swift Actor Isolation The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. It introduces no production Swift changes, so it cannot introduce or worsen the speci…Cmux Swift Blocking Runtime The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. It introduces no Swift production changes, so the Swift blocking-runtime check is not…Cmux Browser Automation Off-Main PASS: The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. It does not change browser socket commands,Sources/TerminalController.swift,…Cmux Expensive Synchronous Load The custom check applies only to production Swift changes. The reviewed diff changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py; it contains no Swift, Obj…Cmux Cache Substitution Correctness PASS: The pull request changes a Bash production wrapper and Python tests, not production Swift, TypeScript, or JavaScript. The diff updates interpreter selection and adds version checks; it does not … Cmux No Hacky Sleeps The production change only discovers the launcher and compares Python versions from pyvenv.cfg; it introduces no sleep, timer, polling, fixed delay, retry wait, or wall-clock synchronization. The ad…Cmux Algorithmic Complexity The production change uses bounded iteration over exactly two possible venv directories and two interpreter names per directory (Resources/bin/cmux-hermes-python-wrapper:75-90). The pyvenv.cfg scan is… Cmux Swift Concurrency The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. It introduces no Swift, Objective-C, or Objective-C++ code, so the Swift concurrency …Cmux Swift @ConcurrentPASS: The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. The diff contains no Swift files or Swift concurrency changes, so `.github/revi…Cmux Swift Package Boundaries The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. It contains no production Swift changes and no Swift package boundary changes, so thi…Cmux Swiftpm Lockfiles The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. It does not change SwiftPM packages, Xcode project package references,.gitignoref…Cmux Swift Logging The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. It adds no Swift code and no production Swift logging. The Swift logging check is the…Cmux User-Facing Error Privacy The production path reaches Hermes TUI users because cmux-hermes-agent-wrappersetsHERMES_PYTHONtocmux-hermes-python-wrapper(lines 381-408). The diff changes interpreter selection only. The …Cmux Full Internationalization PASS: The PR changes only a Bash Hermes Python wrapper and Python tests. It adds no Swift UI text, app catalog or Info.plist entries, web UI/API copy, metadata, markdown, changelog, or locale-specific… Cmux Swiftui State Layout The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. The diff contains no SwiftUI views,ObservableObject,@Published, `GeometryReader…Cmux Architecture Rethink PASS: The pull request changes only the Bash wrapper and Python tests. It introduces no Swift architecture changes, lifecycle ownership, timing repair, or duplicate Swift wiring covered by the rule. Cmux Swift Auxiliary Window Close Shortcuts The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. The authoritative diff contains no Swift, storyboard, XIB, or window-related files, s…Cmux Source Artifacts The diff changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. Both are intentional product source and test code. The test creates temporary data at runtime…Cmux No Test Or Debug Seam In Production Source The pull request changes only Resources/bin/cmux-hermes-python-wrapperandtests/test_hermes_wrapper_hooks.py. It changes no Swift file under a productionSources/path, so this custom check doe…
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
-
- Commit to this branch
- Create a new PR
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Contributor
|
|
Collaborator
Author
|
Subagent review at 840a8d1: merge. Ran tests/test_hermes_wrapper_hooks.py on the head (passes, including the #12948 symlinked-venv case) and with main's wrapper swapped in (the new test fails with the #18680 symptom). pyvenv.cfg parsing falls back to the old venv-first behavior on anything unknown. Follow-up nit: the read loop skips a final line with no trailing newline. |
Contributor
|
Merge receipt for |
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
1 participant
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.
What was wrong
Since #12996,
cmux-hermes-python-wrapperpicked$HERMES_PYTHON_SRC_ROOT/{venv,.venv}/bin/python{,3}before the interpreter that launched Hermes. That fixed #12948: on macOSps commresolves a symlinked venv Python to the base interpreter, which lacks the venv's site-packages. However, a Hermes source checkout managed byhermes pmruns on a store interpreter, and its.venvcan be an unrelated test env on another Python minor version. The gateway then ran on that venv's Python (for example 3.11), Hermes's bootstrap added site-packages built for the launcher's 3.14, and the gateway crash-looped withNo module named 'pydantic_core._pydantic_core'.What changed
ps commof the TUI's parent) first.pyvenv.cfgversion/version_infomajor.minor differs from the launcher's (read with<launcher> -I -S -c ..., only when the launcher's basename looks like Python). A mismatched venv is skipped, and the launcher interpreter is used.pyvenv.cfg, no launcher found), the venv is preferred as before. The Hermes integration: gateway crash-loops when the Hermes venv python is a symlink (cmux-hermes-python-wrapper resolves the base interpreter viaps comm) #12948 case (symlinked uv venv, launcher reported as the framework base Python of the same version) still selects the venv.How tested
test_tui_python_wrapper_skips_venv_for_other_python_versionintests/test_hermes_wrapper_hooks.pyruns the wrapper under a real Python launcher with an intermediate process, matching Hermes's process tree. A.venvwhosepyvenv.cfgnames another version must lose to the launcher, and one naming the launcher's version must still win. The mismatched case fails against themainwrapper (expected the launcher interpreter, got venv).python3 tests/test_hermes_wrapper_hooks.pypasses locally, including the existing Hermes integration: gateway crash-loops when the Hermes venv python is a symlink (cmux-hermes-python-wrapper resolves the base interpreter viaps comm) #12948 symlinked-venv test.Fixes #18680
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes Hermes gateway crash-loops (
No module named 'pydantic_core._pydantic_core') when a source checkout's.venvruns a different Python minor version than the launcher. The wrapper now resolves the launcher interpreter first and only prefers the source-root venv when itspyvenv.cfgversion matches the launcher's.ps comm) #12948.Written for commit 840a8d1. Summary will update on new commits.
Note
Medium Risk
Changes TUI gateway interpreter selection in the Hermes launch path; wrong logic could break venv preference on macOS or reintroduce version-mismatch crashes.
Overview
Fixes Hermes TUI gateway crash-loops when a source checkout’s
.venvtargets a different Python minor than the interpreter that launched Hermes (e.g. store Python 3.14 vs an unrelated test venv on 3.11).cmux-hermes-python-wrappernow resolves the launcher interpreter first (viapson the TUI’s grandparent), then still prefers$HERMES_PYTHON_SRC_ROOT/{venv,.venv}only whenpyvenv.cfgmajor.minor matches the launcher (or when either version is unknown). Mismatched venvs are skipped and the launcher is used. Helpers read the launcher version with a short-I -S -cprobe and the venv version frompyvenv.cfgwithout executing the venv.Tests add a Hermes-like process tree and assert mismatched venvs lose to the launcher while same-version venvs still win (preserving the symlinked-venv/macOS behavior from #12948).
Reviewed by Cursor Bugbot for commit 840a8d1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
python3as a fallback.