mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
fix: extend UTF-8 encoding to _op_version probe (#53428)
Hermes-sweeper review on #60741 flagged that _op_version (the paired op probe used by the same setup/status CLI flow at lines 127 and 205) still ran text=True without explicit encoding/errors. Add encoding='utf-8', errors='replace' to match _op_whoami and the production op read path at agent/secret_sources/onepassword.py:271-278. Also extend the regression test to cover _op_version alongside _op_whoami, and update the module docstring to reflect the widened scope. Test sensitivity verified: reverting the source change makes test_op_version_passes_utf8_encoding fail with encoding=None.
This commit is contained in:
parent
a23115414a
commit
db66119676
2 changed files with 21 additions and 3 deletions
|
|
@ -487,6 +487,8 @@ def _op_version(binary: Path) -> str:
|
|||
[str(binary), "--version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
timeout=5,
|
||||
)
|
||||
if res.returncode == 0:
|
||||
|
|
|
|||
|
|
@ -4,9 +4,9 @@ explicit ``encoding=`` triggers ``UnicodeDecodeError`` on Chinese Windows
|
|||
|
||||
PR #55339 covers 21 call sites in ``agent/``, ``gateway/``, ``cli.py``,
|
||||
``cron/``, plus 5 more in ``tools/`` and ``hermes_cli/`` (main.py,
|
||||
setup.py, tts_tool.py, transcription_tools.py). The one call site it
|
||||
misses — ``hermes_cli/onepassword_secrets_cli.py::_op_whoami`` — is
|
||||
guarded here.
|
||||
setup.py, tts_tool.py, transcription_tools.py). The two call sites it
|
||||
misses — ``hermes_cli/onepassword_secrets_cli.py::_op_whoami`` and
|
||||
``_op_version`` — are guarded here.
|
||||
|
||||
Without ``encoding=``, ``text=True`` decodes child output with
|
||||
``locale.getpreferredencoding(False)`` — cp936 on Chinese Windows —
|
||||
|
|
@ -50,3 +50,19 @@ def test_op_whoami_passes_utf8_encoding(tmp_path):
|
|||
)
|
||||
op_cli._op_whoami(fake_binary, account="")
|
||||
_assert_utf8_kwargs(mock_run)
|
||||
|
||||
|
||||
def test_op_version_passes_utf8_encoding(tmp_path):
|
||||
"""_op_version must pass encoding='utf-8', errors='replace' so op CLI
|
||||
output containing non-ASCII bytes doesn't crash on cp936. Pairs with
|
||||
_op_whoami — both run in the same setup/status CLI flow."""
|
||||
from hermes_cli import onepassword_secrets_cli as op_cli
|
||||
|
||||
fake_binary = tmp_path / "op"
|
||||
fake_binary.write_bytes(b"")
|
||||
with patch.object(op_cli.subprocess, "run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0, stdout="2.24.0", stderr=""
|
||||
)
|
||||
op_cli._op_version(fake_binary)
|
||||
_assert_utf8_kwargs(mock_run)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue