* fix(model-picker): serve cached custom-provider catalog on no-probe opens
#58183 stopped GUI picker opens from live-probing saved custom
OpenAI-compatible endpoints so a stopped local server could not stall the
picker. It gated the whole discovery block, not just the network call, so
`cached_fetch_api_models()` was skipped too — and with it the catalog an
earlier probe had already written to `provider_models_cache.json`.
A custom endpoint that is not the current provider therefore renders only
the models named in its config entry. A local server with 8 models loaded
shows the 1 model that was saved when the provider was first added, on
every picker open, while an explicit Refresh shows all 8.
Add `cache_only` to `cached_fetch_api_models()`: answer from disk within
the existing stale-serve window, never fetch, never revalidate off-thread,
return None on a miss. Split the three call sites in
`list_authenticated_providers()` into what the user's config permits
(`discover_models`, an explicit `models:` allowlist) and how we may obtain
it, so suppressing the probe now downgrades to a cached read instead of
skipping discovery outright. `discover_models: false` still pins, and a
cache hit no longer writes back to config since the probe that populated
it already did.
The latency win stands: a cold cache is a miss, so picker opens against
offline endpoints still make zero network calls.
* test(model-picker): pin the cached-catalog contract for no-probe opens
Cover both halves of the invariant, since fixing either one alone
reintroduces a bug the other guards against.
`cache_only` on `cached_fetch_api_models()`: a fresh entry and an entry
past its TTL but inside the stale-serve window both serve; an entry beyond
that window, an empty cache, rotated credentials, `force_refresh`, and a
missing base_url are all misses — and none of them fetch or spawn a
background revalidation.
`list_authenticated_providers()` on the GUI path: a non-current endpoint
with a warm cache reports its full catalog across all three provider
shapes (`custom_providers`, `providers:`, bare `provider: custom`) with no
live fetch attempted. A cold cache keeps the configured list and still
makes no network call, which is the #58183 guarantee. `discover_models:
false` keeps pinning, and a cache hit does not write back to config.
* fix: persist discovered custom-provider models in the hermes model flow
The `hermes model` named-custom-provider flow (_model_flow_named_custom)
probes the endpoint and shows the full catalog, but never persists it to the
entry's `models:` list. No-probe surfaces (dashboard, desktop, ACP) call
build_models_payload(..., probe_custom_providers=False) and only render the
configured `models:` list, so a provider added via `hermes model` collapses
to the single `model:` default everywhere except the CLI. OpenAI-compatible
providers added via a probing picker already benefit from
_save_discovered_models_to_config; the CLI flow did not.
Persist the live catalog after a successful probe, mirroring the picker path
in model_switch.py. A failed save is non-fatal.
* fix(model-picker): stop an auto-saved catalog pinning a keyless endpoint
The cached-catalog read added for no-probe picker opens still sat behind
the no-key discovery gate, so it never reached the shape that motivated
it: a keyless local model server.
`bool(api_key) or not has_explicit_models` is a network-cost gate. It
exists so Hermes does not probe an endpoint it cannot authenticate to
when that endpoint already declares its catalog (5f00f36ba, 1039e90b5).
Reading a catalog an earlier probe already paid for costs nothing, so
the gate belongs on the probe, not on discovery as a whole.
Left on the discovery side it re-pins the endpoint it was meant to
spare. A successful probe calls `_save_discovered_models_to_config()`,
which writes a plain list into `models:` — exactly the shape
`_models_config_is_allowlist()` reads back as an explicit user
allowlist. A keyless server therefore froze on the catalog of its first
probe and could never widen again, which is the "lineup changes after
config was written" case. f66319097 already carved the dict shape out of
this trap for the same reason; the list shape is the other door into it.
Move the clause to `_probe_live` at both custom-endpoint sites. Probe
suppression is unchanged — verified byte-identical to main across the
keyed/keyless x declared/undeclared matrix — and `discover_models: false`
remains the documented way to pin a catalog.
* test(model-picker): cover the keyless auto-save pinning trap
Three tests around the gate move, each failing on the code before it:
- a keyless endpoint carrying an auto-saved `models:` list still reads
its full cached catalog
- the same row, cold cache and probing enabled, still makes zero live
fetches — the network-cost gate the clause exists for
- an end-to-end round trip: persist a probe result via
`_save_discovered_models_to_config()`, reload it, and assert the shape
we wrote does not read back as a user pin
The round-trip test guards the whole chain rather than one branch, so a
future change that makes the saved shape look like an intentional
allowlist fails here even if the gate logic is refactored.
* fix(model-picker): key the custom-endpoint model cache by api_mode
`cached_fetch_api_models()` fingerprints entries with `api_mode`, but no
call site in `list_authenticated_providers()` passed it, so every custom
row resolved to the `api_mode=None` fingerprint. Two rows sharing a
base_url and credential but differing by `api_mode` are deliberately
distinct picker rows — it is part of `group_key` at both sites — yet they
collapsed onto one cache entry.
That was latent while probing was the only way to fill a row: a mismatched
entry was overwritten by the row's own live fetch. Serving that entry
without a probe makes it visible, so an `anthropic_messages` row could
render the catalog an OpenAI-mode row cached against the same URL. The
wire protocols differ (`x-api-key` + `anthropic-version` vs
`Authorization: Bearer`), so those catalogs are not interchangeable.
Persist `api_mode` on the group at both grouping sites — it is already
part of `group_key`, so it is constant across the group — and pass it
into the cache read. Section 3b (bare `provider: custom`) has no
`api_mode` in scope and already reads with the empty-credential
fingerprint, so it is unchanged.
Reported by Copilot review on #81973.
---------
Co-authored-by: xxxigm <tuancanhnguyen706@gmail.com>
Co-authored-by: Navlem <114683850+Navlem@users.noreply.github.com>
* fix(dashboard): retry stalled events feed reconnects
* fix(dashboard): bound the PTY ticket request before the socket exists
ChatPage's connect awaits a single-use ticket from `api.buildWsUrl()`
before `new WebSocket()`. That request produces no socket, so a
rejection or a hang emits no `close` event and never arms
PTY_CONNECTING_TIMEOUT_MS (set after the socket is constructed). The
tab stranded on "connecting" with `connectInFlightRef` stuck true,
which also suppresses the page-resume reconnect path.
Give the ticket phase its own deadline and route both failure modes
into the existing backoff. A `ticketSuperseded` flag invalidates a late
ticket result so a timed-out attempt cannot open a socket behind the
replacement it scheduled, and cleanup clears the timer on unmount.
`scheduleReconnect` now takes `number | null` so an attempt that died
before any socket existed omits the "(code N)" banner suffix instead of
inventing one.
Same bug class as the events-feed fix in the preceding commit, on the
main chat surface.
Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
* test(dashboard): cover the PTY ticket connect deadline
Mirrors the events-feed cases in ChatSidebar.test.tsx: a rejected ticket
retries, a stalled ticket times out and its late resolution cannot open
a superseded socket, and a settled ticket disarms the deadline so
PTY_CONNECTING_TIMEOUT_MS remains the only guard on a wedged handshake
(NS-591 regression).
Both failure cases fail against ChatPage.tsx without the preceding fix.
Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
---------
Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
The Windows-aware path matching added for project ownership was a second
copy of what the file tree's IPC layer already had — same Windows test, same
containment check, one of them carrying a trailing-slash branch its own
normalisation made unreachable. Both now share lib/path-compare.
On a remote backend a new chat starts in the remembered workspace, and
setCurrentCwd persisted that key on every call — including the six paths
that merely follow a conversation (resume settling, warm switch, stored-row
preview, agent relocation, boot seed, resolved new-chat default). So opening
a chat inside a project quietly made that project the destination for the
next "New session", which is the half of the report the resolver fix does
not reach: a Windows desktop driving a WSL gateway is a remote connection.
setCurrentCwdTransient already meant "move the path, claim nothing" — the
following paths now use it, and setCurrentCwd is reserved for a workspace
the user actually named.
Project ownership compared paths literally, so a nested cwd failed to match
its project whenever the separator or drive-letter case differed — which on
Windows is routine. Normalise both sides for comparison only, folding case
for drive and UNC paths.
resolveNewSessionCwd() inherited the focused chat's workspace, so every
"New session" landed in whatever project you were last looking at — and
after a restart the focused session's stored cwd is often a home-dir
fallback, which shadowed the configured default project dir entirely.
The boot seed had a second failure mode: ensureDefaultWorkspaceCwd() only
seeds while no session is active, and it ran after gateway.connect() — the
same event that un-gates route-resume. On a slow start the resume won and
the seed silently skipped. Seed before connect instead, where no session
can be active yet, and keep both seeds non-fatal.
Widen of the PowerShell codepage cluster: the autostart registration
subprocess in tools_config.py was the last text=True capture in these
modules still decoding with the locale code page. Standardize on
encoding='utf-8', errors='replace' like the rest of the file (#53428).
A gate runs whatever command the operator configured, so its output is
arbitrary bytes. run_gate captured it with text=True and no encoding, which
decodes with locale.getpreferredencoding() under errors="strict".
One byte the decoder rejects — a test runner's checkmarks or CJK on a
non-UTF-8 Windows console, a stray binary byte anywhere in the stream — kills
subprocess's reader thread. proc.stdout comes back None, the `or ""` fallback
turns that into an empty tail, and an unhandled traceback is dumped to stderr.
The gate's pass/fail verdict still lands on the exit code, but the output tail
is exactly what the retry prompt feeds back so the agent can fix the failure.
With it empty the agent is told a gate failed and given nothing to act on, so
it burns every retry and the goal auto-pauses.
workspace_fingerprint has the same two calls; there a non-ASCII path in
`git status --porcelain` empties the fingerprint, silently disabling the
unchanged-gate skip that exists to stop a stalled agent re-running the same
red suite.
Decode as UTF-8 with errors="replace" — what git and modern toolchains emit,
and what 262 of the repo's 299 text-mode subprocess calls already do.
_run() used text=True without an encoding, so Windows decoded git's
UTF-8 output with the locale code page (cp932) and raised
UnicodeDecodeError on non-ASCII filenames or diff content, breaking
the "Never raises on git failure" contract in its docstring. Match
the utf-8 + errors="replace" policy checkpoint_manager's _run_git
already uses. Legacy cp932-encoded blob content degrades to
replacement characters instead of crashing; a test pins that
trade-off so it stays a documented choice.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PowerShell's Get-Clipboard -Raw outputs text in the system's ANSI codepage
(e.g. CP1252, CP936), not UTF-8. When Node.js reads this with encoding: 'utf8',
non-ASCII characters (CJK, emoji, accented chars) are corrupted.
The write path already solved this by base64-encoding UTF-8 bytes and passing
them via -Command argument (see comment at line 94-98). This fix applies the
same approach to the read path:
- Change PowerShell read command to base64-encode the clipboard content
using [Convert]::ToBase64String([System.Text.Encoding]::UTF8.GetBytes(...))
- Add base64 flag to read command type for PowerShell entries
- Decode base64 result in readClipboardText when flag is set
Also adds child.unref() to spawned clipboard child processes in the write
path to prevent delaying process.exit() when the app exits mid-clipboard-write.
Fixes the read_file half of #80308 and the class behind #80261, #80250,
The binary sniff sampled files via 'head -c 1000' through the terminal
transport, which decodes stdout with errors="replace". A multibyte
character cut at byte 1000 therefore arrived as U+FFFD, and
_is_likely_binary treated any U+FFFD as binary — flagging valid CJK and
emoji text as unreadable. At the text layer a stored replacement char
and a transport-manufactured one are indistinguishable, which is why
per-callsite adjustments kept leaving siblings open.
Sample as 'head -c 1000 | base64' so raw bytes survive the transport
(fail-open to the legacy heuristic when the transport cannot produce
clean base64), then classify bytes: NUL => binary; valid UTF-8 allowing
one incomplete multibyte sequence at the sample end => text; mid-stream
invalid UTF-8 (latin-1, true binaries) => read-only, preserving the
anti-mojibake guarantee the old check existed for. Files legitimately
containing U+FFFD become readable.
Three desktop UI tests froze en-US-formatted strings while the
implementation formatters deliberately use the runtime locale
(new Intl.DateTimeFormat(undefined, ...) / Intl.NumberFormat(undefined,
...)) — runtime-locale output is the intended behavior for a localized
UI. On any non-en-US dev machine the tests fail even though the code is
correct:
# zh-CN host:
time.test.ts -> expected '三月' to be 'March'
billing -> Unable to find text 'Threshold: minimum is $10.'
(zh-CN renders USD as 'US$10')
billing -> Unable to find text '$25 added. Balance is refreshing.'
Assert the behavior contract instead of the frozen snapshot, per the
repo's testing guidance (behavior contracts over snapshots):
- time.test.ts: same-year month buckets render via fmtMonth, prior-year
via fmtMonthYear — assert sessionBucketLabel(bucket) equals the shared
formatter's output for bucket.at, with bucket-kind narrowing.
- billing/index.test.tsx: interpolate formatMoney(10) / formatMoney(25)
into the expected strings.
No production code changes.
Verified: zh-CN host 40/40, LANG=C.UTF-8 40/40, tsc clean, eslint clean.
test_cli_entrypoint_end_to_end copies add_contributor.py with
read_text()/write_text() and no encoding argument, so both fall back to
the system locale. add_contributor.py contains UTF-8 multi-byte
characters (an em dash), which makes the read raise UnicodeDecodeError
on any non-UTF-8 Windows locale (observed on cp950 / Traditional
Chinese). The trailing mapping-file read gets the same treatment for
symmetry.
Same footgun class as the subprocess text=True sweep in #60741, just on
the pathlib read_text/write_text side.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Two Windows bugs in scripts/run_tests_parallel.py:
- --files/--paths/HERMES_TEST_PATHS were split on ':', which shreds
absolute Windows paths at the drive letter ('C:\repo\tests' ->
['C', '\repo\tests']): the drive letter became a phantom discovery
root and the rooted remainder only resolved by WindowsPath
re-anchoring it onto repo_root's drive. New _split_pathspec() keeps
drive-letter colons glued to their path and accepts ';' (os.pathsep)
on Windows, while ':'-joined lists (CI generate job) keep working.
- With piped stdout (CI, subprocess capture) Windows encodes the
runner's output as the ANSI code page, so printing the per-file
progress glyphs raised UnicodeEncodeError inside the executor
done-callback and every progress line was silently lost -- which is
also why test_bare_value_flag_keeps_its_value failed on win32 (no
'1[check]' line, and the summary says '1 tests passed', which does not
contain '1 passed'). The runner now reconfigures its own
stdout/stderr to UTF-8 on Windows, and the tests decode the captured
output as UTF-8.
Adds regression tests: os.pathsep-joined absolute roots (all
platforms) and no-phantom-drive-root (win32).
Fixes#57149
`tests/hermes_cli/test_plugins_cmd.py::TestNoAutoActivation::test_compressor_default_ignores_plugin`
fails on every Windows machine:
UnicodeDecodeError: 'charmap' codec can't decode byte 0x8f in
position 47744: character maps to <undefined>
The test reads `run_agent.py` back as text to assert a removed comment is
gone, but called `open()` with no `encoding=`. Python then falls back to
the locale preferred encoding, which is cp1252 on a default Windows
install rather than UTF-8. `run_agent.py` contains nine bytes cp1252
leaves undefined, so the read raises before the assertion is reached. On
Linux and macOS the preferred encoding is UTF-8 and the same line is
fine, which is why CI never caught it.
That one line is the only active failure. The rest of this change closes
the same gap in the files it touches, which `scripts/check-windows-footguns.py`
flags and which the #71014 read_text campaign has been working through
elsewhere in the tree:
- `tests/hermes_cli/test_plugins_cmd.py`: nine bare `write_text`/`read_text`
calls writing YAML manifests, config and plugin sources
- `tests/tools/test_web_tools_truncate.py`: reads stored extracted web text,
which is arbitrary content from the internet
- `tests/stress/test_atypical_scenarios.py`: writes and reads worker task
ids and a barrier file
All three files are now clean under `check-windows-footguns.py`.
Reads go through `Path.read_text(encoding="utf-8")` rather than
`open(...).read()`, which also closes the handle instead of leaving it to
the garbage collector. On Windows a live handle blocks tmpdir cleanup, so
that part is not cosmetic either.
No new test. The repaired test is the regression coverage: it fails
before this change and passes after, on Windows.
ruff PLW1514 (already enforced repo-wide via the blocking lint step)
covers open()/Path.open()/read_text()/write_text() but NOT os.fdopen —
the exact hole the AlexFucuson9 sweep PRs (#56033#56940#65565) kept
patching by hand. Add an fdopen rule to check-windows-footguns.py, which
also runs as a blocking CI step, so a bare text-mode fdopen fails CI.
Also fix a false-negative in the read_text/write_text rule: chained
forms like `read_text()[:4000]` or `read_text().splitlines()` never end
the line with `)` and slipped past the multi-line-call heuristic.
Replace the endswith check with a paren-balance walk (keeps multi-line
calls with encoding= on a continuation line unflagged — verified against
the full tree). This makes the rule the effective standing replacement
for the standalone checker proposed in PR #66669: R1-style coverage now
lives in PLW1514 + this script, both blocking in .github/workflows/lint.yml.
Sabotage-verified: reverting agent/shell_hooks.py's fdopen encoding or
tools/skills_tool.py's read_text encoding now fails the gate.
Co-authored-by: AlexFucuson9 <AlexFucuson9@users.noreply.github.com>
Co-authored-by: Paulo Nascimento <pnascimento9596@gmail.com>
Complements the cherry-picked contributor fixes and closes out the
remaining sites of the 'missing explicit encoding' bug class, which is
now permanently gated by ruff PLW1514 (enabled repo-wide in
pyproject.toml and enforced by the blocking `ruff check .` step in
.github/workflows/lint.yml):
- tools/memory_tool.py: read MEMORY.md/USER.md via utf-8-sig so a
Notepad BOM never glues U+FEFF onto the first entry (issue #10878,
PR #10888 by @easyvibecoding — strict-decode contract of
_read_raw_checked preserved rather than errors="replace", so
undecodable files still refuse read-modify-write instead of being
lossily rewritten). Regression tests included.
- tools/skills_tool.py: SKILL.md and skill file reads pinned to
utf-8-sig + errors="replace" — deterministic across platforms instead
of the locale fallback proposed in PR #51701 (superseded: falling back
to cp1252/GBK makes the same skill render differently per host); .env
reader aligned with the canonical utf-8-sig dialect in hermes_cli/config.py.
- agent/shell_hooks.py, hermes_cli/main.py, gateway/slash_commands.py:
explicit utf-8 on the remaining fdopen/open text-mode sites flagged by
the AlexFucuson9 sweep series (#56033#56940#65565#66782#66791).
Co-authored-by: easyvibecoding <easyvibecoding@users.noreply.github.com>
Co-authored-by: AlexFucuson9 <AlexFucuson9@users.noreply.github.com>
Co-authored-by: flyingdoubleg <wangzhe00zju@gmail.com>
Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
_remove_env_source() decides whether a credential var lives in ~/.hermes/.env
or the shell by scanning the .env with env_path.read_text(errors="replace") —
no encoding. read_text() with no encoding falls back to the system locale
(cp1252/GBK on Windows) and never strips a BOM.
The canonical .env readers in hermes_cli/config.py all use
encoding="utf-8-sig" precisely because 'users may edit .env in Notepad which
adds one' (a BOM), and doctor.py documents that .env is written as UTF-8
everywhere. This sibling reader diverged: on a Notepad-edited .env the BOM
prefixes the first line, so line.strip().startswith(f"{env_var}=") is False
for the first variable — the detector reports a .env-backed key as a phantom
shell export and prints a misleading 'still set in your shell environment'
hint on .
Match the canonical reader (utf-8-sig + errors=replace). Adds a regression
test with a BOM'd .env.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review asked for a BOM-prefixed JSON case alongside the existing
control-character coverage. The sandbox script now also feeds
json_parse a \ufeff-prefixed document and asserts the parsed value
round-trips (fails against the pre-fix helper, passes with the
BOM strip).
json_parse used json.loads(strict=False), which relaxes control
characters but rejects a leading UTF-8 BOM (U+FEFF). Windows CLI
tools and some files prepend a BOM, causing JSONDecodeError on
otherwise valid JSON output.
Strip a leading BOM before calling json.loads when the input is
a string with a U+FEFF prefix.
Original PR by @woxinwuhen713-bit (#57870).
Co-Authored-By: Claude <noreply@anthropic.com>
tools/managed_tool_gateway._read_nous_provider_state read auth.json with a
bare read_text(), which on Windows decodes as cp1252 and raises on any
non-ASCII byte (e.g. an accented Nous provider label). The broad except
swallowed it and returned None, so the gateway treated Nous as
unconfigured — the same Windows UTF-8 hazard the other auth.json readers
in this PR already fix.
Add encoding="utf-8-sig" (consistent with the sibling readers) plus a
non-ASCII regression test reusing the windows_default_encoding fixture.
This covers the one auth.json reader in #66782 not already handled here
(tools/managed_tool_gateway.py:40); the other two readers #66782 touches
(agent/auxiliary_client.py, tools/xai_http.py) are already fixed in this
PR. RED-verified.
Address review feedback on #58158: the regression suite covered four of
the changed readers but not _read_shared_nous_state (auth.py) or
_has_any_provider_configured (main.py), which also read UTF-8 stores the
Windows cp1252 default can corrupt.
Add a non-ASCII UTF-8 regression case for each, reusing the existing
windows_default_encoding fixture and _write_utf8 helper:
- _read_shared_nous_state: a nous_auth.json with an accented display_name
and valid tokens must round-trip intact (not return None). Pins
HERMES_SHARED_AUTH_DIR to tmp to satisfy the shared-store seat belt.
- _has_any_provider_configured: an auth.json whose active provider carries
a CJK label must still report a configured provider (the read must not
raise into the swallowing except). get_auth_status is faked so the
result is driven by the read, and provider env vars are cleared to reach
the auth.json branch.
Both tests are RED-verified — they fail when the respective
read_text(encoding=...) is reverted.
Follow-up to the auth.json UTF-8 read fix in this PR. A repo-wide scan for
the same bug class found three more callers that read ~/.hermes/auth.json
via Path.read_text() with no encoding — same Windows cp1252 hazard:
- agent/auxiliary_client.py _read_nous_auth: a non-ASCII byte raised
UnicodeDecodeError, the broad except swallowed it, and Nous silently
stopped being available as the auxiliary (vision/summarization) provider.
- tools/xai_http.py has_xai_credentials: same failure mode — xAI OAuth
silently looked absent on Windows.
- hermes_cli/main.py is_setup_complete: same; has a config.yaml fallback so
the impact is milder, but the read is still wrong.
All three now use read_text(encoding="utf-8-sig"), matching _save_auth_store's
write encoding. A repo-wide grep confirms there are no remaining
json.loads(...read_text()) reads of auth.json without an explicit encoding.
Tests: rewrote the Windows-encoding regression tests to actually exercise the
bug on POSIX too — a new windows_default_encoding fixture forces a no-encoding
read_text() to decode as cp1252 (the Windows default), and _write_utf8 now
emits real non-ASCII UTF-8 bytes (ensure_ascii=False) so the bytes actually
trip cp1252. Verified each test fails when its fix is reverted (including
the two new sibling-reader tests).
The auth store readers (_load_auth_store, _import_codex_cli_tokens, and the
shared Nous store reader) called Path.read_text() with no encoding, so bytes
were decoded with locale.getpreferredencoding() — cp1252 on Windows. The
stores are *written* as UTF-8 (os.fdopen(..., encoding="utf-8")), so any
non-ASCII byte (a CJK or emoji credential label, an accented display name in
OAuth state) raised UnicodeDecodeError on read.
Worst case: _load_auth_store's broad except then copied the file to .corrupt
and returned an empty store, silently wiping every provider credential on the
next launch. The sibling reader at line 2161 already used
read_text(encoding="utf-8"), confirming the omission was unintentional.
Use utf-8-sig (matching the .env handling in config.py) so a BOM from a
Notepad-edited file is tolerated too.
Adds regression tests covering the UTF-8 round-trip with a non-ASCII label,
BOM tolerance, no-corrupt-on-valid-load, and that the readers pass an explicit
encoding (guard against future regressions). Verified the tests fail when the
fix is reverted.
Closes no issue — found via cross-platform code audit (the bug is not in the
issue tracker).
send_cmd._load_hermes_env intentionally reimplements a minimal dotenv
load (no secret-source pulls, no sanitize rewrite, get_hermes_home path
resolution incl. Windows/profile override), so the shared-loader BOM fix
is mirrored in place: utf-8-sig primary read, BOM strip before the
latin-1 stream fallback.
Claude-Session: https://claude.ai/code/session_01JPmJz5u1Bvtw4cCRvRWnYr
utf-8-sig only covers the primary decode. BOM + invalid UTF-8 (e.g.
PowerShell BOM + cp1252 body) forced latin-1, which kept EF BB BF as
part of the first key name and dropped the canonical name. Strip the
BOM before latin-1 decode and load via stream so override= is preserved.
PowerShell 5.1 Set-Content -Encoding UTF8 and Windows Notepad write a
UTF-8 BOM. load_dotenv(encoding="utf-8") kept U+FEFF on the first key
name, so the canonical name was absent from os.environ and Hermes looked
unconfigured with no error. utf-8-sig strips the BOM and is a no-op for
BOM-less UTF-8; latin-1 fallback unchanged.
Own the surrogate-crash class at three chokepoints instead of leaf sites:
- finalize_turn scrubs final_response once where model text leaves the
conversation loop — covers oneshot stdout (#80366), NIM/any-provider
responses (#19819), and every delivery consumer of the turn result.
- _sanitize_gateway_final_response scrubs at the gateway chat-surface
boundary — Telegram utf16_len (#55309) and Signal formatting (#55143)
can no longer see a lone surrogate; raw-text surfaces keep passthrough.
- run_conversation walks the fully-built api_kwargs with
_sanitize_structure_surrogates so tool descriptions (session_search,
#50959) and every other request-body leaf are JSON-encodable before
any provider sees them.
Regression tests pin all three chokepoints plus helper semantics.
Cherry-picked alongside #79240 (TheophilusChinomona) and #80374
(rainbowgore) whose commits precede this one with authorship preserved.
Prevent UnicodeEncodeError when model text contains U+D800-range
surrogates by sanitizing to U+FFFD before writing to UTF-8 stdout.
Co-authored-by: Cursor <cursoragent@cursor.com>
Unknown charset labels (QQ Mail's RFC 1428 'unknown-8bit' placeholder,
misspelled names, garbage encoded-word charsets) raised LookupError from
bytes.decode — errors='replace' only guards decode errors, not a missing
codec — aborting the whole fetch batch. UIDs are marked seen before the
fetch, so the crash permanently dropped every message in the batch.
- _safe_decode(): alias table (unknown-8bit→utf-8, gb2312/gbk→gb18030,
ks_c_5601-1987→cp949, ...) then utf-8, then latin-1 last resort.
- _decode_header_value(): wraps decode_header() so a malformed RFC 2047
header degrades to the raw string instead of crashing.
- _extract_text_body(): all three decode sites now use _safe_decode.
Fixes#35901, fixes#55381, fixes#55383.
Follow-up per review of #74631.
The prior assertion (call_count == 0 OR interactive != True) also
passed if an unintended non-interactive migration occurred, which the
safe fallback (response='n') is supposed to prevent entirely. Replaced
with mock_migrate.assert_not_called().
6/6 pass in the full tests/hermes_cli/test_update_yes_flag.py file.
Ports #68497 forward onto current main per teknium1's review.
input() can raise UnicodeDecodeError when the terminal encoding
cannot decode the byte sequence (e.g. a non-UTF-8 locale, or an
embedded terminal). The prior port targeted hermes_cli/main.py, the
pre-refactor location -- the update pipeline moved to
hermes_cli/update_cmd.py in 927463efcc.
Per review, fixed all three interactive update prompts that call
input() directly, not just the one this originally targeted:
1. Config-migration prompt (update_cmd.py:~3989): extends the existing
except EOFError to also catch UnicodeDecodeError, prints an
actionable 'hermes config migrate' hint, and falls through to the
skip branch (response=n).
2. Stash-restore prompt (_restore_stashed_changes, ~line 971): the raw
input() call here had NO exception guard at all -- not even for
EOFError. Added a try/except covering both EOFError and
UnicodeDecodeError, falling back to the existing skip-restore path
(changes remain safely in git stash, restorable manually).
3. Upstream-remote prompt (_sync_with_upstream_if_needed, ~line 1274):
already caught (EOFError, KeyboardInterrupt) but not
UnicodeDecodeError -- added it to the existing tuple.
Also dropped the incorrect #12884 reference (a TUI sticky-scroll
report, unrelated to this update-encoding issue, per the review).
4 new tests pass covering all three call sites (config-migration prompt
via cmd_update end to end, stash-restore and upstream-remote prompts
via direct unit tests against their own functions), plus an EOFError
sanity test confirming the stash-restore fix doesn't regress that case
either (it had no guard before). 6/6 in the full
tests/hermes_cli/test_update_yes_flag.py file (no regression).
hermes-seaeye[bot]