Three-layer companion to the salvaged CLI drain-ownership fix (#64240):
1. restore_undelivered_completions stamps restored=True (in-memory only)
on every durable completion re-enqueued at process start.
2. drain_notifications' legacy unfiltered branch re-queues restored
events instead of consuming them — a fresh process can no longer
adopt a dead session's delegation results (#64484). Same-process
keyless events keep the legacy behavior.
3. delegate_tool's async dispatch now falls back to the parent agent's
durable session_id when the approval-context key resolves empty (the
CLI case), so the CLI's new positive-ownership drain can actually
claim its own completions instead of failing closed on ''.
Review finding on the salvaged collector: _wait_for_process is the shared
drain for EVERY env.execute() consumer, not just the terminal tool. Applying
tool_output.max_bytes there silently truncated file-operation cat reads
(read_file_raw feeds the patch engine — read-modify-write on any file >50KB
would corrupt it), paginated read_file, code-execution RPC reads, and log
reads.
bounded_capture is now an explicit opt-in on execute()/_wait_for_process,
set only by the foreground terminal tool. Default preserves the historical
full-fidelity capture via an effectively-unbounded collector (single code
path). Modal transports accept the kwarg for signature parity.
New regression test: default execute() returns a 200KB payload complete and
untruncated. E2E: 20MB internal read intact; ShellFileOperations
read_file_raw round-trips byte-exact; terminal path still bounded at 50KB.
Follow-ups on top of #64061's salvage:
- ResourceLink markers now point at mcp__<server>__read_resource (the
actual registered tool name via mcp_prefixed_tool_name) instead of a
nonexistent <server>_read_resource the agent could hallucinate-call.
- The isError path now surfaces EmbeddedResource .resource.text blocks
instead of dropping them, so error payloads carried in resources no
longer collapse to a bare 'MCP tool returned an error'. (Same-class
fix flagged in #64061 and independently addressed in #63576 by
@alauer.)
- 3 new error-path tests + updated ResourceLink wire-name assertion.
MCP tool results with non-image binary resources (PDFs, archives, office
docs) were silently dropped: the success path only handled TextContent and
ImageContent, so a PDF-returning MCP tool appeared to return metadata only.
- EmbeddedResource blob contents are decoded (50MB cap), materialized into
the Hermes document cache via cache_document_from_bytes (sanitized
filename, traversal-safe), and surfaced as a local-path marker the agent
can read with file/terminal tools.
- EmbeddedResource text contents are inlined directly.
- ResourceLink blocks preserve the URI and point the agent at the server's
read_resource tool; no arbitrary network fetch outside the MCP session.
- AudioContent blocks are cached via cache_audio_from_bytes as MEDIA: tags.
- read_resource blob contents are materialized the same way instead of
returning '[binary data, N bytes]'.
- Unsupported blocks are logged instead of silently discarded.
- Existing ImageContent MEDIA: behavior unchanged.
Reported by an enterprise customer; reproduced against an HTTP MCP server
returning application/pdf resources.
Slack's June 30 Agent messaging experience changelog lists Bolt Python
1.29.0 / Python SDK 3.43.0 as the Agent View minimums. Bump the
messaging/slack extras and the platform.slack lazy-install pins to
match, and regenerate uv.lock. All adapter API surfaces verified
present against the new versions in a clean venv.
The terminal environment is shared process-globally (collapsed to the
default key), so env.cwd tracks the LAST session that ran a command.
_resolve_command_cwd() trusted env.cwd unconditionally — no ownership
check — so when session A left env.cwd pointing at A's checkout,
session B's first terminal command inherited A's stale cwd and ran in
the wrong workspace.
The file tools already solved this exact shared-env problem with
_live_cwd_if_owned() checking env.cwd_owner. The terminal tool never
got the same guard.
Fix: capture env.cwd_owner BEFORE the current session claims it, and
pass it as prev_owner to _resolve_command_cwd. When the previous owner
was a different session, env.cwd is stale — fall through to default_cwd
(the config/override cwd for this session) instead. Once the session
has claimed the env, subsequent calls in the same session still trust
env.cwd so in-session state survives.
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
