diff --git a/AGENTS.md b/AGENTS.md index e5391fc870647..76c988fdd9904 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1260,12 +1260,8 @@ Use `get_hermes_home()` from `hermes_constants` for code paths. Use `display_her for user-facing print/log messages. Hardcoding `~/.hermes` breaks profiles — each profile has its own `HERMES_HOME` directory. This was the source of 5 bugs fixed in PR #3575. -### DO NOT introduce new `simple_term_menu` usage -Existing call sites in `hermes_cli/main.py` remain for legacy fallback only; -the preferred UI is curses (stdlib) because `simple_term_menu` has -ghost-duplication rendering bugs in tmux/iTerm2 with arrow keys. New -interactive menus must use `hermes_cli/curses_ui.py` — see -`hermes_cli/tools_config.py` for the canonical pattern. +### All CLI menu-pickers MUST use curses. +Interactive menus must use `hermes_cli/curses_ui.py`. See `hermes_cli/tools_config.py` for an example. ### DO NOT use `\033[K` (ANSI erase-to-EOL) in spinner/display code Leaks as literal `?[K` text under `prompt_toolkit`'s `patch_stdout`. Use space-padding: `f"\r{line}{' ' * pad}"`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4fbf5b5a3da68..e01511cafa025 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -723,22 +723,9 @@ that touches the OS, assume *any* platform can hit your code path. For process enumeration: PowerShell's `Get-CimInstance Win32_Process` is the modern replacement for `wmic process`. See `hermes_cli/gateway.py::_scan_gateway_pids` for the pattern. - -3. **`termios` and `fcntl` are Unix-only.** Always catch both `ImportError` - and `NotImplementedError`: - ```python - try: - from simple_term_menu import TerminalMenu - menu = TerminalMenu(options) - idx = menu.show() - except (ImportError, NotImplementedError): - # Fallback: numbered menu for Windows - for i, opt in enumerate(options): - print(f" {i+1}. {opt}") - idx = int(input("Choice: ")) - 1 ``` -4. **File encoding.** Windows may save `.env` files in `cp1252`. Always +3. **File encoding.** Windows may save `.env` files in `cp1252`. Always handle encoding errors: ```python try: @@ -750,7 +737,7 @@ that touches the OS, assume *any* platform can hit your code path. similar editors — use `encoding="utf-8-sig"` when reading files that could have been touched by a Windows GUI editor. -5. **Process management.** `os.setsid()`, `os.killpg()`, `os.fork()`, +4. **Process management.** `os.setsid()`, `os.killpg()`, `os.fork()`, `os.getuid()`, and POSIX signal handling differ on Windows. Guard with `platform.system()`, `sys.platform`, or `hasattr(os, "setsid")`: ```python @@ -774,29 +761,29 @@ that touches the OS, assume *any* platform can hit your code path. pass ``` -6. **Signals that don't exist on Windows: `SIGALRM`, `SIGCHLD`, `SIGHUP`, +5. **Signals that don't exist on Windows: `SIGALRM`, `SIGCHLD`, `SIGHUP`, `SIGUSR1`, `SIGUSR2`, `SIGPIPE`, `SIGQUIT`, `SIGKILL`.** Python's `signal` module raises `AttributeError` at import time if you reference them on Windows. Use `getattr(signal, "SIGKILL", signal.SIGTERM)` or gate the whole block behind a platform check. `loop.add_signal_handler` raises `NotImplementedError` on Windows — always catch it. -7. **Path separators.** Use `pathlib.Path` instead of string concatenation +6. **Path separators.** Use `pathlib.Path` instead of string concatenation with `/`. Forward slashes work almost everywhere on Windows, but `subprocess.run(["cmd.exe", "/c", ...])` and other shell contexts can require backslashes — convert with `str(path)` at the subprocess boundary, not inside Python logic. -8. **Symlinks need elevated privileges on Windows** (unless Developer Mode is +7. **Symlinks need elevated privileges on Windows** (unless Developer Mode is on). Tests that create symlinks need `@pytest.mark.skipif(sys.platform == "win32", reason="Symlinks require elevated privileges on Windows")`. -9. **POSIX file modes (0o600, 0o644, etc.) are NOT enforced on NTFS** by +8. **POSIX file modes (0o600, 0o644, etc.) are NOT enforced on NTFS** by default. Tests that assert on `stat().st_mode & 0o777` must skip on Windows — the concept doesn't translate. Use ACLs (`icacls`, `pywin32`) for Windows secret-file protection if needed. -10. **Detached background daemons on Windows need `pythonw.exe`, NOT +9. **Detached background daemons on Windows need `pythonw.exe`, NOT `python.exe`.** `python.exe` always allocates or attaches to a console, which makes it vulnerable to `CTRL_C_EVENT` broadcasts from any sibling process. `pythonw.exe` is the no-console variant. Combine with @@ -805,38 +792,38 @@ that touches the OS, assume *any* platform can hit your code path. See `hermes_cli/gateway_windows.py::_spawn_detached` for the reference implementation. -11. **`subprocess.Popen` with `.cmd` or `.bat` shims needs `shutil.which` +10. **`subprocess.Popen` with `.cmd` or `.bat` shims needs `shutil.which` to resolve.** Passing `"agent-browser"` to `Popen` on Windows finds the extensionless POSIX shebang shim in `node_modules/.bin/`, which `CreateProcessW` can't execute — you'll get `WinError 193 "not a valid Win32 application"`. Use `shutil.which("agent-browser", path=local_bin)` which honors PATHEXT and picks the `.CMD` variant on Windows. -12. **Don't use shell shebangs as a way to run Python.** `#!/usr/bin/env +11. **Don't use shell shebangs as a way to run Python.** `#!/usr/bin/env python` only works when the file is executed through a Unix shell. `subprocess.run(["./myscript.py"])` on Windows fails even if the file has a shebang line. Always invoke Python explicitly: `[sys.executable, "myscript.py"]`. -13. **Shell commands in installers.** If you change `scripts/install.sh`, +12. **Shell commands in installers.** If you change `scripts/install.sh`, make the equivalent change in `scripts/install.ps1`. The two scripts are the canonical example of "works on Linux does not mean works on Windows" and have drifted multiple times — keep them in lockstep. -14. **Known paths that are OneDrive-redirected on Windows:** Desktop, +13. **Known paths that are OneDrive-redirected on Windows:** Desktop, Documents, Pictures, Videos. The "real" path when OneDrive Backup is enabled is `%USERPROFILE%\OneDrive\Desktop` (etc.), NOT `%USERPROFILE%\Desktop` (which exists as an empty husk). Resolve the real location via `ctypes` + `SHGetKnownFolderPath` or by reading the `Shell Folders` registry key — never assume `~/Desktop`. -15. **CRLF vs LF in generated scripts.** Windows `cmd.exe` and `schtasks` +14. **CRLF vs LF in generated scripts.** Windows `cmd.exe` and `schtasks` parse line-by-line; mixed or LF-only line endings can break multi-line `.cmd` / `.bat` files. Use `open(path, "w", encoding="utf-8", newline="\r\n")` — or `open(path, "wb")` + explicit bytes — when generating scripts Windows will execute. -16. **Two different quoting schemes in one command line.** `subprocess.run +15. **Two different quoting schemes in one command line.** `subprocess.run (["schtasks", "/TR", some_cmd])` → schtasks itself parses `/TR`, AND the `some_cmd` string is re-parsed by `cmd.exe` when the task fires. Different parsers, different escape rules. Use two separate quoting @@ -846,18 +833,15 @@ that touches the OS, assume *any* platform can hit your code path. ### Testing cross-platform -Tests that use POSIX-only syscalls need a skip marker. Common ones: -- Symlinks → `@pytest.mark.skipif(sys.platform == "win32", ...)` -- `0o600` file modes → `@pytest.mark.skipif(sys.platform.startswith("win"), ...)` -- `signal.SIGALRM` → Unix-only (per-test timeouts no longer use it directly; see the win32 timeout-method shim in `tests/conftest.py::pytest_configure`) -- `os.setsid` / `os.fork` → Unix-only -- Live Winsock / Windows-specific regression tests → - `@pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific regression")` +Tests that excercise behavior on specific platforms must run on their target platforms. -If you monkeypatch `sys.platform` for cross-platform tests, also patch -`platform.system()` / `platform.release()` / `platform.mac_ver()` — each -re-reads the real OS independently, so half-patched tests still route -through the wrong branch on a Windows runner. +```python +@pytest.mark.linux_only +@pytest.mark.macos_only +@pytest.mark.windows_only +``` +Avoid monkeypatching `sys.platform` unless absolutely needed, but if you do, also patch `platform.system()` / `platform.release()` / `platform.mac_ver()`. +Symlinks, 0o600 permissions, SIGALRM, os.setsid/fork are all unix-only. --- diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 772e9606baa05..0ecd267513b62 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -7626,7 +7626,7 @@ def _prompt_model_selection( menu_title = "Select default model:" if has_pricing: # Align the header with the model column. - # Each choice is " {label}" (2 spaces) and simple_term_menu prepends + # Each choice is " {label}" (2 spaces) and we prepend # a 3-char cursor region ("-> " or " "), so content starts at col 5. pad = " " * 5 header = f"\n{pad}{'':>{name_col}} {'In':>{price_col}} {'Out':>{price_col}}" @@ -7639,10 +7639,6 @@ def _prompt_model_selection( menu_title += " ★ = on sale" # Try arrow-key menu first, fall back to number input. - # Uses the shared curses radiolist (ESC/arrow-key handling that works - # across terminals, incl. those that emit raw escape sequences) instead - # of simple_term_menu, which conflicts with /dev/tty and left ESC/arrow - # keys unreliable in the setup model picker. try: from hermes_cli.curses_ui import curses_radiolist diff --git a/hermes_cli/curses_ui.py b/hermes_cli/curses_ui.py index 301b2295594c1..9a127d936d4a8 100644 --- a/hermes_cli/curses_ui.py +++ b/hermes_cli/curses_ui.py @@ -340,11 +340,10 @@ def _handle_active_search_key( def flush_stdin() -> None: """Flush any stray bytes from the stdin input buffer. - Must be called after ``curses.wrapper()`` (or any terminal-mode library - like simple_term_menu) returns, **before** the next ``input()`` / - ``getpass.getpass()`` call. ``curses.endwin()`` restores the terminal - but does NOT drain the OS input buffer — leftover escape-sequence bytes - (from arrow keys, terminal mode-switch responses, or rapid keypresses) + Must be called after ``curses.wrapper()`` returns, and before the next + ``input()`` / ``getpass.getpass()`` call. + ``curses.endwin()`` restores the terminal but does NOT drain the OS input buffer. + Leftover escape-sequence bytes (from arrow keys, terminal mode-switch responses, or rapid keypresses) remain buffered and silently get consumed by the next ``input()`` call, corrupting user data (e.g. writing ``^[^[`` into .env files). @@ -874,8 +873,7 @@ def curses_single_select( ) -> int | None: """Curses single-select menu. Returns selected index or None on cancel. - Works inside prompt_toolkit because curses.wrapper() restores the terminal - safely, unlike simple_term_menu which conflicts with /dev/tty. + Works inside prompt_toolkit. curses.wrapper() restores the terminal safely. When ``searchable`` is true, ``/`` opens a type-to-filter prompt; the returned value is always the original item index (or None for cancel). diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 5741a85f0a6cc..b1047c44c379f 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -1082,8 +1082,6 @@ def _session_browse_picker(sessions: list) -> Optional[str]: """Interactive curses-based session browser with live search filtering. Returns the selected session ID, or None if cancelled. - Uses curses (not simple_term_menu) to avoid the ghost-duplication rendering - bug in tmux/iTerm when arrow keys are used. """ if not sessions: print("No sessions found.") diff --git a/pyproject.toml b/pyproject.toml index 2e25486333655..25d16bef19ab5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -192,7 +192,6 @@ matrix = ["mautrix[encryption]==0.21.0", "aiosqlite==0.22.1", "asyncpg==0.31.0", # and XXE. aiohttp/httpx are already in [messaging]; defusedxml lands # here to keep the dependency local to wecom_callback's threat model. wecom = ["defusedxml==0.7.1"] -cli = ["simple-term-menu==1.6.6"] tts-premium = ["elevenlabs==1.59.0"] voice = [ # Local STT pulls in wheel-only transitive deps (ctranslate2, onnxruntime). @@ -280,7 +279,6 @@ termux = [ # Baseline Android / Termux path for reliable fresh installs. "python-telegram-bot[webhooks]==22.6", "hermes-agent[cron]", - "hermes-agent[cli]", "hermes-agent[mcp]", "hermes-agent[honcho]", "hermes-agent[acp]", @@ -349,7 +347,6 @@ all = [ # and failed on `make`. Lazy-install routes that build to first use, # where the user is expected to have a toolchain available. "hermes-agent[cron]", - "hermes-agent[cli]", "hermes-agent[pty]", "hermes-agent[mcp]", "hermes-agent[homeassistant]", diff --git a/tests/hermes_cli/test_setup_menu_curses_migration.py b/tests/hermes_cli/test_setup_menu_curses_migration.py deleted file mode 100644 index 2993113f813c6..0000000000000 --- a/tests/hermes_cli/test_setup_menu_curses_migration.py +++ /dev/null @@ -1,67 +0,0 @@ -"""Regression tests confirming the setup model/provider/reasoning pickers route -through the shared curses radiolist (ESC + arrow-key handling that works across -terminals, incl. Ghostty) instead of simple_term_menu. - -Guards against silently regressing back to simple_term_menu, whose ESC/arrow -handling was unreliable in `hermes setup` (the provider->model sub-menu). -""" -from unittest.mock import patch - - -def test_prompt_model_selection_uses_curses_radiolist(): - from hermes_cli.auth import _prompt_model_selection - from hermes_cli.curses_ui import radio_item_plain - - seen = {} - - def _fake( - title, - items, - *, - selected=0, - cancel_returns=None, - description=None, - searchable=False, - search_labels=None, - ): - seen["title"] = title - seen["items"] = items - seen["search_labels"] = search_labels - return 1 # pick second model - - with patch("hermes_cli.curses_ui.curses_radiolist", side_effect=_fake), \ - patch("builtins.print"): - result = _prompt_model_selection(["model-a", "model-b"]) - - assert result == "model-b" - assert seen["title"] == "Select default model:" - # Items are the models plus the custom/skip entries. Model rows may be - # rich (text, style) segments for sale chrome — compare the plain text. - plain = [radio_item_plain(item) for item in seen["items"]] - assert plain[:2] == ["model-a", "model-b"] - assert "Skip (keep current)" in plain - assert seen["search_labels"] is not None - assert len(seen["search_labels"]) == len(seen["items"]) - - -def test_prompt_model_selection_esc_cancels(): - from hermes_cli.auth import _prompt_model_selection - - # curses_radiolist returns the cancel sentinel (-1) on ESC. - with patch("hermes_cli.curses_ui.curses_radiolist", return_value=-1), \ - patch("builtins.print"): - result = _prompt_model_selection(["model-a", "model-b"]) - - assert result is None - - -def test_reasoning_effort_uses_curses_radiolist(): - from hermes_cli.main import _prompt_reasoning_effort_selection - - with patch("hermes_cli.curses_ui.curses_radiolist", return_value=2), \ - patch("builtins.print"): - result = _prompt_reasoning_effort_selection(["low", "medium", "high"], current_effort="") - - assert result == "high" - - diff --git a/uv.lock b/uv.lock index fb624cbc9e457..e02f0840f45af 100644 --- a/uv.lock +++ b/uv.lock @@ -1616,7 +1616,6 @@ all = [ { name = "mcp" }, { name = "pyasn1" }, { name = "python-multipart" }, - { name = "simple-term-menu" }, { name = "starlette" }, { name = "uvicorn", extra = ["standard"] }, { name = "youtube-transcript-api" }, @@ -1630,9 +1629,6 @@ azure-identity = [ bedrock = [ { name = "boto3" }, ] -cli = [ - { name = "simple-term-menu" }, -] computer-use = [ { name = "mcp" }, { name = "starlette" }, @@ -1744,7 +1740,6 @@ termux = [ { name = "honcho-ai" }, { name = "mcp" }, { name = "python-telegram-bot", extra = ["webhooks"] }, - { name = "simple-term-menu" }, { name = "starlette" }, ] termux-all = [ @@ -1761,7 +1756,6 @@ termux-all = [ { name = "pyasn1" }, { name = "python-multipart" }, { name = "python-telegram-bot", extra = ["webhooks"] }, - { name = "simple-term-menu" }, { name = "starlette" }, { name = "uvicorn", extra = ["standard"] }, ] @@ -1845,8 +1839,6 @@ requires-dist = [ { name = "google-auth-oauthlib", marker = "extra == 'google'", specifier = "==1.3.1" }, { name = "hermes-agent", extras = ["acp"], marker = "extra == 'all'" }, { name = "hermes-agent", extras = ["acp"], marker = "extra == 'termux'" }, - { name = "hermes-agent", extras = ["cli"], marker = "extra == 'all'" }, - { name = "hermes-agent", extras = ["cli"], marker = "extra == 'termux'" }, { name = "hermes-agent", extras = ["cron"], marker = "extra == 'all'" }, { name = "hermes-agent", extras = ["cron"], marker = "extra == 'termux'" }, { name = "hermes-agent", extras = ["google"], marker = "extra == 'all'" }, @@ -1918,7 +1910,6 @@ requires-dist = [ { name = "sentencepiece", marker = "extra == 'wake'", specifier = "==0.2.2" }, { name = "setuptools", marker = "extra == 'dev'", specifier = "==83.0.0" }, { name = "sherpa-onnx", marker = "extra == 'wake'", specifier = "==1.13.4" }, - { name = "simple-term-menu", marker = "extra == 'cli'", specifier = "==1.6.6" }, { name = "slack-bolt", marker = "extra == 'messaging'", specifier = "==1.29.0" }, { name = "slack-bolt", marker = "extra == 'slack'", specifier = "==1.29.0" }, { name = "slack-sdk", marker = "extra == 'messaging'", specifier = "==3.43.0" }, @@ -1940,7 +1931,7 @@ requires-dist = [ { name = "websockets", specifier = "==15.0.1" }, { name = "youtube-transcript-api", marker = "extra == 'youtube'", specifier = "==1.2.4" }, ] -provides-extras = ["anthropic", "exa", "firecrawl", "parallel-web", "fal", "edge-tts", "modal", "daytona", "vercel", "hindsight", "dev", "messaging", "cron", "slack", "matrix", "wecom", "cli", "tts-premium", "voice", "wake", "honcho", "supermemory", "mem0", "vision", "pty", "mcp", "nemo-relay", "homeassistant", "sms", "teams", "computer-use", "acp", "mistral", "otlp", "bedrock", "vertex", "azure-identity", "termux", "termux-all", "dingtalk", "feishu", "google", "youtube", "web", "all"] +provides-extras = ["anthropic", "exa", "firecrawl", "parallel-web", "fal", "edge-tts", "modal", "daytona", "vercel", "hindsight", "dev", "messaging", "cron", "slack", "matrix", "wecom", "tts-premium", "voice", "wake", "honcho", "supermemory", "mem0", "vision", "pty", "mcp", "nemo-relay", "homeassistant", "sms", "teams", "computer-use", "acp", "mistral", "otlp", "bedrock", "vertex", "azure-identity", "termux", "termux-all", "dingtalk", "feishu", "google", "youtube", "web", "all"] [[package]] name = "hf-xet" @@ -4169,15 +4160,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/82/40/ee8a0a8c83fc6d7f5245a5a031e471d3b115e20cce867e7abb2f9d4185c9/sherpa_onnx-1.13.4-cp313-cp313-win_amd64.whl", hash = "sha256:17050fdfb48d37ae996364f697c554a1399740d18e5a56b143c011d00cfed3e0", size = 2244504, upload-time = "2026-07-07T12:32:59.882Z" }, ] -[[package]] -name = "simple-term-menu" -version = "1.6.6" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/d8/80/f0f10b4045628645a841d3d98b584a8699005ee03a211fc7c45f6c6f0e99/simple_term_menu-1.6.6.tar.gz", hash = "sha256:9813d36f5749d62d200a5599b1ec88469c71378312adc084c00c00bfbb383893", size = 35493, upload-time = "2024-12-02T16:31:50.639Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/9c/09/21d993e394c1fe5c44cd90453d88ed44932da8dfca006e424c072d77d29b/simple_term_menu-1.6.6-py3-none-any.whl", hash = "sha256:c2a869efa7a9f7e4a9c25858b42ca6974034951c137d5e281f5339b06ed8c9c2", size = 27600, upload-time = "2024-12-02T16:31:48.934Z" }, -] - [[package]] name = "six" version = "1.17.0" diff --git a/website/docs/developer-guide/contributing.md b/website/docs/developer-guide/contributing.md index 3e5cf9d981037..3ed14ba030b11 100644 --- a/website/docs/developer-guide/contributing.md +++ b/website/docs/developer-guide/contributing.md @@ -159,23 +159,7 @@ When contributing code, keep these rules in mind: Key patterns: -### 1. `termios` and `fcntl` are Unix-only - -Always catch both `ImportError` and `NotImplementedError`: - -```python -try: - from simple_term_menu import TerminalMenu - menu = TerminalMenu(options) - idx = menu.show() -except (ImportError, NotImplementedError): - # Fallback: numbered menu - for i, opt in enumerate(options): - print(f" {i+1}. {opt}") - idx = int(input("Choice: ")) - 1 -``` - -### 2. File encoding +### 1. File encoding Some environments may save `.env` files in non-UTF-8 encodings: @@ -186,7 +170,7 @@ except UnicodeDecodeError: load_dotenv(env_path, encoding="latin-1") ``` -### 3. Process management +### 2. Process management `os.setsid()`, `os.killpg()`, and signal handling differ across platforms: @@ -196,7 +180,7 @@ if platform.system() != "Windows": kwargs["preexec_fn"] = os.setsid ``` -### 4. Path separators +### 3. Path separators Use `pathlib.Path` instead of string concatenation with `/`. diff --git a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/developer-guide/contributing.md b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/developer-guide/contributing.md index 480018a1e1b55..2122a16fa2486 100644 --- a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/developer-guide/contributing.md +++ b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/developer-guide/contributing.md @@ -132,24 +132,7 @@ Hermes 官方支持 **Linux、macOS、WSL2 以及原生 Windows（通过 PowerSh - **使用 `pathlib.Path` / `os.path.join`，不得手动用 `/` 拼接路径。** 这对我们构造后传给子进程的字符串尤为重要，而非 OS 返回给我们的字符串。 关键模式： - -### 1. `termios` 和 `fcntl` 仅适用于 Unix - -始终同时捕获 `ImportError` 和 `NotImplementedError`： - -```python -try: - from simple_term_menu import TerminalMenu - menu = TerminalMenu(options) - idx = menu.show() -except (ImportError, NotImplementedError): - # 回退：编号菜单 - for i, opt in enumerate(options): - print(f" {i+1}. {opt}") - idx = int(input("Choice: ")) - 1 -``` - -### 2. 文件编码 +### 1. 文件编码 某些环境可能以非 UTF-8 编码保存 `.env` 文件： @@ -160,7 +143,7 @@ except UnicodeDecodeError: load_dotenv(env_path, encoding="latin-1") ``` -### 3. 进程管理 +### 2. 进程管理 `os.setsid()`、`os.killpg()` 以及信号处理在各平台间存在差异： @@ -170,7 +153,7 @@ if platform.system() != "Windows": kwargs["preexec_fn"] = os.setsid ``` -### 4. 路径分隔符 +### 3. 路径分隔符 使用 `pathlib.Path` 代替用 `/` 进行字符串拼接。