Skip to content

Fix Hermes gateway with symlinked venv Python - #12996

Merged
teamleaderleo merged 3 commits into
mainfrom
task-12948-hermes-python-symlink
Sep 24, 2026
Merged

teamleaderleo merged 3 commits into
mainfrom
task-12948-hermes-python-symlink

Conversation

@austinywang

@austinywang austinywang commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix Hermes Python wrapper resolution for uv/Homebrew environments where venv/bin/python is a symlink. The wrapper now prefers the interpreter under HERMES_PYTHON_SRC_ROOT before falling back to process discovery, preserving the venv site-packages used by the gateway.

Adds an executable regression test covering a symlinked venv interpreter.

Closes #12948

Validation

  • Focused symlinked interpreter regression: passed
  • bash -n Resources/bin/cmux-hermes-python-wrapper: passed
  • python3 tests/test_hermes_wrapper_hooks.py: existing TUI bridge lifecycle assertion fails in this environment (unrelated to this change)

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes the Hermes gateway in uv/Homebrew setups where venv/bin/python is a symlink. The wrapper previously resolved the launcher's executable to the base interpreter, which lacks Hermes's site-packages; it now prefers the interpreter under HERMES_PYTHON_SRC_ROOT first and falls back to process discovery. Closes #12948.

  • Adds an executable regression test covering a symlinked venv interpreter.

Written for commit f3486bf. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved Python interpreter selection to prioritize the Hermes environment’s local virtual environment.
    • Added fallback behavior for installations where the source environment cannot be discovered.
    • Improved support for symlinked virtual-environment interpreters on macOS.
  • Tests

    • Added coverage verifying that the correct virtual-environment interpreter is selected and preserves command-line arguments.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Note

Pull Request opener @austinywang is not an author or co-author of any commit in this PR (commit identities: lawrencecchen, teamleaderleo). The CLA check will still proceed and requires every listed identity plus @austinywang to have signed.

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d39df4dd-dfab-4ae2-9b17-f340754469aa

📥 Commits

Reviewing files that changed from the base of the PR and between 341dda7 and f3486bf.

📒 Files selected for processing (2)
  • Resources/bin/cmux-hermes-python-wrapper
  • tests/test_hermes_wrapper_hooks.py

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 059596f5-8c91-4b38-adfa-d8905c271bbd

📥 Commits

Reviewing files that changed from the base of the PR and between 9c2ba78 and 341dda7.

📒 Files selected for processing (2)
  • Resources/bin/cmux-hermes-python-wrapper
  • tests/test_hermes_wrapper_hooks.py

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The Hermes Python wrapper now prefers interpreters from the source-root virtual environment before using ps comm discovery. A regression test covers symlinked venv interpreters and verifies TUI argument forwarding.

Changes

Hermes interpreter resolution

Layer / File(s) Summary
Source-root interpreter precedence
Resources/bin/cmux-hermes-python-wrapper
The wrapper checks environment-local interpreter candidates under HERMES_PYTHON_SRC_ROOT before the existing ps comm fallback. Comments document the macOS symlink behavior.
Symlinked venv regression coverage
tests/test_hermes_wrapper_hooks.py
The test creates a symlinked venv interpreter, runs the TUI wrapper, verifies successful execution and original argument forwarding, and registers the test in main().

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant TUI as TUI Python wrapper
  participant Venv as Hermes venv interpreter
  participant Gateway as tui_gateway.entry
  TUI->>Venv: Select interpreter under HERMES_PYTHON_SRC_ROOT
  TUI->>Gateway: Execute -m tui_gateway.entry with original arguments
  Gateway-->>TUI: Exit successfully
Loading
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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 4 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 ✅ Passed The title clearly identifies the Hermes gateway fix and the symlinked virtual-environment Python issue.
Description check ✅ Passed The description provides the change rationale, testing results, regression coverage, linked issue, and known unrelated failure. It omits the template's demo video, review-trigger block, and checklist,…
Linked Issues check ✅ Passed The change addresses issue #12948. cmux_hermes_resolve_real_python now checks HERMES_PYTHON_SRC_ROOT/venv/bin/python and related venv paths before ps comm. This selects the venv interpreter when…
Out of Scope Changes check ✅ Passed The reviewed changes are limited to Hermes Python interpreter resolution and a regression test for the linked-venv case in issue #12948. The wrapper retains its existing hook bootstrap and fallback be…
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS: The review-range diff changes only Hermes Python interpreter resolution and its regression test. It does not change Cloud terminal creation, cmux-tui client or transport allocation, PTY or shell…
Cmux Swift Actor Isolation ✅ Passed The pull request changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py. The patch contains no Swift or production Swift UI code. Therefore it cannot introduc…
Cmux Swift Blocking Runtime ✅ Passed PASS: The pull request changes only a Bash wrapper and a Python test. It introduces no production Swift changes and no covered blocking or timing-based synchronization in the authoritative diff.
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py. The policy-scoped Swift files remain unchanged, and the diff adds no `browser.*…
Cmux Expensive Synchronous Load ✅ Passed PASS. The authoritative pull-request diff changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py; it contains no Swift changes. The custom check applies to ex…
Cmux Cache Substitution Correctness ✅ Passed PASS: The reviewed diff changes only Resources/bin/cmux-hermes-python-wrapper (Bash) and tests/test_hermes_wrapper_hooks.py (Python). It does not contain a production Swift, TypeScript, or JavaScr…
Cmux No Hacky Sleeps ✅ Passed PASS: The PR introduces no fixed sleep, timer, polling, delayed dispatch, or wall-clock synchronization. The production shell change only reorders interpreter resolution and retains ps lookup as a f…
Cmux Algorithmic Complexity ✅ Passed PASS: The production change adds only a fixed four-candidate loop for venv and .venv interpreter paths. It does not scan user-owned collections, rescan targets, sort/filter scalable data, or add n…
Cmux Swift Concurrency ✅ Passed PASS: The pull request changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py; it contains no Swift files. The diff adds shell resolution logic and a Python r…
Cmux Swift @Concurrent ✅ Passed PASS: The pull request changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py. The authoritative diff contains no Swift files, Swift declarations, or Swift ca…
Cmux Swift Package Boundaries ✅ Passed PASS: The authoritative diff changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py. These are Bash and Python files. The patch contains no Swift, SwiftPM, Ap…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The pull request changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py. The diff contains no Package.swift, Package.resolved, .gitignore, Xcode p…
Cmux Swift Logging ✅ Passed PASS: The pull request changes only a Bash wrapper and a Python test; it adds no Swift files or Swift runtime logging. The added printf calls are inside test fixture scripts, and the test’s print …
Cmux User-Facing Error Privacy ✅ Passed PASS. The production diff only changes interpreter-selection logic and developer comments. It does not add or modify user-facing errors, alerts, command output, API bodies, or recovery copy. The new t…
Cmux Full Internationalization ✅ Passed PASS. The pull request changes only the Hermes Python wrapper and its regression test. The production diff adds or changes shell comments and interpreter-selection logic, not user-facing Swift, web, m…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only a Bash Hermes wrapper and a Python test. The authoritative diff contains no Swift or SwiftUI files and introduces none of the prohibited state, layout, list-row, or…
Cmux Architecture Rethink ✅ Passed PASS: The pull request changes only a shell wrapper and a Python test. It introduces no Swift architecture change, timing repair, mutable Swift state, duplicate Swift entrypoint wiring, or split UI li…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The pull request changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py. The authoritative diff contains no Swift files and no NSWindow, NSPanel, `N…
Cmux Source Artifacts ✅ Passed PASS. The authoritative diff changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py. These are intentional product source and regression-test files. The patch…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The pull request changes only Resources/bin/cmux-hermes-python-wrapper and tests/test_hermes_wrapper_hooks.py. It adds no Swift file under a production Sources/ path, so the specified prod…
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 4 functions across 1 files. (1 skipped: 1 unsupported.)

✨ 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

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lawrencecchen

Copy link
Copy Markdown
Contributor

Mac fleet instructions for head 341dda70f8362313c00820da407770e939dab301. Planned tag: pr-12996-341dda70; this is not yet a published build.

JOB_JSON=$(~/.local/bin/cmux-ci submit --kind cmux --command 'CMUX_FLEET_BUILD_TAG=pr-12996-341dda70 /Users/Shared/cmux-build-fleet/recipes/cmux.sh https://github.com/manaflow-ai/cmux.git 341dda70f8362313c00820da407770e939dab301' --artifact artifacts/cmux.app.zip --workspace https://github.com/manaflow-ai/cmux/pull/12996 --source-digest 341dda70f8362313c00820da407770e939dab301 --cache-key cmux:pr-12996 --min-free-bytes 268435456000 --label cmux --label ram48)
JOB_ID=$(python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])' <<<"$JOB_JSON")
~/.local/bin/cmux-ci wait "$JOB_ID" --receipt artifacts/fleet/$JOB_ID.json
~/.local/bin/cmux-ci publish-hq "$JOB_ID"

Use an existing campaign job ID if one is already posted; do not submit a duplicate. A wait timeout leaves the remote job running. Published results will include an exact-head artifact link and timing/disk receipt. This recipe validates the macOS app only, not iOS or tests. Never use maclease or put credentials in a PR comment.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 24, 2026 08:56
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Reviewed: Hermes always exports HERMES_PYTHON_SRC_ROOT, so preferring $ROOT/{venv,.venv}/bin/python over the ps comm lookup (which resolves the symlink to the base interpreter) is right. pipx and site-packages installs still fall through to ps comm. The red checks came from an iOS fixture failure on main that's since been fixed. I merged main in so the macOS lane runs, and turned on auto-merge. Thanks :)

@teamleaderleo
teamleaderleo merged commit 7b1cb4a into main Sep 24, 2026
55 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 24, 2026
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 24, 2026
4aa2736 Fix cmux events access-denied stream error (manaflow-ai#10712)
f4ff9af ci: never let a focused test run pass after executing zero tests (manaflow-ai#14053)
fbaf239 Fix persistent LaunchServices registration from duplicate plist keys (manaflow-ai#12990)
7b1cb4a Fix Hermes gateway with symlinked venv Python (manaflow-ai#12996)
b6b2720 ssh-tmux mirror: preserve deliberate pane titles (manaflow-ai#10714)
33edbc7 Fix Cloud VM panel text readability across all terminal themes (manaflow-ai#7538)
30dccd6 fix: prevent detached TUI preferred editor processes (manaflow-ai#10681)
22eec58 Reap disowned shell watchers on parent PID reuse (issue 10926) (manaflow-ai#11035)
0b9b318 ci: start Linux-only jobs beside Fast static checks (manaflow-ai#14181)
9e78d22 ci: one git archive for the trusted router; delete duplicate CI guard tests (manaflow-ai#14199)
20e79e6 feat: load local cmux config packs (manaflow-ai#13356)
224327b ci: download the admission DerivedData seed while packages resolve (manaflow-ai#14184)
886a6f0 ci: run changed suites inside compile admission (manaflow-ai#14182)

# Conflicts:
#	.github/workflows/ci-guards.yml
#	.github/workflows/ci-macos.yml
#	.github/workflows/ci.yml
#	.github/workflows/test-e2e.yml
#	.github/workflows/test-macos-suite.yml
teamleaderleo added a commit that referenced this pull request Oct 11, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hermes integration: gateway crash-loops when the Hermes venv python is a symlink (cmux-hermes-python-wrapper resolves the base interpreter via ps comm)

3 participants