fix(install): detect non-Python venv/bin/hermes and regenerate via pip#22388
Closed
wesleysimplicio wants to merge 1 commit into
Closed
Conversation
Problem ------- After running the curl|bash installer (or after any condition that clobbers the pip-generated console script), `venv/bin/hermes` can end up as a bash wrapper that execs back into itself. The installer then writes `~/.local/bin/hermes` as a launcher that execs `$HERMES_BIN`, which execs `venv/bin/hermes`, which execs itself — `hermes` hangs silently with no output, no error, no CPU activity. Root cause ---------- `scripts/install.sh::setup_path` validates only that `$HERMES_BIN` exists and is executable. It never inspects the file's shape, so a clobbered console script with a bash shebang slips through and the launcher we write on top of it perpetuates the recursion. Fix --- After the existing `[ ! -x "$HERMES_BIN" ]` check, inspect the first line of `$HERMES_BIN`. A correct pip-generated entry point starts with a python shebang (`#!.../python...`). Anything else triggers a `pip install --force-reinstall --no-deps -e .` to regenerate the console script in place; if regeneration also fails, surface the exact manual fix instead of silently writing the broken launcher. The guard is conditional on `USE_VENV=true` because the system-install path has no venv to reinstall into. Tests ----- `tests/test_install_sh_recursive_wrapper_guard.py` asserts the guard's shape: it inspects `$HERMES_BIN`'s shebang, accepts a python shebang only, regenerates with `--force-reinstall --no-deps`, references the issue number so future maintainers can trace it, falls back to a manual instruction on failure, runs before the launcher write, and is gated on `USE_VENV`. Pre-existing install.sh tests (pythonpath sanitization / setup wizard tty / termux prereqs) still pass. Bash syntax validated with `bash -n`. Closes NousResearch#21802 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens scripts/install.sh against a regression where venv/bin/hermes is clobbered into a self-referential bash wrapper, causing the installed hermes command to hang silently (infinite exec recursion), and adds a regression test to ensure the guard is not removed in future refactors.
Changes:
- Add a shebang-based validation step in
setup_path()to detect non-Pythonvenv/bin/hermesand attempt regeneration with a force-reinstall pip install. - Print a manual remediation command when regeneration fails, avoiding writing a launcher that would perpetuate the hang.
- Add a pytest regression test module that asserts the guard’s presence, ordering, and
USE_VENVgating.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/install.sh |
Adds a defensive guard to detect a non-Python venv entrypoint and attempt to regenerate it before writing the launcher shim. |
tests/test_install_sh_recursive_wrapper_guard.py |
Adds regression tests that assert the new guard exists, is ordered correctly, and is gated on USE_VENV. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1126
to
+1132
| return 0 | ||
| ;; | ||
| esac | ||
| else | ||
| log_warn "Could not regenerate venv entry point" | ||
| log_info "Manual fix: cd $INSTALL_DIR && uv pip install -e '.[all]'" | ||
| return 0 |
| *) | ||
| log_warn "venv entry point at $HERMES_BIN is not a Python script" | ||
| log_info "Regenerating with pip to avoid recursive-exec hang (#21802)..." | ||
| if (cd "$INSTALL_DIR" && ./venv/bin/python -m pip install \ |
Contributor
|
Closing in favor of #25734 (merged as c75e1a0), which fixes the root cause of the recursive-wrapper hang — the The shebang-inspection + |
19 tasks
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.
Problem
After running the curl|bash installer (or after any condition that clobbers the pip-generated console script),
venv/bin/hermescan end up as a bash wrapper that execs back into itself. The installer then writes~/.local/bin/hermesas a launcher that execs\$HERMES_BIN, which execsvenv/bin/hermes, which execs itself —hermeshangs silently with no output, no error, no CPU activity. The reporter describes the exact shape in #21802.Root cause
scripts/install.sh::setup_pathvalidates only that\$HERMES_BINexists and is executable. It never inspects the file's shape, so a clobbered console script with a bash shebang slips through and the launcher we write on top of it perpetuates the recursion.Fix
After the existing
[ ! -x \"\$HERMES_BIN\" ]check, inspect the first line of\$HERMES_BIN. A correct pip-generated entry point starts with a python shebang (#!.../python...). Anything else triggers apip install --force-reinstall --no-deps -e .to regenerate the console script in place; if regeneration also fails, surface the exact manual fix instead of silently writing the broken launcher.The guard is conditional on
USE_VENV=truebecause the system-install path has no venv to reinstall into.Tests
tests/test_install_sh_recursive_wrapper_guard.pyasserts the guard's shape:\$HERMES_BIN's shebang--force-reinstall --no-depsUSE_VENVPre-existing install.sh tests (pythonpath sanitization, setup wizard tty probe, termux network prereqs) still pass. Bash syntax validated with
bash -n.Closes #21802