* fix(process): a kill's result stands on every backend, and a failed scope stop never counts as stopped
Follow-up to #133434, which fixed these races for local processes only.
Non-local backends (spawn_via_env) settle through _env_poller_loop, which
still wrote the session's final state without the session lock:
- It set exit_code before _finish_exited, so a kill that landed first ended
as killed with the sandbox's exit code. The exit now goes through
_finish_exited alone, and a committed kill keeps its code.
- Its backend-lost path set exited/-1/lost directly, replacing a recorded
kill. It now goes through _finish_exited(reason="lost"), so the kill stands
and the completion is still published by its owner.
- A log delta (or a log-rotation reset) fetched before a kill was applied
after it, so the session's output no longer matched the kill's snapshot.
Both are now dropped once the session has exited, decided under the lock.
A failed systemd scope stop could still count as a successful stop:
- A recovered PID that died between get() and _signal_kill ignored the stop
result and returned a plain already_exited. It now reports
scope_stop_failed.
- kill_all counted already_exited as stopped even when the scope stop
failed, and only looked at _running, so a session that exited after being
picked kept a live scope that no later sweep retried. Every kill path now
stops the scope through _stop_scope, which marks a failed stop pending;
kill_all does not count a result with scope_stop_failed and also retries
finished sessions whose stop is pending.
_log_delta_command moves unchanged into tools/process_registry_env_log.py
(ProcessRegistry._log_delta_command stays as an alias) to keep
process_registry.py under its size cap.
Refs #132358. Both gaps were found by @andrexibiza in the review of #133434.
* fix(process): pruning keeps a session whose scope stop is pending
kill_all retries a failed scope stop through the finished session, but
_prune_if_needed still evicted that session by TTL (measured from process
start, so a long-running process was prunable the moment it finished) or
by capacity, leaving a possibly live scope with no retry target. Sessions
with _scope_stop_pending are now exempt from both until the stop succeeds;
capacity pruning drops the oldest prunable session instead.
Found by the independent review of #133648.
Third-party drivers (e.g. a Codex-style computer-use reimplementation) could
only replace cua-driver by patching core. computer_use now follows the
single-select provider model Hermes uses for memory providers, context
engines and cron schedulers:
- plugins/computer_use/__init__.py discovers bundled
plugins/computer_use/<name>/ then user $HERMES_HOME/plugins/<name>/
(marker: register_computer_use_provider / ComputerUseProvider) and loads
ONLY the provider computer_use.backend names (default "cua"), read per
call so each profile gets its own. Unselected providers are never
imported; listing reads plugin.yaml only. An unknown/unloadable name
returns an error naming it and the installed providers; no fallback.
- The built-in cua-driver is an ordinary bundled provider at
plugins/computer_use/cua/ (driver implementation stays in
tools/computer_use). tools/computer_use/backend.py gains
ComputerUseProvider (create_backend(permission_mode=...), is_available(),
optional doctor()).
- The tool's check_fn and backend construction go through the selected
provider; the model-facing schema is unchanged for every provider
(prompt-cache invariant). The test-only HERMES_COMPUTER_USE_BACKEND env
switch is gone (tests patch _new_backend).
- General plugin discovery treats computer-use providers as exclusive
(not eagerly imported, not listed as toggleable) and skips the bundled
computer_use/ category, same as memory/, context_engine/, cron_providers/.
- hermes tools and the Desktop toolset panel list installed providers next
to cua; computer-use doctor/status/permissions and the Desktop status
payload skip cua-driver/TCC probes when another provider is selected.
Plugins that drive the agent's browser below the browser tools (Secure ENV,
catalog review on #132272) had no public way to send a raw CDP command over a
task's supervisor connection, so they bumped CDPSupervisor._next_call_id,
inserted futures into _pending_calls and sent on _ws. That breaks silently on
any internal change and collides between plugins.
SUPERVISOR_REGISTRY.capture(task_id) now returns a CapturedCDP pinned to the
supervisor's current socket and default page session:
call(method, params=None, *, session_id=None, timeout=10.0) -> raw reply
is_valid() / page_session_id / task_id / cdp_url
The validity check and the send run in one supervisor-loop step, so a call
never reaches a newer connection: after a reconnect, stop or registry
replacement every call raises CapturedCDPInvalid. Calls in flight when the
socket drops now fail at once (the supervisor fails pending replies on
teardown) instead of sitting out their timeout.
No model tool, config default or new capability: in-process Python could
already reach this. The handle enforces no origin/consent/ownership policy.
Reduced-scope salvage of #132752 (API shape and names from that PR); its
attach-reply handoff, old-wire cleanup and dispatch fences are not needed
because the supervisor closes the old socket on every reconnect, which drops
its sessions with it.
Co-authored-by: web3blind <264741654+web3blind@users.noreply.github.com>
Screen finding on #131927: when QQ media delivery failed (stale local
path, daily upload quota, oversized file, or any upload/API error),
_send_qqbot's media path returned a bare error and the caption/message
text was never delivered — a regression against main, where QQ
text+media degraded to the text delivered plus a 'MEDIA attachments
were omitted' warning.
- collect per-attachment failures as warnings and keep delivering
(previously the first failed attachment aborted the whole send)
- when nothing was deliverable, fall back to sending the text or the
caption standalone, matching the pre-media generic path
- surface collected warnings on the final result (and on error returns)
Tests: missing-file, upload-quota and generic-delivery failure flows
assert the text still lands exactly once with a warning (red on the
PR head before this fix).
Introduced a new test suite for QQ Bot media handling, including file type detection and media routing. Enhanced the send_message_tool to support media uploads for QQ Bot, allowing for both direct media messages and text with media attachments. Updated error handling and added support for media delivery in C2C and group chats.
Review fixes for the generic-dir fallback:
- SkillsShSource.fetch now names a generic-dir bundle after the slug too, so a skills.sh install
that bypasses the index no longer lands in a shared skills/ dir.
- HermesIndexSource.fetch applies the slug name whether the row resolved via resolved_github_id
or fell back to the tree, so a rebuilt index does not create a second lock entry ('skills').
- The tree fallback only runs for skills.sh rows: a deleted concrete GitHub path stays unavailable
instead of installing whichever skill remains in that repo under the requested name.
- _find_skill_in_repo_tree's lone-SKILL.md fallback only accepts a generic dir (skills,
.agents/skills, .claude/skills); a lone named skill (skills/bar) no longer answers for slug foo.
- The PR's two tests move to tests/tools/test_skills_hub_generic_dir.py (test_skills_hub.py is at its
code-health line cap).
skills.sh rows in the hermes-index carry the skills.sh slug as `path` and,
since mid-September, never a `resolved_github_id`: the scheduled index build
resolves paths AFTER the ClawHub walk, ~80 min into the job, when the 1 h
GitHub App token has expired. Every tree fetch 401s and the log says
"Resolved 0/20000 paths" (last good run 2026-09-15: 17917/20000). Installs
then work only where the repo dir happens to equal the slug, and everything
else is reported as a "stale index entry".
- build_skills_index: resolve skills.sh paths straight after the skills.sh
crawl, while the token is fresh.
- HermesIndexSource.fetch: when the indexed candidates miss, locate the skill
in the repo tree (slug-named dir, a lone SKILL.md in a generic dir such as
`skills/`, or the repo root) and install it under the slug, not as
`skills/`.
- _find_skill_in_repo_tree: a tree with exactly one non-root SKILL.md
resolves to it when no dir matches the slug (fails closed on 2+).
- GitHubAuth: once the credential chain came up empty, stay anonymous for the
instance instead of re-running `gh auth token` before every API call; with
gh installed but not logged in that cost up to 5 s per request and pushed an
anonymous install past four minutes.
Fixes#130129
Based on #130135 by @dskwe.
The typography-folded fallback in _substring_matches rejected a needle only when
it was quotes/dashes at the ends (needle.strip("'-")), so old_text="\u201c \u2014"
(curly quote, space, em dash) folded to "' -" and matched any entry containing
`" -`, deleting it via single or batch remove. The fallback now requires at least
one alphanumeric character; exact-entry and byte-substring priority are unchanged.
Tests: the PR's four new tests are folded into two invariants (layout/typography
match + punctuation-only rejection; self-correcting failure copy + #90468 alias).
Fleet telemetry (14d, v0.21.5) shows the builtin store returning
success=False on 34-47% of add/replace/remove calls. Live `hermes chat -q`
runs against origin/main reproduced the dominant causes:
- Drift guard refused every replace/remove/batch on a file whose only
difference from the tool's own layout was whitespace (blank lines
around the delimiter, an empty entry). Parsing only strips whitespace,
so flushing such a file loses nothing; the round-trip signal could not
protect any content and turned the store read-only until hand-fixed
(#107270). The guard now fires only on the signal that can actually
lose foreign text: one entry larger than the whole store's limit.
- An over-budget add/replace/batch said "would exceed the limit" but not
by how much, so models retried with consolidations that were still too
big. Every over-budget message now states the exact chars to free and
points at the one-batch fix.
- An old_text that matched nothing (paraphrased, stale after a concurrent
write, or several entries glued with the delimiter) returned no hint on
the batch path. Zero-match errors now carry `closest_entries` (top word
overlap) on both paths, and flag a delimiter-spanning old_text.
- A batch replace missing its own `content` now says to set it on the
operation object.
Fixes#107270. Same diagnosis as #107278 (@KoNit-K) and #61612 (@webtecnica),
which normalized the round-trip comparison instead of dropping it.
Background-review (and human /memory) replaces re-emit entry text with
different quote characters (straight ' vs stored ") or re-wrapped lines, so
byte-exact old_text substring matching wrongly reported 'No entry matched'
and the staged consolidation write failed to apply. Fall back to a
quote/whitespace-normalized match only when no exact match exists, preserving
today's unambiguous behavior. Adds regression tests for the quote-style
replace and for unrelated old_text still being rejected.
PR #86642 documented the single-op coalescing as
`content = content or new_text`, but the implementation only fell back
when content was None. Runtimes that materialize an omitted optional
string field as "" (some providers fill optional schema fields rather
than omitting them) bypassed the alias, and replace/add calls carrying a
genuine new_text failed with 'content is required for replace action.'
(#90468).
Treat content == "" like None on the single-op alias path, matching the
coalescing the batch ops already use (op.get('content') or
op.get('new_text')). A non-empty content still wins when both are set.
Fixes#90468
GitHubAuth resolved the .env PAT first and latched it for the process; GitHub's
401 Bad credentials was returned to callers as a plain miss, so every
GitHub-backed install failed even when `gh auth login` worked or the repo was
public, and the CLI blamed a "stale index entry".
- GitHubSource._github_get: on a 401 for the credential it sent, mark it
rejected and replay the same request (caller headers kept) with the next
method, ending anonymous.
- GitHubAuth: skips rejected tokens/methods; records a label per rejection.
`gh auth token` now goes through git_credentials._gh_cli_token, which strips
GH_TOKEN/GITHUB_TOKEN so gh returns its keyring login instead of echoing
the dead env token back.
- _print_fetch_failure: names the rejected credential and never calls such a
failure a stale index entry.
Credit: @easonlao's #46402 proposed the same fallback against the pre-split
tools/skills_hub.py.
mcp 2.0 renamed ToolAnnotations.readOnlyHint to read_only_hint; camelCase
survives only as a pydantic serialization alias, which is not applied to
attribute access. _annotation_read_only_hint() read the camelCase attribute
off the SDK object, so on mcp 2.x the hint was always None and every tool --
including honestly annotated read-only ones -- fell into the
"unknown = write-capable" branch.
With trust: untrusted that means the approval gate fires before every read
call (get_*, search_*, list_*), which is exactly the failure mode that
trains a user to click through the prompt without reading it, weakening the
gate on the writes it exists for.
Use mcp_field() for the object path (the helper already used across the MCP
client for this 1.x -> 2.x rename) and accept snake_case in the cache dict
too, for caches written by another client. Unknown still means
write-capable; only the field lookup changes.
Regression test covers the SDK 2.x model, both snake_case shapes, the 1.x
camelCase attribute and a snake_case cache dict.
The child_env fixture still reset the allowlist via the removed
_allowed_env_vars_var ContextVar token, so every test using the fixture
(AttributeError at setup) failed across all Python/OS lanes once #90004's
process-wide set landed. Save/restore the set contents instead.
skill_view registers a skill's declared env vars (required_environment_
variables and the legacy prerequisites.env_vars) into the sandbox
passthrough allowlist. The allowlist was backed by a ContextVar, and
tool dispatch fans each tool call onto a worker whose context is a
copy_context() snapshot taken at submit time
(tools.thread_context.propagate_context_to_thread). The registration
therefore lived and died inside the skill_view worker's snapshot: the
submitting thread's context never saw it, and the next tool's snapshot
(execute_code, terminal) always started from an empty allowlist — so a
skill's env vars were never passed through, on any turn, in any session
(#90004). The config-based allowlist (terminal.env_passthrough) is a
module-level global and worked, which is why manually adding the var
there was the only workaround.
Store the skill-registered names in a process-wide set, matching the
config path's visibility. Cross-session exposure is limited to the
names — values still resolve per profile through
resolve_passthrough_value's secret_scope, so a name registered by one
session cannot read another profile's secret. clear_env_passthrough
keeps its reset semantics; a later skill_view re-registers on demand.
The grandchild in test_real_shared_group_child_descendants_are_killed
reparents to init the moment proc.kill() lands, and psutil's kill routes
through os.kill, which the guard only authorizes for the test process's
subtree — a walk that races with the reparenting. The blocked kill was
suppressed by the sweep's contextlib.suppress, so the grandchild survived
and the test flaked ~1.5% on the macOS CI lane (and 3/200 locally).
Signalling a reparented descendant is the exact contract under test, the
established pattern for live-signal tests; the flake is gone over 400
local full-file runs.
- process_registry._reap_untracked: delegate to _terminate_host_pid with the
session's recorded host_start_time instead of a raw killpg -- pgid == pid
only proves group leadership, and a reaped child recycled onto an unrelated
group leader would still have been signalled. The helper revalidates the
kernel start time and refuses on mismatch, and settles descendants from a
psutil snapshot so no group signal is needed at all.
- browser_tool_lifecycle._legacy_kill_process_tree: in the non-leader path,
snapshot descendants via psutil before proc.kill() and kill each
individually -- a bare proc.kill() left grandchildren holding the capture
pipe's write end, reintroducing the communicate() hang the helper exists
to prevent.
_kill_process_tree fallback and three latent killpg callers resolved
os.getpgid(child) and signalled the result unconditionally. A child that
shares the caller's group resolves to that group, so TERM+KILL took down
the whole Hermes tree; a reaped-and-recycled pid resolves to a foreign
leader's group. Signal the group only when pgid == pid, the rule
deadline.kill_process_tree and _subprocess_compat already use, and fall
back to signalling the direct child otherwise.
The per-call provider argument on text_to_speech (added via #73513,
closing #47459) let the model reroute speech to any configured backend,
contradicting the tool's own schema header ("Voice and provider are
user-configured ... not model-selected") and leaving compact channels
unable to keep a house voice without prompt-layer appendices.
- drop provider from the model-facing TTS_SCHEMA
- stop forwarding args["provider"] in the registry handler
- keep the function argument for internal/test callers, but a value
disagreeing with the configured tts.provider is now ignored with a
warning (both text_to_speech_tool and _text_to_speech_single)
Fixes#90109
When the heartbeat's stale verdict fires (frozen activity fingerprint for the
threshold cycles), await_child used to raise FuturesTimeoutError the moment the
wait ended if the worker future had not resolved — even though a child that
already wrote its final answer often lands its recorded result moments later
while its worker thread unwinds. That synthesized timeout abandoned the real
result: in an async batch the never-collected child left the unit unable to
satisfy 'all children terminal', so the batch stayed running for hours with no
completion delivered (#113222; the reporter's 7h hang, and the follow-up traces
where a task's final line sat uncollected for 23+ minutes).
Collect instead of abandoning: after the stale verdict, signal the cooperative
stop, then wait up to _STALE_RESULT_GRACE_SECONDS (2s) for the worker to
settle. The real result wins when it lands (status completed, real summary);
only a worker that never settles falls through to the existing structured
timeout with the deferred-close drain. A configured inactivity cap
(child_timeout_seconds) keeps its teeth — it is NOT a stale verdict, so the
#116001 abandonment contract is unchanged (pinned by test_delegate_liveness_timeout).
Steering stays open across the window: the success path's _merge_late_steer (or
the failure path) remains the linearization boundary, so a concurrent steer is
never lost.
Command STT providers received the raw audio file with no format
normalization: _transcribe_command_stt only checked .exists() before
substituting {input_path}. Desktop voice recordings arrive as WebM/Opus
48 kHz and providers with container/sample-rate contracts (e.g. Tencent
16k_zh) reject the raw webm, so desktop voice chat failed on every
message. _transcode_audio_for_stt (ffmpeg -> 16 kHz mono m4a) existed
but was wired only into the OpenAI path's rejection retry.
Add stt.providers.<name>.normalize (default off, so whisper-style CLIs
that accept any container keep the original file): when truthy, the
input is transcoded into the per-call temp dir and {input_path} points
at the normalized file. Transcode failure returns an error envelope
instead of silently feeding the raw container to a provider the user
declared needs normalization.
Fixes#81811
Co-authored-by: embwl0x <embwl0x@users.noreply.github.com>
Child exit, result finalization, notice publication and output consumption are
separate steps, but several callers treated an earlier one as the last:
- A duplicate _move_to_finished set _completion_event while the owning call was
still before its notice, so a one-shot parent could exit without the follow-up.
Only the call that moved the session out of _running releases waiters now, and
it always does, even when its publish raises.
- _finish_exited ran mark_exited without the session lock, so a kill committed
between its `killed` check and its writes was overwritten with exited/0.
- After a reconcile flips `exited` ahead of a reader still on its final drain,
wait/read_log/kill no longer consume the partial output as the completion,
and poll no longer marks it observed. poll/read_log/kill decide that under
the same lock hold as their snapshot, so a reader finishing in between cannot
make an earlier partial snapshot count as the whole output.
- kill_process ignored a failed systemd scope stop. It now returns an error
with scope_stop_failed whenever the stop fails, including when the root
exited meanwhile (that proves nothing about detached descendants); the
session keeps its unit so a later kill retries the stop. An already-exited
session reports scope_stop_failed too.
Process-tree termination and post-kill survivor checks move into
tools/process_registry_termination.py (ProcessTerminationMixin) to keep
process_registry.py under its size cap. The only change in the move:
_live_descendants logs the traceback of the failure it swallows.
Refs #132358. The races were found by @andrexibiza and @jonpol01 in that thread.
When poll/wait/list saw the direct child exit while the reader was still
behind, the reconcile asked the reader to finish and flipped the session to
exited at once. The reader then stopped after ONE more 4 KiB chunk, so the
rest of the pipe - up to a whole pipe buffer, the job's final lines - was
never read, and the caller had already been handed a snapshot without it
(wait() also marked that snapshot consumed, suppressing the completion).
After a finish request the reader now drains what is already buffered
(zero-timeout select, capped so a grandchild that floods the pipe still
cannot pin it), and the reconcile waits briefly for the reader to publish
before falling back to marking the exit itself.
(cherry picked from commit f8266be8ccc682482f18aaeeccb4d397b5310a9b)
_chat_run_agent registers the CLI sudo callback in -q too and the process
exports HERMES_INTERACTIVE=1, so a sudo command without SUDO_PASSWORD /
NOPASSWD filled a panel nothing draws and waited its 45 s deadline per call
(every kanban worker). Single-query is now headless for sudo like a
delegated child: _no_sudo_user() gates the prompt (_transform_sudo_command),
the SUDO_PASSWORD tip on a headless sudo failure (_handle_sudo_failure, now
worded for any session that cannot prompt), and the "you will be prompted
again" hint (terminal_tool_result.can_reprompt). A configured SUDO_PASSWORD
and the session cache still apply; an interactive `hermes chat -q` on a TTY
never sets HERMES_SINGLE_QUERY_SESSION, so it keeps its prompt. A /dev/tty
fallthrough was rejected: it prints the prompt box onto -Q / stream-json
stdout, and on Windows msvcrt blocks 45 s on a worker's hidden console.
The protected-instruction tests now pin the cron and webhook contexts the
shared helper added, and the single_query_mode case registers a callback
that would approve, so it cannot pass via the no-callback early return.
Cases duplicating test_file_write_safety.py are dropped, and the guard
comment states the rule instead of the incident.
`hermes chat -q`, cron and unattended platforms can all have the CLI panel
callback registered with nobody to answer it. The MCP/vault consent prompt
(#133229) and the vault unlock prompt each spelled out
single_query or cron or unattended_platform by hand; the protected-instruction
gate and the memory prompt (salvaged above) checked single-query only, so a
cron job or a webhook session could still wait the full approval timeout
there. tools.approval_context._no_user_can_answer() now answers it once and
every one of those call sites uses it.
The memory prompt's _interactive_approval_available() wrapper is folded into
that one check, and its import- and runtime-failure fallbacks (and their
parametrized test) go: _no_user_can_answer reads session env through
_session_env, which already swallows errors, and tools.approval_context
imports nothing from the memory/terminal side.
Every write to a protected agent-instruction file (SOUL.md, AGENTS.md, ...)
from a kanban worker failed with:
BLOCKED: ... approval prompt timed out without a user response.
Silence is not consent.
That message says a human ignored a prompt. No prompt was ever rendered.
Root cause: `HermesCLI.__init__` registers `_approval_callback` as the
thread-local CLI approval callback before it is known whether the run will
build a prompt_toolkit Application. One-shot `hermes chat -q` runs — exactly
how the kanban dispatcher spawns every worker — never build one. The callback
still looked present, so `_request_protected_instruction_approval` took the
"a CLI human is here" branch, pushed modal state into a layout nothing
renders, and blocked on a response queue only a key binding can fill. It
burned the full approvals timeout and returned "timeout".
`_run_approval_gate` learned about this context in #86878 via
`HERMES_SINGLE_QUERY_SESSION`. This gate deliberately bypasses that gate
(one-operation approval every time, no allowlist, no persistent consent, no
--yolo) and so never got the fix. Reuse the same detector rather than adding
a second mechanism: in a single-query session, skip the unreachable modal and
fail closed immediately with the honest "no interactive user or gateway is
present" reason.
Policy is unchanged and deliberately stricter than `_run_approval_gate`:
`approvals.single_query_mode: approve` does NOT auto-approve this gate. A
protected-instruction write always needs a live human. Interactive CLI users
can still approve, and the gateway path is untouched.
Measured on the reported case: pre-fix 30s+ then a bogus "timed out";
post-fix immediate with an accurate reason.
Regression coverage (tests/tools/test_protected_instruction_cli_channel.py),
verified red before this change and green after:
- interactive approve/deny through a registered callback,
- an end-to-end round trip through the real HermesCLI._approval_callback modal,
- a single-query session never dispatches to the unrenderable modal and never
reports "timed out",
- single_query_mode approve/deny/ask never auto-approves this gate.
Refs kanban t_9691f08d (defect surfaced by t_4e642903, 10 identical failures).
(cherry picked from commit 360d456b83250d9f348a9fc0f22e41b26ca65e15)
ef1faa4cf8 (#133162) passed the agent thread's panel callback to the
classic-CLI branch of request_elicitation_consent. `hermes chat -q`
registers that callback too, so an untrusted MCP tool or a vault card
fill in a -q (or cron) turn now waited the full approval timeout for a
panel nobody can answer. tests/e2e/core/mcp_plugins/
test_mcp_stdio_conformance.py's untrusted-server turn hit its 240 s limit,
which is red on every PR that runs the e2e lane.
Decline before prompting in single-query, cron and unattended-platform
sessions: the same three no-user contexts the dangerous-command gate
resolves without a prompt, and the gateway branch of this function
already skips. Interactive chat keeps the panel.
One observer pair fires wherever the agent blocks waiting on a person, so a
notifier plugin no longer needs a hook per prompt type or a `sudo` regex over
terminal commands (#132333, consumer: pushover-hermes-plugin).
Fire sites are the existing choke points every surface already routes through:
- sudo: tools/terminal_tool_sudo.py::_prompt_for_sudo_password (CLI callback,
Ink TUI/Desktop sudo.request card, /dev/tty fallback)
- clarify: tools/clarify_tool.py::clarify_tool (CLI, TUI/Desktop, gateway)
- approval: tools/approval_prompt.py::prompt_dangerous_approval (CLI/TUI/ACP
callback and input() paths), ::_present_with_selected_transport (plugin
transports), tools/approval_gateway_wait.py::_await_gateway_decision
(gateway platforms)
Payload: kind, request_id (shared by the pair), session_id, session_key,
platform, prompt; resolved adds outcome. prompt is always force-redacted and
the typed password / clarify answers are never passed. Smart (aux-LLM)
approvals and coalesced gateway followers do not fire it: no new human prompt.
Closes#132333
request_elicitation_consent's CLI branch called prompt_dangerous_approval
without the agent thread's approval callback. Under prompt_toolkit that
fails closed before any panel renders, so an untrusted MCP tool, MCP
sampling or a vault card fill was auto-cancelled in hermes chat without
the user ever being asked (and observers saw pre/post with no human wait).
Pass the registered panel callback, as the protected-write prompt does.
Replace the four salvaged tests with two invariants: each classic-CLI
prompt (protected agent-instruction write, MCP/vault consent) fires the
exact pre/post payload its gateway branch fires, differing only in
surface; and the no-human fail-closed branch fires nothing. Drop the
protected-write try/finally to match the dangerous-command CLI site.
The protected agent-instruction write prompt and the MCP/vault consent
prompt called prompt_dangerous_approval directly on the classic CLI
without firing pre_approval_request / post_approval_response, so
observer plugins (notifiers, audit loggers) could not see those human
waits in the terminal even though their gateway twins were visible.
Fire both hooks around the CLI prompt with the same payload shape the
gateway branch builds (pattern_key protected_instruction_file /
mcp_elicitation, surface="cli" or the caller's consent surface); the
post hook fires in a finally so the wait always settles for observers
even when the panel itself raises.
Also document the consent surface values (mcp-elicitation/<server>,
mcp-trust/<server>, vault-payment) and that the Ink TUI/Desktop report
surface="gateway" today.
Fixes#131876
Drop three pieces that change behaviour beyond plugin skills:
- _session_home_scope binding the full runtime scope (secret hydration, which
can run external secret CLIs, and terminal policy) on every catalog,
completion and dispatch RPC: a cross-cutting change for all callers.
- slash.exec moving plugin commands ahead of the skill/bundle gate: a separate
precedence question that predates plugin skills.
- commands.catalog filing every skill under a new Skills category.
Plugin skills still list, complete and dispatch under the existing home scope.
Include namespaced ctx.register_skill entries in CLI/TUI completion and
invocation while keeping native messaging command maps portable. Cover
compatible -s preloading and preserve disabled, platform, environment,
and profile gates.
Based on the qualified interactive-registry approach from #85725.
Co-authored-by: Konrad Stawiski <konrad.stawiski@umed.lodz.pl>
(cherry picked from commit cd3e30b5ceb5da9ef9daabc2458d0ea2e340da04)
test_live_dm_runner_retry_never_reexecutes_failed_claim claimed the queued
DM from a helper thread after a fixed 50 ms sleep. On a loaded runner
_run_delivery had not queued it yet, the claim returned None, the thread's
assert died silently, and _run_delivery waited forever on the mocked
still-live owner (it waits while the owner lives, by design since the
late-reply fix). The 600 s file timeout then failed the whole shard.
The helper now polls for the queued DM (10 s bound) and, if it never
appears, drops the mocked owner so the runner returns and the test fails
on an assertion instead of hanging.
Review follow-up on the bare stdio PATH fix (#125318):
- `_first_user_which_hit` reused `_npx_bin_candidates`, whose Windows
candidate list is the npx cache layout's `.cmd`/`.exe` pair. A user
install that only ships a `.bat`/`.py` wrapper (stock PATHEXT carries
both) was never found, so the scan returned None and silently fell
back to the managed hit — the #125300 symptom persisted for those
installs. Candidates now come from `_pathext_suffixes`: the child
env's PATHEXT first (shutil.which reads the PARENT's, same source
rationale as `_which_with_config_pathext`), then the parent's, then
the OS default; a command that already carries a PATHEXT suffix is
matched as written.
- The new `_bare_` test fixtures wrote extensionless executables, which
`shutil.which` can never resolve on win32 (PATHEXT suffixes only), so
the tests could only pass on POSIX and the Windows lane skipped the
marker-less file entirely. Fixtures are now platform-shaped via
`_bare_exe` (`python3.exe` on Windows, bare name + 0o755 elsewhere),
and a direct `_first_user_which_hit` unit test covers the
PATHEXT-driven candidate walk cross-platform.
Bootstrap prepends the managed runtime's bin dir to this process's PATH
(pm/environments.py activation), and the stdio resolver stopped at the
first PATH hit, so a bare `python3` MCP server resolved to the bundled
interpreter — which lacks the user's packages and dies on import, while
the agent log only said "Connection closed" (#125300).
- `_resolve_stdio_command` now steps past Hermes-managed bin dirs to
the user's own hit for bare commands; the explicit launcher family
(npx/npm/node/uv/uvx) keeps the managed-first resolution (that is
`_launcher_fallback`'s purpose), and a managed-only PATH keeps the
managed hit rather than failing
- a connect-failure log line now quotes the child's last stderr lines
from the shared mcp-stderr.log, so the actual import error is visible
where the failure is reported
The completed sibling's close() runs inside its worker before its future
resolves. Stopping the parent in that window found the future still pending
and reported the finished sibling as interrupted (CI run 37198702518).
