Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -1284,14 +1284,15 @@ def profile_env(tmp_path, monkeypatch):
### Python
**ALWAYS use `scripts/run_tests.sh`** — do not call `pytest` directly. The script enforces
hermetic environment parity with CI (unset credential vars, TZ=UTC, LANG=C.UTF-8,
`-n auto` xdist workers, in-tree subprocess-isolation plugin). Direct `pytest`
per-file subprocess isolation via `scripts/run_tests_parallel.py` — no xdist,
worker count auto-scaled from CPU count). Direct `pytest`
on a 16+ core developer machine with API keys set diverges from CI in ways
that have caused multiple "works locally, fails in CI" incidents (and the reverse).

```bash
scripts/run_tests.sh # full suite, CI-parity
scripts/run_tests.sh tests/gateway/ # one directory
scripts/run_tests.sh tests/agent/test_foo.py::test_x # one test
scripts/run_tests.sh tests/agent/test_foo.py -k test_x # one test (file + -k; the runner is file-granular)
scripts/run_tests.sh -v --tb=long # pass-through pytest flags
```

Expand Down
5 changes: 3 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ ln -sf "$(pwd)/venv/bin/hermes" ~/.local/bin/hermes
### Run tests

```bash
# Preferred — matches CI (hermetic env, 4 xdist workers); see AGENTS.md
# Preferred — matches CI (hermetic `env -i`, per-file subprocess isolation
# via run_tests_parallel.py, worker count auto-scaled); see AGENTS.md
scripts/run_tests.sh

# Alternative (activate the venv first). The wrapper is still recommended
Expand Down Expand Up @@ -848,7 +849,7 @@ that touches the OS, assume *any* platform can hit your code path.
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 (see `tests/conftest.py::_enforce_test_timeout`)
- `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")`
Expand Down
16 changes: 14 additions & 2 deletions cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -3235,10 +3235,12 @@ def _termux_example_image_path(filename: str = "cat.png") -> str:
"/storage/emulated/0",
"/storage/self/primary",
]
# Termux/Android roots are POSIX paths — join with literal forward
# slashes so the hint stays correct even when this renders on Windows.
for root in candidates:
if os.path.isdir(root):
return os.path.join(root, "Pictures", filename)
return os.path.join("~/storage/shared", "Pictures", filename)
return f"{root}/Pictures/{filename}"
return f"~/storage/shared/Pictures/{filename}"


def _split_path_input(raw: str) -> tuple[str, str]:
Expand Down Expand Up @@ -3309,6 +3311,16 @@ def _resolve_attachment_path(raw_path: str) -> Path | None:
expanded = unquote(parsed.path or "")
if parsed.netloc and os.name == "nt":
expanded = f"//{parsed.netloc}{expanded}"
elif (
os.name == "nt"
and len(expanded) >= 3
and expanded[0] == "/"
and expanded[1].isalpha()
and expanded[2] == ":"
):
# file:///C:/... parses to path "/C:/..." — drop the
# leading slash so it resolves as a drive-letter path.
expanded = expanded[1:]
except Exception:
expanded = token
expanded = os.path.expandvars(os.path.expanduser(expanded))
Expand Down
7 changes: 5 additions & 2 deletions gateway/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,9 +507,12 @@ def _command_line_belongs_to_profile(command: str, profile_home: Path) -> bool:
explicit ``HERMES_HOME=<path>``) on its argv; the default/root gateway runs
bare with no profile flag.
"""
command_lc = command.lower()
# Normalize separators before the substring match: on Windows,
# str(Path) renders backslashes while a HERMES_HOME= value on the argv
# may carry forward slashes (Git Bash, JSON configs) — and vice versa.
command_lc = command.lower().replace("\\", "/")
profile_name = _profile_name_for_home(profile_home)
home_lc = str(profile_home).lower()
home_lc = str(profile_home).lower().replace("\\", "/")

if profile_name is not None and profile_name != "default":
profile_lc = profile_name.lower()
Expand Down
9 changes: 8 additions & 1 deletion hermes_cli/banner.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,14 @@ def cprint(text: str):
"""Print ANSI-colored text through prompt_toolkit's renderer."""
from prompt_toolkit import print_formatted_text as _pt_print
from prompt_toolkit.formatted_text import ANSI as _PT_ANSI
_pt_print(_PT_ANSI(text))
try:
_pt_print(_PT_ANSI(text))
except Exception:
# prompt_toolkit needs a real console. On Windows, a redirected or
# absent stdout (pythonw.exe, CI, `hermes ... > file`) raises
# NoConsoleScreenBufferError from its Win32Output — display helpers
# must never crash the caller over that, so degrade to plain print.
print(text)


# =========================================================================
Expand Down
6 changes: 5 additions & 1 deletion hermes_cli/browser_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import logging
import os
import platform
import posixpath
import shlex
import shutil
import subprocess
Expand Down Expand Up @@ -95,7 +96,10 @@ def add_windows_install_paths(
for _, group in install_groups:
for base in filter(None, bases):
for parts in group:
add(os.path.join(base, *parts))
# Only called with WSL ``/mnt/c/...`` bases — those are
# POSIX paths regardless of the host OS, so join with
# posixpath (os.path.join would emit backslashes on nt).
add(posixpath.join(base, *parts))

if system == "Darwin":
for app in _DARWIN_APPS:
Expand Down
6 changes: 4 additions & 2 deletions hermes_cli/gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,15 +359,17 @@ def _scan_gateway_pids(
looks_like_gateway_runtime_command_line,
)
current_home = str(get_hermes_home().resolve())
current_home_lc = current_home.lower()
# Forward slashes on both sides of the HERMES_HOME= match — see
# gateway.status._command_line_belongs_to_profile, which this mirrors.
current_home_lc = current_home.lower().replace("\\", "/")
current_profile_arg = _profile_arg(current_home)
current_profile_name = (
current_profile_arg.split()[-1] if current_profile_arg else ""
)
current_profile_name_lc = current_profile_name.lower()

def _matches_current_profile(command: str) -> bool:
command_lc = command.lower()
command_lc = command.lower().replace("\\", "/")
if current_profile_name:
return (
f"--profile {current_profile_name_lc}" in command_lc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ run_conversation():

### Testing

Use the canonical runner — it enforces CI-parity (hermetic env, unset
credentials, TZ=UTC, xdist workers, per-test subprocess isolation):
Use the canonical runner — it enforces CI-parity (hermetic `env -i`, unset
credentials, TZ=UTC, per-file subprocess isolation via
`scripts/run_tests_parallel.py` — no xdist, worker count auto-scaled):

```bash
scripts/run_tests.sh # full suite
Expand All @@ -102,7 +103,7 @@ scripts/run_tests.sh -v --tb=long # pass-through pytest flags
**Cross-platform test guards:** tests using POSIX-only syscalls need a skip marker. Common ones already in the codebase:
- Symlink creation → `@pytest.mark.skipif(sys.platform == "win32", reason="Symlinks require elevated privileges on Windows")` (see `tests/cron/test_cron_script.py`)
- POSIX file modes (0o600, etc.) → `@pytest.mark.skipif(sys.platform.startswith("win"), reason="POSIX mode bits not enforced on Windows")` (see `tests/hermes_cli/test_auth_toctou_file_modes.py`)
- `signal.SIGALRM` → Unix-only (see `tests/conftest.py::_enforce_test_timeout`)
- `signal.SIGALRM` → Unix-only (per-test timeouts no longer use it directly; see the win32 timeout-method shim in `tests/conftest.py::pytest_configure`)
- Live Winsock / Windows-specific regression tests → `@pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific regression")`

**Monkeypatching `sys.platform` is not enough** when the code under test also calls `platform.system()` / `platform.release()` / `platform.mac_ver()`. Those functions re-read the real OS independently, so a test that sets `sys.platform = "linux"` on a Windows runner will still see `platform.system() == "Windows"` and route through the Windows branch. Patch all three together:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@ echo `os.environ` inside an `execute_code` block to confirm `SYSTEMROOT` is set.

`scripts/run_tests.sh` is POSIX-only (expects `.venv/bin/activate`); the
Hermes-installed `venv/Scripts/` has no pip/pytest (stripped for size).
Install pytest into a system Python and run directly with `-n 0`
(`pyproject.toml`'s `addopts` already sets `-n`):
Install pytest into a system Python and run directly (the repo no longer
uses pytest-xdist; the canonical runner does per-file subprocess isolation,
which the POSIX-only wrapper handles):

```bash
"/c/Program Files/Python311/python" -m pip install --user pytest pytest-xdist pyyaml
"/c/Program Files/Python311/python" -m pip install --user pytest pyyaml
export PYTHONPATH="$(pwd)"
"/c/Program Files/Python311/python" -m pytest tests/foo/test_bar.py -v --tb=short -n 0
"/c/Program Files/Python311/python" -m pytest tests/foo/test_bar.py -v --tb=short
```

(POSIX-only tests need skip guards — see the cross-platform guard list in
Expand Down
12 changes: 7 additions & 5 deletions skills/creative/comfyui/tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ When you change a script:

## Why the explicit `-c` / `-o`?

The parent hermes-agent repo's `pyproject.toml` enables `pytest-xdist` by
default (`-n auto`). This suite is small enough that parallelism isn't
worth the complexity, and pytest-xdist isn't always installed in the user's
environment. The `-c tests/pytest.ini -o addopts="-p no:xdist"` flags make
the suite run identically regardless of the parent project's config.
The parent hermes-agent repo used to enable `pytest-xdist` by default
(`-n auto`); the canonical runner has since moved to per-file subprocess
isolation via `scripts/run_tests_parallel.py` and no longer uses xdist.
This suite is small enough that parallelism isn't worth the complexity, and
pytest-xdist isn't always installed in the user's environment. The
`-c tests/pytest.ini -o addopts="-p no:xdist"` flags make the suite run
identically regardless of the parent project's config.
16 changes: 7 additions & 9 deletions skills/software-development/python-debugpy/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,9 @@ scripts/run_tests.sh tests/path/to/test_file.py::test_name --trace
scripts/run_tests.sh tests/path/to/test_file.py --showlocals --tb=long
```

Note: `scripts/run_tests.sh` uses xdist (`-n 4`) by default, and pdb does NOT work under xdist. Add `-p no:xdist` or run a single test with `-n 0`:
Note: `scripts/run_tests.sh` runs each test file in a captured subprocess via `run_tests_parallel.py` (no xdist), so interactive pdb does NOT work under the wrapper. Run pytest directly for `--pdb`:

```bash
scripts/run_tests.sh tests/foo_test.py::test_bar --pdb -p no:xdist
# or
source .venv/bin/activate
python -m pytest tests/foo_test.py::test_bar --pdb
```
Expand Down Expand Up @@ -276,7 +274,7 @@ nc 127.0.0.1 4444
## Debugging Hermes-specific Processes

### Tests
See Recipe 3. Always add `-p no:xdist` or run single tests without xdist.
See Recipe 3. The wrapper captures subprocess output, so run pytest directly for interactive pdb.

### `run_agent.py` / CLI — one-shot
Easiest: add `breakpoint()` near the suspect line, then run `hermes` normally. Control returns to your terminal at the pause point.
Expand Down Expand Up @@ -308,7 +306,7 @@ Long-lived. Use `remote-pdb` at a handler, or `debugpy` with `--wait-for-client`

## Common Pitfalls

1. **pdb under pytest-xdist silently does nothing.** You won't see the prompt, the test just hangs. Always use `-p no:xdist` or `-n 0`.
1. **pdb under a parallel/output-capturing runner silently does nothing.** You won't see the prompt, the test just hangs (true of pytest-xdist and of `scripts/run_tests.sh`'s captured per-file subprocesses). Run pytest directly on a single file for interactive debugging.

2. **`breakpoint()` in CI / non-TTY contexts hangs the process.** Safe locally; never commit it. Add a pre-commit grep as a safety net.

Expand All @@ -333,7 +331,7 @@ Long-lived. Use `remote-pdb` at a handler, or `debugpy` with `--wait-for-client`

- [ ] After `pip install debugpy`, confirm: `python -c "import debugpy; print(debugpy.__version__)"`
- [ ] For remote debug, confirm the port is actually listening: `ss -tlnp | grep 5678`
- [ ] First breakpoint actually hits (if it doesn't, you likely have `PYTHONBREAKPOINT=0`, you're under xdist, or execution finished before attach)
- [ ] First breakpoint actually hits (if it doesn't, you likely have `PYTHONBREAKPOINT=0`, you're under a parallel/capturing runner, or execution finished before attach)
- [ ] `where` / `w` shows the expected call stack
- [ ] Post-debug cleanup: no stray `breakpoint()` / `set_trace()` in committed code
```bash
Expand All @@ -354,10 +352,10 @@ breakpoint()

**"This test passes in isolation but fails in the suite."**
```bash
scripts/run_tests.sh tests/the_test.py --pdb -p no:xdist
# But if it only fails WITH other tests:
scripts/run_tests.sh tests/the_test.py # confirm it fails under the isolated runner first
# For interactive debugging, or if it only fails WITH other tests:
source .venv/bin/activate
python -m pytest tests/ -x --pdb -p no:xdist
python -m pytest tests/ -x --pdb
# Now it pdb-traps at the exact failing test after state accumulated.
```

Expand Down
11 changes: 11 additions & 0 deletions tests/cli/test_cli_browser_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,17 @@ def test_linux_candidates_include_official_brave_and_edge_stable_paths(self):
assert candidates == [brave, edge]


def test_wsl_install_candidates_keep_posix_separators_on_nt_host(self):
expected = "/mnt/c/Program Files/Google/Chrome/Application/chrome.exe"

with patch("hermes_cli.browser_connect.shutil.which", return_value=None), \
patch("hermes_cli.browser_connect.os.path.isfile", side_effect=lambda path: path == expected):
candidates = get_chrome_debug_candidates("Linux")

assert candidates == [expected]
assert "\\" not in candidates[0]


def test_wait_for_browser_debug_ready_or_exit_detects_early_exit(self, monkeypatch):
class _Proc:
def __init__(self):
Expand Down
16 changes: 16 additions & 0 deletions tests/cli/test_cli_file_drop.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Tests for _detect_file_drop — file path detection that prevents
dragged/pasted absolute paths from being mistaken for slash commands."""

import os

import pytest

Expand Down Expand Up @@ -157,6 +158,8 @@ def test_tilde_prefixed_path(self, tmp_path, monkeypatch):
img.parent.mkdir(parents=True, exist_ok=True)
img.write_bytes(b"\x89PNG\r\n\x1a\n")
monkeypatch.setenv("HOME", str(home))
# ntpath.expanduser ignores HOME (Python 3.8+) — it wants USERPROFILE.
monkeypatch.setenv("USERPROFILE", str(home))

result = _detect_file_drop("~/storage/shared/Pictures/cat.png what is this?")

Expand All @@ -166,6 +169,19 @@ def test_tilde_prefixed_path(self, tmp_path, monkeypatch):
assert result["remainder"] == "what is this?"


@pytest.mark.skipif(os.name != "nt", reason="Windows drive-letter URI contract")
def test_windows_drive_letter_file_uri_drops_url_leading_slash(self, tmp_path):
image = tmp_path / "drive-uri.png"
image.write_bytes(b"\x89PNG\r\n\x1a\n")
uri = image.as_uri()
assert uri.startswith("file:///") and ":/" in uri

result = _detect_file_drop(uri)

assert result is not None
assert result["path"] == image


# ---------------------------------------------------------------------------
# Tests: edge cases
# ---------------------------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions tests/cli/test_cli_image_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ def test_collect_query_images_supports_tilde_paths(self, tmp_path, monkeypatch):
home = tmp_path / "home"
img = _make_image(home / "storage" / "shared" / "Pictures" / "cat.png")
monkeypatch.setenv("HOME", str(home))
# ntpath.expanduser ignores HOME (Python 3.8+) — it wants USERPROFILE.
monkeypatch.setenv("USERPROFILE", str(home))

message, images = _collect_query_images("describe this", "~/storage/shared/Pictures/cat.png")

Expand Down
15 changes: 15 additions & 0 deletions tests/cli/test_worktree.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,21 @@ def test_ten_concurrent_worktrees(self, git_repo):
assert not Path(info["path"]).exists()


def _can_symlink():
"""Check if we can create symlinks (needs admin/dev-mode on Windows)."""
import tempfile
try:
with tempfile.TemporaryDirectory() as d:
src = Path(d) / "src"
src.write_text("x")
lnk = Path(d) / "lnk"
lnk.symlink_to(src)
return True
except OSError:
return False


@pytest.mark.skipif(not _can_symlink(), reason="Symlinks need elevated privileges")
class TestWorktreeDirectorySymlink:
"""Test .worktreeinclude with directories (symlinked)."""

Expand Down
16 changes: 16 additions & 0 deletions tests/cli/test_worktree_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@
import pytest


def _can_symlink():
"""Check if we can create symlinks (needs admin/dev-mode on Windows)."""
import tempfile
try:
with tempfile.TemporaryDirectory() as d:
src = Path(d) / "src"
src.write_text("x")
lnk = Path(d) / "lnk"
lnk.symlink_to(src)
return True
except OSError:
return False


@pytest.fixture
def git_repo(tmp_path):
"""Create a temporary git repo for testing real cli._setup_worktree behavior."""
Expand Down Expand Up @@ -76,6 +90,7 @@ def test_rejects_parent_directory_directory_traversal(self, git_repo):
finally:
_force_remove_worktree(info)

@pytest.mark.skipif(not _can_symlink(), reason="Symlinks need elevated privileges")
def test_rejects_symlink_that_resolves_outside_repo(self, git_repo):
import cli as cli_mod

Expand Down Expand Up @@ -110,6 +125,7 @@ def test_allows_valid_file_include(self, git_repo):
finally:
_force_remove_worktree(info)

@pytest.mark.skipif(not _can_symlink(), reason="Symlinks need elevated privileges")
def test_allows_valid_directory_include(self, git_repo):
import cli as cli_mod

Expand Down
9 changes: 9 additions & 0 deletions tests/gateway/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,15 @@ def test_runtime_status_running_pid_accepts_matching_profile_cmdline(self, monke
), cmdline


def test_command_line_belongs_to_profile_normalizes_separators(self):
"""A Windows argv renders HERMES_HOME with backslashes while the
profile's Path may carry forward slashes (and, on Windows, vice
versa). The separator difference must not defeat the match."""
home = Path("c:/opt/data/profiles/coder")
cmdline = r"hermes_home=c:\opt\data\profiles\coder hermes gateway run --replace"
assert status._command_line_belongs_to_profile(cmdline, home) is True


def test_write_runtime_status_explicit_none_clears_stale_fields(self, tmp_path, monkeypatch):
monkeypatch.setenv("HERMES_HOME", str(tmp_path))

Expand Down
Loading
Loading