fix(bot-relay): Windows path SyntaxError in waiter + PATH-less delivery ENOENT - #93601
Closed
liuhao1024 wants to merge 2 commits into
Closed
liuhao1024 wants to merge 2 commits into
liuhao1024 wants to merge 2 commits into
Conversation
…ry ENOENT Two failures on a Windows desktop install relaying to a remote gateway (NousResearch#93590): 1. waiter_command embeds the reply path in generated python -c source with !r. repr escapes each backslash, but the Windows execution layer folds \\ back to \, so \U in C:\Users\... parses as a unicode escape and SyntaxErrors the whole waiter script. Raw-string literals keep the folded single backslash a literal; POSIX paths have no backslashes so the prefix is a no-op there, and \' inside a raw literal still cannot terminate the string, keeping the NousResearch#93091 injection defense intact. 2. local_delivery_command hardcoded "hermes", relying on PATH — absent in service contexts (systemd units, desktop launchers, non-login SSH shells), so delivery died with ENOENT. It now resolves the CLI next to this gateway's own interpreter (venv bin/Scripts sibling, hermes.exe on Windows) with a bare-name fallback. The NousResearch#93091 per-profile turn-lock recognition in bot_mode_dm now matches the CLI element by basename (split on both separators) so resolved absolute paths still take the lock instead of silently bypassing it. Fixes NousResearch#93590
CI runners have a real hermes sibling next to the venv python, so local_delivery_command now resolves an absolute path there — the exact argv filters in the retry-policy fakes and the relay-methods pins must match by basename instead of the literal "hermes", mirroring the _delivery_lock matcher.
alt-glitch
added
type/bug
Related: #93597 addresses the same Windows #93590 path/entrypoint failure with a narrower diff. This PR also covers the generated label literal, hermes.exe/basename lock behavior, fallback handling, and regression coverage; maintainers should choose or consolidate the approach. |
teknium1
pushed a commit
that referenced
this pull request
Aug 24, 2026
… decoding on delivery subprocess Salvage hardening on top of #93601 (with #93597 covering the same core mechanisms) for #93590: - _hermes_cli(): after the venv-sibling check (hermes.exe on win32), try shutil.which('hermes') before the bare-name fallback, so environments with a PATH but no venv sibling resolve exactly what an interactive shell would. Platform test switched os.name -> sys.platform ('win32') per repo convention. - tui_gateway/methods_bot_relay.py deliver: pin encoding='utf-8', errors='replace' on both subprocess.run sites — without them the child's UTF-8 output is decoded with the locale codec (cp1252/GBK on Windows), mangling non-ASCII replies or raising on undecodable bytes. - Regression tests: shutil.which resolution step, bare-name fallback with which=None, and encoding-pin assertions in the deliver transport test. Refs #93590, #93597, #93601
This was referenced Aug 24, 2026
Collaborator
|
Merged via #93658 with both your commits cherry-picked (authorship preserved); we added a shutil.which step and utf-8 pinning on the delivery subprocesses. @jdtimothy's #93597 credited as earliest diagnosis. Thanks @liuhao1024! |
Author
|
Thanks for the merge and for the note! Good additions — the |
and7777
pushed a commit
to and7777/hermes-agent
that referenced
this pull request
Aug 27, 2026
… decoding on delivery subprocess Salvage hardening on top of NousResearch#93601 (with NousResearch#93597 covering the same core mechanisms) for NousResearch#93590: - _hermes_cli(): after the venv-sibling check (hermes.exe on win32), try shutil.which('hermes') before the bare-name fallback, so environments with a PATH but no venv sibling resolve exactly what an interactive shell would. Platform test switched os.name -> sys.platform ('win32') per repo convention. - tui_gateway/methods_bot_relay.py deliver: pin encoding='utf-8', errors='replace' on both subprocess.run sites — without them the child's UTF-8 output is decoded with the locale codec (cp1252/GBK on Windows), mangling non-ASCII replies or raising on undecodable bytes. - Regression tests: shutil.which resolution step, bare-name fallback with which=None, and encoding-pin assertions in the deliver transport test. Refs NousResearch#93590, NousResearch#93597, NousResearch#93601
melon-xf
added a commit
to melon-xf/hermes-agent
that referenced
this pull request
Sep 3, 2026
… decoding on delivery subprocess Salvage hardening on top of NousResearch#93601 (with NousResearch#93597 covering the same core mechanisms) for NousResearch#93590: - _hermes_cli(): after the venv-sibling check (hermes.exe on win32), try shutil.which('hermes') before the bare-name fallback, so environments with a PATH but no venv sibling resolve exactly what an interactive shell would. Platform test switched os.name -> sys.platform ('win32') per repo convention. - tui_gateway/methods_bot_relay.py deliver: pin encoding='utf-8', errors='replace' on both subprocess.run sites — without them the child's UTF-8 output is decoded with the locale codec (cp1252/GBK on Windows), mangling non-ASCII replies or raising on undecodable bytes. - Regression tests: shutil.which resolution step, bare-name fallback with which=None, and encoding-pin assertions in the deliver transport test. Refs NousResearch#93590, NousResearch#93597, NousResearch#93601
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
3 participants
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 does this PR do?
Fixes both
message_agentrelay failures from #93590 on a Windows desktop install talking to a remote gateway:waiter_commandembeds the reply path into generatedpython -csource with!r. repr escapes each backslash (C:\\Users\\...), but the Windows execution layer the waiter runs under folds\\back to\, so\Uparses as an invalid unicode escape and SyntaxErrors the whole script. The two string literals in the generated source now carry a raw-string prefix: the folded single backslash parses as a literal.local_delivery_commandhardcoded"hermes", relying on PATH — which service contexts (systemd units, desktop launchers, non-login SSH shells) do not provide, so the delivery subprocess died withNo such file or directory: 'hermes'. It now resolves the CLI next to this gateway's own interpreter (the venvbin/Scriptssibling —hermes.exeon Windows), falling back to the bare name when no sibling exists.Related Issue
Fixes #93590
Type of Change
Changes Made
tools/bot_relay.py:waiter_command:p = r{reply_path!r}andlabel = r{label!r}(both string injection points of the same mechanism; comment documents the fold, the POSIX no-op, and that\'inside a raw literal still cannot terminate the string — the Bot Mode reliability program: typed failure reasons, envelope TTL, attention badges, leader-routed group rooms, retry session policy #93091 injection defense is unchanged)._hermes_cli(): resolves the hermes entrypoint besidesys.executable(the target gateway's own venv interpreter — the deliver RPC runs on THIS gateway per the existing docstring),hermes.exesuffix on Windows, bare-name fallback preserving PATH lookup for source-tree runs.local_delivery_command: first element is_hermes_cli(); the rest of the argv is untouched.tools/bot_mode_dm.py:_delivery_locknow matches the CLI element by basename (split on both/and\), so resolved absolute paths — andhermes.exe— still take the Bot Mode reliability program: typed failure reasons, envelope TTL, attention badges, leader-routed group rooms, retry session policy #93091 per-profile turn lock instead of silently bypassing it. Without this, fix 2 would have re-opened the concurrent-turn race the lock exists to prevent.tests/tools/test_bot_relay_windows_paths.py— new: Windows path compiles after simulated backslash folding (the exact\Ucrash); POSIX path/label values round-trip unchanged with the raw prefix pinned; injection defense holds under the raw prefix (hostile connection id stays data); sibling resolution + bare-name fallback; turn-lock recognition for resolved paths, bare names, andhermes.exe, with unrelated argvs still bypassing.tests/tools/test_bot_turn_lock.py: the argv-shape pin goes by basename now (argv[1:3] == ["-p", <profile>]+ CLI basename), same intent, robust to the resolved-path form.How to Test
.venv/bin/python -m pytest tests/tools/test_bot_relay_windows_paths.py -q— should pass (6 tests). On unpatchedmain, 4 of the 6 fail (no raw literals → folding SyntaxError; no sibling resolution; lock rejects resolved paths), pinning both regressions..venv/bin/python -m pytest tests/tools/test_bot_relay.py tests/tools/test_bot_turn_lock.py tests/tools/test_bot_mode_dm.py tests/tui_gateway/test_bot_relay_methods.py -q— should pass (85 tests), including the existing hostile-connection-id injection test against the new raw-prefixed literals.Observed result: locally, all 6 new tests pass and the 85 existing bot-relay/turn-lock/bot-mode tests stay green — the raw prefix is a no-op for POSIX paths, and the turn lock keeps matching every hermes-CLI delivery shape.
Checklist
upstream/mainfrom a fork branch