The cooldown-clear sites, the selected_status branch and the only_if_idle
argument all repeated 'scope is None and names is None' (or its negation);
use the wildcard name so the sites cannot drift.
The new test also binds the orphaned-adopter bookkeeping the fix enabled for
the unscoped owner, and starts the MCP loop through _ensure_mcp_loop like the
sibling fixtures instead of a hand-rolled thread.
`reconcile_mcp_servers_with_config` prunes dropped servers with
`shutdown_mcp_servers(scope=_mcp_registry_scope(), names={...})`. For the
launch profile that scope is `None`, and `shutdown_mcp_servers` read
`scope is None` as "every owner": a dashboard/desktop backend whose own
profile removes (or disables) server `x` also closed profile B's `(B, "x")`
connection and dropped its tools, and skipped the orphaned-adopter
bookkeeping that would have re-registered them.
Only the bare call (no scope, no names) is the process-wide wildcard now;
`scope=None` with `names` selects the unscoped owner's servers, matching how
`reconcile_mcp_servers_with_config` already computes `owned`.
Reported by @andrexibiza in review of #111354.
record_agent_write and safe_restore_plan each spelled out
_project_hash(get_working_dir_for_path(...)); the bug being fixed was those two
sites disagreeing on the key. Name it once (_ledger_key) and have the reader
use only that key: the writer has keyed by the marker walk since the ledger
was introduced (bd4b709258), so an exact-dir ledger only exists where the two
keys coincide.
record_agent_write keys the agent-write ledger by the marker-walk result of
get_working_dir_for_path(), while safe restore reads the ledger by the exact
hash of the directory named in the restore. The two keys diverge whenever
the walk lands somewhere other than that directory:
- a markerless project dir under an ancestor that carries a generic project
marker (e.g. /tmp with a stray package.json) -> the ledger is filed under
the ancestor's hash and restore reads an empty ledger;
- restoring a repo subdir while the write was recorded at the repo root.
An empty ledger makes restore() silently degrade to a full restore, which
overwrites files the user hand-edited after Hermes' last write — the exact
outcome safe restore exists to prevent.
safe_restore_plan now falls back to the walked key when the exact-key ledger
is empty, so the recorded protection is honored regardless of where the walk
landed. Verified: 9 previously failing TestSafeRestore tests now pass, plus
a new regression test that seeds an ancestor marker and asserts the user
hand-edit survives the restore.
Two authority gaps in served_profile_child_env (#111617 review, andrexibiza P1 #1/#2,
kvnloo finding 1):
- The base was hermes_subprocess_env(inherit_credentials=True) = the launch environ's
provider credentials; strip_launch_profile_env only knows names with .env/source
provenance, so a key systemd/Compose/the shell injected into the launch process
survived into profile B's child whenever B did not define the same name. Now a ROUTED
target scrubs every Tier-1/Tier-2 credential from the base regardless of provenance
before B's own scope is overlaid (the child boundary gets get_secret's contract: a
scoped miss is no credential, never ambient fallback). The launch profile's own child
keeps its env. bot_relay's base=os.environ goes through the same scrub.
- strip_launch_profile_env / the scrub keyed on is_multiplex_active(); the Desktop and
dashboard backends serve ?profile=B by installing the HERMES_HOME override without
that flag, so B's slash worker / helper children kept A's .env and settings. The
authority test is now "is the target a routed home" (target != process home).
- _build_browser_env resolved the passthrough keys via get_secret, which falls through
to os.environ on a scoped miss while multiplexing is inactive: a routed B with no
Firecrawl key got A's. Under serves_routed_profile() the bound scope is the only source.
- served_profile_child_env(inherit_credentials=True) with no target and no scope bound
under multiplex minted with the launch credentials (key_cmd TTL refresh on a worker
thread); it now raises UnscopedSecretError like get_secret.
tests/tui_gateway/test_served_profile_child_env_authority.py: ambient-only A key + B
missing it (mux on), flag-off routed B (helper child + browser), real child observation.
3/3 red on base.
A long-lived serve process keeps a deleted profile as the context home of threads
that outlive the delete. A bare `mkdir(parents=True)` right before an atomic write
brings `profiles/<name>/` back after `hermes profile delete` has written the
tombstone and removed the tree.
The writers in `utils` and the seven callers named in #112592 are guarded by the
preceding commits; this one applies the same `mkdir_under_hermes_home` idiom to the
other pre-write directory creations found by the same mechanical rule (auth,
personality, plugin catalog, skills sync, tool discovery cache, platform adapters,
memory plugins, local runtime supervisor, process identity, breadcrumbs). The two
sites that pass `mode=` keep their mkdir behind `assert_named_profile_home_live`.
The guard is a no-op unless the target has a provable `profiles/<name>` ancestor.
Salvaged from #112596 (30-file sweep) on top of #112594 / #112601; the overlapping
files were resolved to the already-landed versions.
Background writers that still carry a tombstoned profile as their Hermes
home (reasoning-caps warm thread, models cache, models.dev ETag, gateway
lifecycle ledger, MCP OAuth tokens, memory store) re-created
profiles/<name>/ with a bare mkdir right before an atomic write. Route the
parent-dir creation through mkdir_under_hermes_home so a deleted named
profile raises FileNotFoundError and stays gone, matching the tombstone
contract already enforced for logging and state.
When the CLI approval callback raises, when no callback is registered on the
thread while prompt_toolkit owns the terminal, or when the input() read is
interrupted, prompt_dangerous_approval returned "deny" and the command gate
rendered "BLOCKED: User denied this command" — attributing a refusal to a
user who was never asked (#22992). #112308 fixed the gateway half of the
class (withdrawn prompts -> outcome "cancelled" with a cause); this closes
the CLI residual on the same shape.
- tools/approval_prompt.py: those three paths return an Unanswered("cancelled")
sentinel carrying the cause; MCP elicitation consent maps it to "cancel".
- tools/approval.py: the CLI gate renders "BLOCKED: <noun> was not approved: the
approval prompt could not be delivered or was not answered (<cause>)" with
outcome "cancelled" — still fail-closed, "Silence is not consent".
- tools/file_tools_write_guards.py: the protected-instruction write gate
reports the undelivered prompt instead of "was denied by the user".
- Shared metrics: "cancelled" is a counted approval outcome (contract + v2
schema) instead of falling into "unknown".
- Docs: hook `choice="cancelled"` now covers the CLI causes.
Fixes#22992
The per-session notification poller ran `_poll_bot_live_delivery_once` every
0.5 s. Once a "Bot Chat" session exists, each pass opens state.db and takes
the exclusive active-session registry lock; on Windows (`msvcrt` LK_LOCK
gives up after 10 s of contention) that raised
`RuntimeError: active session file lock unavailable` and the loop logged
`Bot live-owner delivery poll failed` on every attempt — 8,838 warnings in
three days, 91% of one install's WARNING output (#111719).
Two changes:
- `tools/bot_live_delivery.has_mailbox`: the mailbox directory is created
only when a delivery is first admitted, so a profile without it has nothing
to claim — the poll now returns before the state.db open / registry lock.
This keeps cron→Bot Chat and Bot Mode DM delivery intact on installs with
no messaging platform configured (both deliver through this mailbox), which
is why the poll is gated on the mailbox rather than on connected platforms
(PR #111733's guard would have broken those).
- `_poll_bot_live_delivery_guarded`: a failing poll backs off 5 s before the
next attempt and is logged at WARNING once per 60 s window (with the count
of suppressed repeats), DEBUG otherwise.
Live probe (real poller loop, temp HERMES_HOME with a Bot Chat row and the
registry lock made unavailable, 3 s):
before: owner_lookups=6 WARNING=6 (with or without a mailbox)
after: no mailbox -> owner_lookups=0 WARNING=0; mailbox -> owner_lookups=1 WARNING=1
Fixes#111719
Co-authored-by: KoNit-K <124019182+KoNit-K@users.noreply.github.com>
execute_code ran the same unbounded _is_supervised_gateway_process() probe
ahead of every cell, so the wedge #111922 bounds in terminal_tool still hung
an execute_code call (and its cron slot) forever: share the cell's deadline
and fail closed with a retryable error when the probe renders no verdict.
Moving the terminal pre-exec guard onto a deadline worker made it blind to
/stop, which keys on the tool thread's ident: record the acting-for tid in a
contextvar (copied into the worker by run_bounded_sync) so is_interrupted()
on the worker honours the tool thread's bit too.
Floor the guard's share of the deadline at 30s so a short command timeout
does not turn the guard's own cold-start cost (imports, git probes under
load) into a refusal — tests/tools/test_terminal_error_redaction.py was red
on the branch for exactly that.
The salvaged commit put `_pre_exec_block` behind the command's
`run_bounded_sync` deadline but let a timed-out guard fall through into
execution. The gateway-lifecycle, dangerous-workdir and self-repo checks
apply unconditionally (`force=True` cannot bypass them), so a guard that
never rendered a verdict must not let the command run unguarded: return
the terminal error envelope (`status: error`, "did not finish ... Retry
the call") instead, mirroring how the bounded `env.execute` path reports
its own expiry as a result rather than continuing.
Tests trimmed to the two invariants: a wedged guard returns a bounded
error without executing; a completed guard keeps its verdict (pass ->
execution, rejection -> its own blocked result).
A `lazy: true` MCP server registers its tools from the schema cache and
spawns on first use. Three consumers still equated "alive" with a live
session, so a healthy all-lazy startup was reported as a total failure:
- `get_mcp_status()` fell through to `status: configured, tools: 0` for a
lazily registered server. It now reports `lazy` with the cached tool
count (`connected: False`); an in-flight or failed first-use connect
still outranks it because the error is the actionable part.
- `discover_mcp_tools()`'s summary counted every name absent from
`_servers` as failed, logging `MCP: 0 tool(s) from 0 server(s) (2
failed)` right after registering every cached tool, and re-announced
the same "failure" on every repeat discovery. Lazy servers are now
reported as `(N lazy, not spawned yet)` and an already-lazy server is
not re-announced.
- `hermes_cli/mcp_startup.py` judged a discovery run by `connected` at
two sites, so every startup logged `Background MCP discovery completed
with zero connected servers` and every later call re-spawned the
discovery thread as a retry. One predicate,
`_discovery_registered_servers`, treats a lazy registration as a
usable outcome at both sites.
- `hermes_cli/banner.py` rendered the unknown `lazy` status through the
red "could not connect" line; it now shows the cached tool count with
`(lazy, starts on first use)`.
Ported from #100648 (core hunks only; the toolsets-filter predicate
branch, the Ink TUI component extraction and 13 tests were not ported).
Fixes#111717
A write-capable tool on a `trust: untrusted` MCP server was denied instantly
from POST /v1/runs: request_elicitation_consent only took the gateway path
when _is_gateway_approval_context() was true, and api_server sits in
_UNATTENDED_APPROVAL_PLATFORMS (webhook-style sessions have nobody to
answer). A live /v1/runs run is the exception: it registers a gateway notify
callback and answers via approval.request -> POST /v1/runs/{id}/approval —
the same bridge 04fcf9159 keeps alive for the dangerous-command gate. Treat
an api_server session that is neither cron nor single-query as
callback-backed; a run without a registered callback still fails closed.
Salvaged from #111529 with the redundant single-query re-gate on the
generic gateway branch dropped (no real surface binds a chat platform,
HERMES_SINGLE_QUERY_SESSION and an in-process callback together).
Part of #111526
Review follow-up on the MCP HTTP proxy PR:
- Proxy mounts win over transport= for matching URLs, so a bare
AsyncHTTPTransport mount bypassed the 10 MiB wire-body cap whenever a
proxy applied. Each mount is now wrapped in _make_mcp_body_cap_transport.
- Loopback MCP servers (127.0.0.1 / ::1 / localhost) were dialed through
HTTP_PROXY unless NO_PROXY covered them; _mcp_proxy_mounts now returns
None for is_loopback_host (agent.proxy_bypass rule).
- Dropped the fail-open try/except around the proxy transport construction;
a proxy httpx cannot build surfaces as the server's connect error.
- The content-type preflight client now takes an explicit transport plus
the same proxy mounts as the SDK client, so probe and handshake take the
same route (no httpx env auto-detection divergence).
Follow-up to the salvaged #111796 commit:
- NO_PROXY matching goes through `agent.proxy_bypass.should_bypass_proxy` (the one
matcher the LLM transport and the gateway adapters already use), so CIDR ranges and
`*.host` patterns bypass the proxy for MCP servers exactly as they do for the model
endpoint. The stdlib `proxy_bypass` stays for the OS bypass list (Windows
ProxyOverride / macOS exceptions). Live probe: NO_PROXY=10.255.255.0/24 still routed
the MCP request through the proxy before this commit, direct after.
- Drop the try/except around `getproxies()` / `proxy_bypass()`: the stdlib guards its
own registry/sysconf reads and httpx calls the same functions unguarded.
- Trim the six contributor tests to two invariants (mount + NO_PROXY incl. CIDR; both
client builders carry mounts next to the body-cap transport). Fixture uses the stdlib
`getproxies_environment` / `proxy_bypass_environment` instead of a hand-rolled copy and
skips when the mcp SDK is absent.
- Docs: one sentence on the MCP page about proxy resolution for HTTP/SSE servers.
- contributors/emails mapping for the PR author.
httpx auto-detects proxies only when ``transport is None``
(``allow_env_proxies = trust_env and transport is None``). The wire-body cap hands
every MCP HTTP/SSE client a custom transport, so HTTP_PROXY / HTTPS_PROXY and the
Windows-registry / macOS system proxy were silently ignored: on a network that
reaches the MCP host only through a proxy, every connect failed with
"All connection attempts failed" and the server was parked (tools never appeared).
Rebuild httpx's own proxy resolution as explicit ``mounts`` — environment first,
then the OS proxy, NO_PROXY / platform bypass honoured, socks:// normalized, and
TLS settings identical to the transport they accompany.
On Windows a stdio MCP server configured with `command: npx|npm|node` failed
with WinError 2 whenever the desktop/gateway PATH lacked the managed Node dir:
`_node_fallback` probed only the POSIX shape `<HERMES_HOME>/node/bin/<cmd>`
with no extension, while `scripts/install.ps1` unpacks Node directly into
`<HERMES_HOME>\node` as `npx.cmd`/`npm.cmd`/`node.exe`. It also derived the
home from raw `os.getenv("HERMES_HOME")`, so a context-local profile home
(multiplexed gateway) was ignored.
Reuse the platform-aware helpers instead of a second hand-rolled layout:
`hermes_constants.iter_hermes_node_dirs(get_hermes_home())` supplies both
managed shapes in the right order, and the module's own `_npx_bin_candidates`
supplies the `.cmd` -> `.exe` precedence (same injectable `windows=` seam the
npx-cache shortcut already uses, so the branch is testable on Linux CI).
POSIX candidates (`node/bin`, `~/.local/bin`, `/usr/local/bin`) are unchanged.
Slimmer redo of #111941 by @KoNit-K, which re-derived the Windows shape
in-place and kept the raw env read.
Fixes#111937
Co-authored-by: KoNit-K <124019182+KoNit-K@users.noreply.github.com>
Review finding on #112218 (major): `_skill_lock_path` opened `<skills>/.locks/<name>.lock`
before the name was validated, so `skill_manage(action='create', name='a'*300)` raised
OSError (File name too long) and a NUL name raised ValueError instead of the handler's
JSON error, and every rejected name ('../../etc', '') left a residue lock file.
- tools/skill_manager_tool.py: lock filename is sha256(basename).lock (fixed width, no
filesystem limit reachable; `foo` and `category/foo` still share one lock), the redundant
`_find_skill` rglob is gone, and `skill_manage` runs `_validate_name` on the name
(create) / basename (other actions) before the lock is opened.
- '.locks' joins the skills-dir exclusion sets (EXCLUDED_SKILL_DIRS, ledger
_NON_PACKAGE_TOPS, learning-graph/skill-commands skip parts, curator backup excludes).
- tests: 2 invariants in TestSkillMutationLock (rejected names -> JSON + no .locks residue;
digest-keyed lock shared across name forms), red on the old head.
Slim follow-up to the cherry-picked #111585 (@KoNit-K):
- tools/skill_usage.py: generalize the usage ledger's `_usage_file_lock()` into
`skill_file_lock(lock_path)` — same fcntl/msvcrt idiom, now thread-re-entrant
via a per-thread held set (flock is not re-entrant across separate fds; a
ContextVar would leak "held" into copy_context() timer threads).
- tools/skill_manager_tool.py: drop the third fcntl/msvcrt copy, hashlib and the
ContextVar; the per-skill lock is `<skills>/.locks/<skill-dir-name>.lock`
(readable, outside the skill dir so delete/recreate cannot unlink it under a
waiting writer). Batch locks sort by lock PATH, not name, so two batches
naming the same skills in different forms cannot deadlock.
- tools/skill_manager_batch.py: plain `with` around snapshot -> commit/rollback
instead of manual __enter__/__exit__ bookkeeping.
- tests: trimmed to two invariants — the two-writer lost-update test on
SKILL.md (from #111585) and a re-entrancy/exclusivity test on the helper.
Dropped: the edit/write_file/remove_file parametrization (same dispatcher
path as patch) and the category-dir cleanup test (lock files never lived in
category dirs here).
Review finding: _exc_children returned only .exceptions for a group, so
_is_session_expired_error missed a session-expiry marker (or the
InterruptedError override) hanging off a group's __cause__/__context__
that main used to inspect. Groups now yield nested + chain like every
other node; _flatten_messages' "group str() is opaque" rule is unchanged.
The salvaged fix gave `_find_missing` and `_flatten_messages` each their own
visited-set loop, next to the one `_is_session_expired_error` already had —
three copies of the same idiom in one module. Collapse them into
`_iter_exception_nodes` (pre-order, left-to-right, each node once, bounded by
`_EXC_TRAVERSAL_MAX_NODES`) and read all three scans off that list. Acyclic
output is byte-identical: the missing-executable search keeps its depth-first
order and a message-less leaf still renders as its class name.
Tests move from the issue-numbered file into `tests/tools/test_mcp_tool_errors.py`
(mirror of the source module): a two-node cycle renders the real messages, and a
missing stdio binary wrapped deeper than the recursion limit with the chain
looping back to the top is still reported as the missing executable. Both are
red on origin/main (RecursionError).
Co-authored-by: Stephan Mongstad <stephan@users.noreply.github.com>
Review finding on #112198: _mask_prose_link_destinations matched
_FENCE_LINE against the raw line, so a fence behind a CommonMark
container prefix (`- ```sh`, `1. ```sh`, `> ```sh`, nested) was not
seen and its body was scored as prose with link destinations masked.
Strip the container prefix before fence matching (open and close).
Bundled-skill rescan vs origin/main: 208 skills, 1447 findings on
both, no new/gone findings, no verdict changes.
Follow-up to the two cherry-picked contributor commits.
The picked fence tracker never checked for a closing fence once a block was
open (the closer test sat inside the not-in-code branch), so every prose link
after any code block was scanned verbatim again and the #111254 documentation
link exemption was lost; a fence line carrying an info string was also accepted
as a closer, which handed the scanner back to prose mode mid-block. Rewrite the
loop around CommonMark fence semantics: a block opens on 3+ backticks/tildes
indented at most 3 spaces (backtick info strings may not contain a backtick)
and closes only on a fence with the same marker, at least as long, and nothing
after it; tab- or 4-space-indented lines are code; an unclosed fence stays
code to EOF. plugin_guard inherits the behaviour through scan_file.
The temp-root exemption in destructive_root_rm now also refuses a parent
segment reached through an empty path segment or followed by a shell
separator, which the first cut let through.
Tests trimmed to one invariant per fix: the fence test covers the six code
shapes plus the prose-link-after-fence control that the picked version broke;
the rm test gains the two residual shapes.
Part of #111334Fixes#112129Fixes#111335
replace the boolean fence toggle in _mask_prose_link_destinations with
proper (marker_char, opener_length) tracking so a mismatched-markdown-fence
body or an indented code block cannot re-enable prose-masking over live
command lines. closes an exploitable bypass in the community-source
install path; plugin_guard inherits the fix through scan_file.
also tighten is_indented_code to treat any tab indent (single or double)
as code, per CommonMark §4.4.
`hermes mcp login <server> --flow device` took `authorization_servers[0]`
from the protected-resource metadata and failed when that entry was a
browser-only or issuer-inconsistent server, even though a later entry was
the issuer-bound device_code server meant for headless clients (Higgsfield
advertises exactly this shape: a PKCE server first, the device server second).
Discovery now tries each advertised server in order and binds to the first
whose metadata issuer matches its advertised URL and that offers device
authorization. Issuer validation (RFC 8414 / SEP-2468) is unchanged per
server; a single-server resource raises exactly the error it raised before,
and a multi-server resource with no usable entry reports every attempt.
The browser path (`tools/mcp_oauth_manager.py` pre-flight) is deliberately
left on the SDK's own first-entry selection: the SDK's 401-branch discovery
re-selects `authorization_servers[0]` itself, so a divergent pre-flight pick
would only desynchronise the cached metadata from what the SDK authorizes against.
`terminal(background=true, notify_on_complete=true)` appended its watcher descriptor to
`process_registry.pending_watchers`, which only the post-turn hooks drain. A process that
finished while the turn that launched it was still running (an agent sleep-polling for
hours) had no watcher task at all: the completion_queue entry sat inert, nothing was
injected, and the chat stayed mute until that turn ended (#112033).
- `_register_completion_watcher` arms the watcher on the live gateway loop at registration
(`GatewayRunner.arm_process_watcher`, via the existing `_gateway_runner_ref` /
`_gateway_loop` seam that send_message and cron already use); `pending_watchers` stays
the fallback while the gateway is not serving (checkpoint recovery at startup, shutdown).
- The agent-notify branch of `_run_process_watcher` keeps its design (the agent's next turn
is the user-facing report) but, when the launching turn is still active at process exit,
the injection only queues a follow-up — so the concise receipt is sent to the chat right
away instead of never. The busy check is taken before injection because the injected turn
itself installs the adapter's session guard.
Live probe (real process, real GatewayRunner loop, fake telegram adapter, busy session):
before — pending_watchers=1 after exit, 0 watcher tasks, 0 injections, 0 receipts;
after — pending_watchers=0, watcher task armed at launch, 1 injection, 1 concise receipt.
Control (idle session): 1 injection, 0 receipts, unchanged.
Slimmer redo of #112038 by @KoNit-K: same two gaps closed, without a second scheduler
registry / loop attribute on ProcessRegistry and GatewayRunner.
Co-authored-by: KoNit-K <124019182+KoNit-K@users.noreply.github.com>
CI runs the suite as an unprivileged user with HOME=/root in one fixture;
Path.is_dir() raised PermissionError from _user_local_bin_entries and the
run-env builder crashed. An unreadable home has no usable ~/.local/bin, so
treat the OSError as absent.
Slim follow-up to the salvaged #111790: the helper becomes a list-returning
sibling of _managed_runtime_path_entries (same shape, same "only when it
exists" convention) and loses the Windows check the caller already performs.
Why here and not in the Electron remote spawn: propagating the login-shell PATH
that locateHermes discovered into `exec env HERMES_DESKTOP=1 … hermes serve`
would fix only the Desktop SSH surface; the terminal environment's PATH
completion is the seam every thin-PATH launcher (SSH, systemd, launchd, cron)
already goes through, so the class closes once. Windows twin out of scope.
Tests move to the mirror dir tests/tools/environments/ with an absent-dir
control; FAQ documents the terminal PATH composition.
Fixes#111778
clear_session (/new, /reset, auto-reset boundary) stamped entry.result="deny"
before waking the wait, and an interrupted coalesced leader published the
same deny to its followers, so both still rendered outcome="denied" /
"denied by user". Carry the cause on the entry (entry.cancelled) and let
_cancel_cause map a result-less wake to a withdrawn prompt; the wait still
unwinds fail-closed and the leader's own decision is unchanged.
When a gateway approval wait ends without anyone answering — the parent's
delegate_task finishing and tearing the child down, a /stop, or the turn's
notifier being unregistered at turn end — the tool result said
"BLOCKED: Command denied by user" (outcome="denied", user_summary "You denied
this command"). The user never saw or answered the prompt, so the parent agent
went on reasoning about a refusal that never happened (#112026, #22992).
The action stays fail-closed (the command does not run, the model still gets
the NOT-consented stop text), but the attribution is now truthful:
- tools/approval_gateway_wait.py: `_cancel_cause()` reads the existing
per-thread interrupt-cause channel (`get_interrupt_reason()`, a trusted fixed
category — no string matching) for the interrupted state and marks a
notifier-unregister wake (event set, result None) as "the turn ended before
the prompt was answered". Both the direct and the coalesced-follower wait
return `cancelled=<cause>`; the post_approval_response hook fires
choice="cancelled" instead of "deny"/"timeout".
- tools/approval.py: a cancelled decision renders
"BLOCKED: Command approval was withdrawn before the user answered (<cause>)."
with outcome="cancelled" and its own user_summary; an explicit /deny is
untouched.
- tools/delegate_tool_child_run.py: `_signal_child_stop` publishes a fixed
tool_reason ("parent delegation ended"; the late-child mirror forwards the
parent's own category) so a child's pending approval can tell teardown from a
user /stop — previously it rode the default "explicit stop requested".
- tools/file_tools_write_guards.py / tools/approval_prompt.py: the protected
instruction-file gate and MCP elicitation consume the same key instead of
reporting "denied by the user" / "decline".
Co-authored-by: zccyman <16263913+zccyman@users.noreply.github.com>
Co-authored-by: KoNit-K <124019182+KoNit-K@users.noreply.github.com>
terminal_tool's approval gate answers `status: pending_approval` with an
EMPTY `error` (#28323) and no `session_id`, so _spawn_delivery's specific
branch (`if parsed.get("error")`) was skipped and every unanswered
approval fell through to "Delivery to X failed to start: no process id
returned" — blaming the spawn for an approval nobody in a non-interactive
turn (api_server, `hermes peer dm`, cron) could grant.
- _spawn_delivery: the pending shape gets its own message (the runner
command needs terminal approval nobody in this turn can grant); a
local/peer DM adds "nothing was sent — approve it or add it to
command_allowlist and send again". Ownership is never transferred, so
the existing finally still reclaims the plaintext DM file.
- _try_relay_delivery: the envelope is queued on disk BEFORE the reply
waiter spawns and the Desktop drains it independently, so ANY waiter
spawn failure is a lost wake-up, not a failed delivery; reporting it as
an error made the sender resend and deliver the message twice. The
relay path now returns the shape _start_delivery's live-owner branch
already uses (status queued + notification_error + "Do NOT resend")
instead of inventing a new status value nothing reads.
Slimmer redo of #92971 by @jonpol01 (same diagnosis, same relay/local
split on `dm_file is None`); the source-text contract test and the
`sent_no_reply_wake` status were dropped.
Fixes#111716
Co-authored-by: John Paul Soliva <soliva.johnpaul@icloud.com>
Two defects in tools/async_delegation.py:
- _push_completion_event called _persist_completion unguarded before
publishing onto completion_queue. One sqlite3 error (locked/full
state.db) dropped the completion event, left the record parked on
"finalizing" (a permanently leaked max_concurrent_children slot) and
let recover_abandoned_delegations later rewrite a succeeded unit as
"unknown". The write is now try/except: the failure is logged and the
event is still delivered, so _finalize flips the status and frees the
slot. A lost durable row is acceptable degradation; a lost result and
a leaked slot are not.
- _prune_completed_locked treated anything != "running" as finished,
while the module's own _LIVE_STATES also names stalling/finalizing.
A stalling record has no completed_at, so it sorted oldest and was the
first eviction candidate once the retained cap overflowed; its late
runner return then hit the missing-record path and the real result was
dropped. The predicate is now `status not in _LIVE_STATES`.
Slim redo of #76606 (earliest fix) and #112031: the converge/shield/
delete-row machinery both PRs built around the write is dropped as
defense-in-depth; the two core hunks are ported as-is.
Fixes#76605Fixes#112030
Co-authored-by: luckystar2026 <1393268817@qq.com>
Follow-up to the cherry-picked gateway fix: instead of re-inferring "keyless
mode" from the key env var (wrong for Firecrawl, whose managed-gateway and
self-hosted routes bypass the ring without a key), `_rescue_eligible` asks the
ring vendor's own predicate — `_use_keyless_ring()` for Firecrawl, `use_keyless`
for the others. That covers the persisted `nous` selection the contributor fix
handled AND the legacy never-configured fallback onto a ready gateway, plus
`FIRECRAWL_API_URL`. A ring vendor that actually walked the ring stays
ineligible (its failure means the ring already failed). Docs mention the
gateway route is rescued.
Follow-up to the cherry-picked "cache extracts by returned URL": Keenable and
Firecrawl report the post-redirect address in `url` and the REQUESTED URL in
`metadata.sourceURL`, so matching on `url` alone left every redirected page
uncached. Accept either field, as long as it names a URL from this batch;
anything else is served but never cached (a miss re-fetches, a mis-key poisons
the cache for the whole TTL). Docs: say the cache key is the requested URL the
provider reports, not the batch position.
Co-authored-by: nemofq <5635994+nemofq@users.noreply.github.com>
Co-authored-by: wooyongbin3-cpu <256294002+wooyongbin3-cpu@users.noreply.github.com>
Reuse tests/tools/test_delegate_output_schema.py's _StubChild instead of a
new one-test file with its own double; the invariant (the retry turn sees
is_delegated_child_context() True and the flag is restored afterwards) is
unchanged. Trim the source comment to the WHY.
_validate_child_output_schema issues a second run_conversation on the child when
the first answer fails the declared output_schema. The main child turn is wrapped
in delegated_child_context; this one was not. It runs on the parent worker's
thread, where HERMES_KANBAN_TASK is set and nothing marks the execution as a
child, so every identity gate keyed on is_delegated_child_context() fails open.
The visible effect is the kanban stop guard: it nudges the child to call
kanban_complete or kanban_block. A child owns no board task and carries no kanban
toolset, so it cannot, and the nudge text ("do not narrate intent", "finish any
remaining deliverable") displaces the structured answer the retry exists to
produce. The retry then fails the same schema and delegate_task reports an error
for a child whose work was already complete.
Observed with four children, each nudged during its retry:
[subagent-0] Kanban worker tried to exit without kanban_complete/kanban_block
[subagent-2] Kanban worker tried to exit without kanban_complete/kanban_block
[subagent-3] Kanban worker tried to exit without kanban_complete/kanban_block
[subagent-1] Kanban worker tried to exit without kanban_complete/kanban_block
4/4 - Final answer does not satisfy the declared output_schema (after 1 retry)
Wrap the retry the same way the main turn is wrapped. The context is entered and
exited around the single call, so nothing outside the retry sees it.
Signed-off-by: moep90 <volleyballlive@googlemail.com>
_expand_parent_toolsets built the parent's tool surface from each
toolset's declared `tools` only, so a composite parent's `includes` were
invisible: a child of a `debugging` parent (terminal/process_manage +
includes web/file) asking for `file` or `web` was refused, and `safe` /
`hermes-gateway` parents could grant nothing but their own name. Same
root cause as the `_strip_blocked_tools` fix in the previous commit
(#111700, "Related" section).
Both sides of the subset check now use the resolved static surface
(`resolve_toolset(name, include_registry=False)`), so a child may request
any toolset whose real tools the parent genuinely holds, and still never
gains a tool the parent lacks. Candidates that resolve to nothing are not
expanded into (they cannot be a meaningful subset).
Co-authored-by: DresvyanskiyDenis <dresvyanskiydenis@gmail.com>
`_sanitize_node` deleted the `required` key whenever the pruned list came
out empty. Four built-in tools (skills_list, todo, delegate_task,
session_search) declare `required: []`, so they left the sanitizer with no
key at all. Strict OpenAI-compatible proxies read the missing key as
`null` and 400 the whole request ("null is not of type array"), which is
non-retryable and kills the session on its first call.
An empty array is valid for every backend; the pruning was added (34c3e67)
to drop names that are not in `properties`, not to delete the key. Keep the
key with the filtered list, even when that list is empty.
Fixes#111684Fixes#59386
Co-authored-by: Cr4ckMe <jiqing.liu@whu.edu.cn>
