Return actionable errors when HERMES_WRITE_SAFE_ROOT blocks a path instead of
labeling every denial as a protected credential file. Wire the helper through
write_file, patch, delete/move, and the Copilot ACP shim; sync docs examples.
Include the Hermes client name and version with Gemini inference, model and tier checks, and TTS requests. Add focused coverage for the request headers and keep the Gemini-specific context scoped to Google Gemini endpoints.
#63955 made Hermes survive a broken `bash -l` (Ainz's `Directory
\drivers\etc does not exist`) by falling back to non-login `bash -c`.
But a non-login shell never sources /etc/profile, so it never gets
`…\usr\bin` on PATH — and that dir holds every coreutil the file/terminal
tools shell out to (cat, mktemp, mv, wc, head, stat, chmod, mkdir, find).
Result: `write_file` returned bytes_written:0 with an EMPTY error (the
failure text went to a missing binary's stderr) and terminal commands
exited 127. The survive-broken-login-bash fix was only half-done: it
stopped crashing but silently failed every write.
Derive Git Bash's bin dirs (mingw64/bin, usr/bin, bin, …) from the
resolved bash.exe and prepend them to the subprocess PATH on Windows, in
/etc/profile precedence order so coreutils win over same-named System32
tools (find.exe, sort.exe) inside the shell. No-op off Windows and when a
login snapshot is healthy (the snapshot re-exports the full PATH inside
the shell), so this only bites on the broken-login fallback path.
Adds _git_bash_bin_dirs() (derivation, cached) + _prepend_git_bash_dirs()
(PATH merge), plus regression tests for PortableGit/MinGit layouts and
the run-env injection ordering.
`_module_registers_tools()` reads each `tools/*.py` file and fully
AST-parses it to check for a top-level `registry.register()` call.
90 files are scanned on every process start — but only 32 actually
register tools.
Add a cheap text prefilter: after reading the file (which we need to
do anyway for AST), check that both `"registry"` and `"register"`
appear in the source before calling `ast.parse`. A file with a
top-level `registry.register()` call must contain both strings, so
this is a perfect superset — zero false negatives. 50 of 90 files
skip the AST parse entirely.
The `source=` parameter is not threaded through `discover_builtin_tools`;
the prefilter lives entirely inside `_module_registers_tools`, keeping
the public API unchanged.
Benchmark (median of 10 runs, scanning 90 files):
before (read + ast.parse all): 305.9ms
after (text prefilter + ast): 187.8ms
speedup: 1.6x (118ms saved)
Identical module set: 32 modules, same names, same order.
#63621 fixed path quoting, but Ainz's Git for Windows still dies on
`bash -l` itself (`Directory \drivers\etc`). Hermes then fell back to
bash -l *per command*, so every write_file/terminal call failed the same way.
After a failed login snapshot, probe non-login bash -c; if it works, skip
-l for the session. Also skip a stale HERMES_GIT_BASH_PATH that fails a
noprofile probe in favor of %LOCALAPPDATA%\hermes\git portable bash.
#63113 rewrote native drive paths in ShellFileOperations, but init_session
/_wrap_command still embedded C:/... hermes-snap paths from get_temp_dir.
MSYS arg-converts those during bash -l and surfaces Directory \drivers\etc
— including for relative write_file targets, since the wrapper is the fault.
Add _bash_safe_path, override BaseEnvironment._quote_shell_path on
LocalEnvironment (no base→local import), and normalize mixed /c/Users\...
paths in file ops.
Co-authored-by: xxxigm <tuancanhnguyen706@gmail.com>
ShellFileOperations builds bash commands (wc/head/sed/cat/tee ...) with the
target path as an argument. On a Windows/Git-Bash host a native `C:\...` path
has its backslashes eaten by bash (and mangled by the msys runtime even when
single-quoted) — the "Directory \drivers\etc does not exist; exiting — update
your msys package" class of failures. Rewrite a native drive path to forward
slashes in `_escape_shell_arg`, reusing the env layer's `_windows_to_msys_path`.
Both `C:/...` and `/c/...` fix the backslash bug (the MSYS coreutils resolve
either via the POSIX API). We emit `/c/...` purely for consistency: it's the
same form `_windows_to_msys_path` already produces for the terminal `cd`
(LocalEnvironment._quote_cwd_for_cd), so shell file ops and `cd` share one
helper and one path form.
Scoped from #55481, which also patched BaseEnvironment._quote_cwd_for_cd — but
LocalEnvironment already overrides that through `_windows_to_msys_path`, so on a
real Windows host the base branch never ran (the cwd is already `/c/...`).
Co-authored-by: konsisumer <der@konsi.org>
When web_search results are passed directly to web_extract, the URLs
field contains dict objects (e.g., {"url": "...", "title": "..."})
rather than plain URL strings. Two code paths assumed URLs were always
strings and crashed:
- agent/display.py get_cute_tool_message for web_extract: tried to call
url.replace() on a dict, causing AttributeError
- tools/web_tools.py web_extract_tool loop: tried regex search on a dict,
causing TypeError
Both now extract the URL string from dict objects (url or href field) or
fall back to empty string, preserving the cosmetic display and allowing
the tool to process the URLs correctly.
Fixes#61693
Git Bash hands file tools paths like /c/Users/... which Path() on native
Windows treats as relative \\c\\Users\\... under the process cwd. Reuse
local._msys_to_windows_path (extended for /cygdrive and /mnt drive forms)
in _resolve_path_for_task / _resolve_base_dir so read/write/search land on
the real drive. Container/WSL Linux paths are left untouched.
Salvages #50488 (drops unrelated desktop artifact commit); tests adapted
from #46995.
Co-authored-by: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com>
Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
`_load_web_config()` is typed `-> dict` but returned `load_config().get("web",
{})`, which is `None` when the config has a present-but-null `web:` section
(YAML `web:` with no body). Every caller then does
`_load_web_config().get(...)` and raises `AttributeError: 'NoneType' object
has no attribute 'get'` — this hits `_get_backend`, `check_web_api_key`, and
the extract-char-limit reader.
Separately, `check_web_api_key()` read the backend as
`.get("backend", "").lower()`; a null `web.backend` value yields `None` (the
`""` default only applies when the key is absent), so `None.lower()` raised.
`check_web_api_key` is the `check_fn` gate for `web_search`/`web_extract`, so
this surfaced as an exception during tool-availability checking.
- Make `_load_web_config()` honor its `-> dict` contract (`... or {}`), fixing
the null-`web:`-section crash at every call site.
- Guard the backend value in `check_web_api_key` with `or ""`, mirroring the
existing guard in `_get_backend`.
Adds regression tests for both the null-backend-value and null-web-section
cases.
Sibling sites of the salvaged #47334 fix: xai/openai/elevenlabs/gemini/
mistral/piper/neutts subsection reads in transcription_tools.py and
tts_tool.py used .get(key, {}) which passes a present-but-null value
through as None. All provider-subsection reads now use .get(key) or {}.
Providers without a DEFAULT_CONFIG entry (e.g. stt.xai) were still
receiving None even after the load_config() deep-merge fix, since the
merge can only fill sections that have defaults.
When stt.local, tts.edge, or other config subsections are explicitly set
to null in config.yaml (which happens by default on a fresh --voice
setup), stt_config.get('local', {}) returns None instead of {} because
YAML null preserves the key. The chained .get('model') then crashes
with 'NoneType' object has no attribute 'get'.
Apply the defensive (x or {}) pattern to every place a config subsection
is read via .get('xxx', {}). Covers local, edge, openai, mistral, and
elevenlabs subsections in both transcription_tools.py and tts_tool.py.
Closes#47318
hermes send "MEDIA:/x.png This Caption" now arrives as one native captioned
bubble instead of a separate text message followed by an uncaptioned bubble.
Root cause: the standalone senders (hermes send / cron / send_message tool)
stripped the MEDIA: tag, sent the remaining text as its own message, and
called the media send with no caption -- even though hermes send's help
advertises the captioned form and the bridges/adapters already support a
caption. Signal already captioned correctly.
- tools/send_message_tool.py: new _media_caption_split() chokepoint decides
caption-vs-separate-body (single captionable non-voice file within the
platform's message-length cap). Wired into the Telegram, WhatsApp and
Discord dispatch paths.
- Telegram/WhatsApp/Discord: when the single captioned file is missing, the
caption text is delivered as a plain message so it is never silently lost.
- Telegram caption send gets a MarkdownV2->plain parse fallback.
- Tests: _media_caption_split unit tests + per-platform caption tests
(ride, multi-file fallback, voice exclusion, over-limit fallback,
missing-file text fallback); updated the 3 tests that asserted the old
text-then-media split.
Closes the gap reported against #58911 (the MEDIA_CAPTION directive PR);
credit to @ferreiraesilva for surfacing the caption behavior.
Review finding: callers mutate the returned dicts in place —
hermes_cli/web_server.py annotates s['enabled']/s['usage'] on the skills
list — so handing out the cached objects poisons the cache for every
subsequent caller (and is a cross-thread shared-mutable hazard in the
gateway). Return [dict(s) for s in cached] on both hit and miss paths;
warm-path cost is negligible (241x speedup retained on a 300-skill
fixture). Regression test mutates a returned list/dict and asserts the
next cached call is clean.
Review findings on the cherry-picked cache (follow-up to #58985):
- The cache key was the max mtime of only the TOP-LEVEL scan dirs.
Adding/removing a skill inside a category subdir bumps the category
dir's mtime, NOT the root's, so the cache served a stale list
indefinitely. Replace with a per-dir signature covering roots +
immediate children (one scandir per dir; mirrors
hermes_cli/profiles.py::_count_skills from d5eee133e).
- The disabled-set is config-driven and changes with no filesystem
mtime bump; fold it into the signature so /skills disable takes
effect without a restart.
- Platform is part of the signature (gateway processes serve multiple
platform scopes; scan results are platform-filtered).
- Add a 30s TTL to bound staleness from in-place SKILL.md edits (file
mtime is invisible to any directory signature).
- The original also keyed dirs off the module-level SKILLS_DIR constant;
the scan itself uses _skills_dir() (live profile HERMES_HOME) — use
the same resolution for the signature.
Mutation-verified: nested-add, disabled-set, and TTL tests fail against
the pre-fix cache and pass with it.
_find_all_skills() re-reads every SKILL.md on every call, which is
wasteful when nothing changed between turns. Cache results keyed by
the max mtime across all scanned skill directories — a skill write
touches the directory, bumping mtime past the cached value and
triggering an automatic re-scan.
skip_disabled True/False are cached separately.
This commit is unstacked from #58984; it carries only the skill
discovery cache change.
(cherry picked from commit cd65673a8f)
Completes the session-binding class on the gateway surface (#55578),
matching the TUI rules:
1. Fail-closed pinning: switch_session() re-opens ended sessions, so
pinning a completion to a spawning session that has since ENDED
(user /new, closed rotation) would resurrect a conversation the user
explicitly ended and inject into it. The injection path now checks
the pinned row's ended_at first and drops the injection with a
WARNING when the spawning session is dead or unknown - the result
stays in the delegation records.
2. /new ends the old conversation's delegations: _handle_reset_command
calls interrupt_for_session() with the expiring durable session id
(matching the parent_session_id pin stamped at dispatch) plus the
routing key as fallback, so a reset can't leave dangling subagents
whose completions have no live owner.
interrupt_for_session() gains the parent_session_id selector because a
gateway chat's session_key (the platform conversation key) survives a
reset while the session id rotates - key-based matching alone could
never sever a gateway conversation's delegations.
Background delegate_task completions only carried session_key. When multiple
active sessions shared a routing peer, get_or_create_session could recover the
latest ended_at IS NULL row and inject the subagent result into the wrong
session.
Capture parent_agent.session_id at dispatch time, include it on async-delegation
completion events, and pin gateway routing via switch_session when the
synthetic completion message is handled.
Fixes#57498
Extends the salvaged session_key filter with the same fail-closed,
compression-chain-aware ownership gate the poller uses (#55578):
- drain_notifications() accepts an owns_event callback; when provided,
an async-delegation event is consumed ONLY on positive proof of
ownership, and a broken callback re-queues (never leaks). Bare key
equality remains for single-session callers (CLI); no filter remains
legacy behavior.
- The TUI post-turn drain passes _session_owns_notification_event, so
it can't adopt another session's (or an orphan's) delegation payload,
while a post-compression session still claims its own pre-compression
dispatches - the gap bare key equality left open.
The completion event already carries the dispatching session's session_key
(captured at dispatch time in delegate_tool.py:2798), but the delivery
router ignored it — results landed in whatever session was active at
completion time instead of the session that dispatched the subagent.
Changes:
- drain_notifications() in process_registry.py: optional session_key
filter. Non-matching async_delegation events are re-queued instead of
consumed, so they remain available for the correct session's drain.
- cli.py process_loop: passes active session_key to drain_notifications()
- tui_gateway/server.py post-turn drain: passes session_key from the
TUI session dict
- gateway/run.py _build_process_event_source: logs warning when routing
metadata is unresolvable (previously silent drop)
- Regression tests verifying session-scoped drain filtering
Fixes#58684
Two invariants layered on the origin-routing commit (#55578):
1. Fail closed on orphaned async-delegation payloads. The poller's
belongs-elsewhere check handles events owned by another LIVE session,
but an event whose owner is gone previously fell through and was
adopted by whichever poller saw it - injecting one chat's delegation
output into another chat. Delegation completions are now injected
only into a session that PROVABLY owns them (origin UI id, or
session-key/lineage match via the compression chain); unowned
payloads are dropped from injection with a WARNING (the subagent's
output is already persisted in the delegation records, so nothing is
lost). The shutdown drain applies the same rule. Non-delegation
events keep the historical adopt-orphans behavior.
2. A session's in-flight async delegations end with the session.
_finalize_session now calls interrupt_for_session(): delegations
commissioned by the closing UI session are interrupted always;
key-matched delegations only when the TUI owns the session lifecycle,
so closing a viewer tab on a live gateway session never kills the
gateway's own background work.
Carry the live TUI session id with async delegation completion events and prefer the commissioning UI session when desktop pollers share the completion queue. Resolve compressed session keys to their continuation before treating events as orphaned, and capture the live parent agent session id for TUI/ACP dispatch.
Three fixes for the silent post-restart ticker stall:
1. _jobs_lock() bounds its cross-process flock: LOCK_NB polled against a
30s deadline instead of an unbounded LOCK_EX taken while holding the
process-wide RLock. On timeout it logs at ERROR and degrades to
in-process-only locking (the existing fallback path), so a sibling
process wedged while holding .jobs.lock can no longer freeze every
cron function - including the ticker's get_due_jobs() and thus the
heartbeat - forever with zero logging.
2. fire_claim/run_claim freshness checks are bounded on both sides
(0 <= age < ttl): a claim stamped in the future (clock/TZ skew across
a restart) was previously fresh forever, making the job permanently
unfireable and every manual run report 'already being fired'.
3. _execute_job_now distinguishes paused/disabled/missing jobs from a
genuinely held claim instead of mislabeling them all as 'already
being fired'.
safe_load() raises ComposerError on multi-document streams (k8s manifests)
and ConstructorError on application-defined tags (CloudFormation !Sub,
Ansible !vault) — both valid YAML syntax. Now that the linter's verdict is
a fail-closed write gate, those false positives would refuse legitimate
writes outright. Switch to yaml.parse() (scanner+parser only), which still
catches real syntax failures.
write_file() previously called _atomic_write() first and only ran the
JSON/YAML/TOML/Python syntax check afterward as an informational lint
delta -- a parse failure never set the top-level `error` key, so a
corrupt structured-data write still landed on disk (and file_tools.py's
files_modified gating, which keys off `error`, silently reported it as
a successful modification).
Move the in-process syntax check for JSON/YAML/TOML ahead of
_atomic_write() and refuse the write outright on a parse failure: no
temp file, no rename, nothing touches disk, and the result carries a
top-level `error` so callers correctly see it as unmodified.
Deliberately scoped to _FAIL_CLOSED_INPROC_EXTS (JSON/YAML/TOML), not
all of LINTERS_INPROC -- .py is excluded because this codebase's own
test fixtures (TestPatchReplacePostWriteVerification et al.) write
arbitrary non-Python text through *.py paths purely to exercise
write-mechanics; a hard block there broke 3 previously-passing tests
during development. Python keeps its pre-existing non-blocking
lint-delta report.
Adds tests/tools/test_write_file_syntax_gate.py: invalid JSON/YAML/YML/
TOML refused with nothing written (new file) and nothing modified
(existing file); valid JSON/YAML still written byte-for-byte; a
non-linted extension with garbage content is unaffected; invalid Python
is confirmed NOT hard-refused (still just reported).
