Same class as the XLSX rPh fix: a phonetic guide annotates the base text and
is not part of it. `_extract_docx` collected every w:t in the paragraph, so a
ruby-annotated run rendered as guide + base ("トウキョウ東京"). Skip the w:rt
subtree; w:rubyBase stays. The phonetic test now covers XLSX shared/inline/rich
and DOCX ruby through the registered read_file handler.
Follow-up to the two salvaged commits. The nudge text asserted the card was
"still `running`" and offered only kanban_complete / kanban_block, so even
when it fired legitimately it steered a review-bound card toward a false
completion. It now states what the transcript actually shows (no terminal
board call yet) and lists kanban_complete / kanban_request_review /
kanban_block, plus the reviewer exits.
Every remaining copy of the "terminal tools" knowledge is brought in line:
turn_stop_gates docstring + diagnostic status, goals.py finalize comment,
kanban_db_dispatch grace/concurrency comments, the orchestrator-only
refusal in kanban_tools, and the kanban / tutorial / codex-runtime docs.
Fixes#114598
Salvage follow-up to the cherry-picked #114529 (@Roblmvp):
- `hermes kanban block --kind dependency` now says the card was blocked as
needs_input because no parent is open, and the triage verdict keys off the
landed block_kind (a re-kinded dependency block is a human question).
- kanban_block tool: report the landed kind + requested_kind + a note telling
the worker why it did not park in todo (dropped the redundant rekind_reason
echo — it lives in the event payload).
- Tests trimmed to two invariants in test_kanban_block_kinds.py (terminal
parents -> blocked/needs_input -> triage at BLOCK_RECURRENCE_LIMIT; open
parent -> todo with no recurrence across a dispatch tick) plus one tool
surface test.
- Docs: kanban_block row, `blocked`/`dependency_wait` event rows.
- contributors/emails mapping for the salvaged author.
Fixes#114627
A worker can call kanban_block(kind=dependency) with no incomplete parent.
That used to park the card in todo as dependency_wait, so the next dispatch
tick promoted and spawned a fresh worker with none of the prior context.
When every parent is already terminal, record the block as needs_input
(sticky until a human unblocks) and tell the worker why. Real fan-in with
an open parent still waits in todo and auto-resumes when the parent finishes.
Follow-up to the salvaged fix from #114734 (@liuhao1024), widening it to the
whole class behind #114727:
- DashboardOAuthFlow.mark_error: first reason wins. Waking the callback
waiter makes the worker fail with a derived error and its own mark_error
used to overwrite flow.error, so the dashboard/Desktop poll showed the
generic "did not include an authorization code" instead of the cause
(e.g. "OAuth cancelled by user", a pre-redirect DCR failure).
- wait_for_callback fall-through names the server, status and recorded
error instead of the fixed no-code sentence.
- exception_message(): str(exc) or the type name; used at every
mark_error call site (tui_gateway/mcp_oauth_sessions.py worker and
start_flow, hermes_cli/web_server_mcp.py, hermes_cli/web_routers/mcp.py)
so a bare TimeoutError()/RuntimeError() no longer records "".
- start_flow no longer stamps "Timed out waiting for MCP authorization URL"
over a worker failure that happened before the URL was published.
Tests trimmed to two invariants (cause reaches the waiter and is never
clobbered; blank exception text stays diagnosable).
mark_error() wrote self.error but never the _callback_error channel the
SDK's callback waiter reads, so any failure marked before a browser
redirect (worker crash, authorization-URL timeout, user cancel) woke the
waiter with no callback and no error and surfaced as the generic
'OAuth callback did not include an authorization code' RuntimeError —
the real cause stayed unread. Copy the message (with an empty-message
fallback for bare TimeoutError-style exceptions) into _callback_error,
guarded so a late mark_error cannot override an already delivered
callback.
Fixes#114727
Same class as the Bot Chat drain wedge already on this branch: every JSON-file
scan guarded "did it parse?" and then assumed the value was a dict. A file
holding `42`, `"oops"` or `[1,2,3]` (corruption, truncated write, foreign tool)
passed the guard and raised AttributeError/TypeError at the first `.get()`,
usually before a single healthy sibling was processed. Each site now treats a
non-object payload like a corrupt file under that subsystem's existing policy:
- tools/bot_relay.py::_expire_if_stale / claim_pending_envelopes — the
envelope is skipped by the sweep and not claimed (same as unparseable).
- tools/browser_lightpanda.py::reap_orphaned_lightpanda — record unlinked,
scan continues.
- tools/write_approval.py::list_pending / get_pending — record skipped with
the existing "unreadable pending record" warning / None.
- tui_gateway/methods_session.py::_legacy_spawn_tree_entry / spawn_tree.load —
scalar snapshot reads as empty / returns the existing 5000 error instead of
violating the SpawnTreeLoadResult contract.
- hermes_cli/local_runtime/binaries.py::manifest_verified — False.
- plugins/platforms/a2a/protocol.py::load_conversation — non-dict lines are
dropped, keeping the declared list[dict] return.
- batch_runner.py::_load_dataset / _scan_completed_prompts_by_content /
_combine_batch_files — line skipped and counted as filtered.
- trajectory_compressor.py::process_entry_async — scalar entry passed through
unchanged.
Ported from the source hunks of PR #114241; its gateway/shutdown_flush.py
drain_transcript_spool hunk is left to open PR #84785, and its
recover_pending_to_db / cron / bot_live_delivery / bot_mode_dm hunks are
already on this branch or on main.
(cherry picked from commit d4b54568887e69b3ee3d363ebe4dcd657ccf64f9)
Follow-up to the previous commit: rather than an isinstance check in every
caller (defer, deliver_to_live_owner, complete_delivery, the scheduler's
receipt comparison, bot_mode_dm's admit/wait), the two exact-id readers —
cron/bot_chat_delivery.py::read_pending and tools/bot_live_delivery.py::_read
— now raise ValueError for a payload that parses but is not a JSON object,
the same way they already propagate a PermissionError: a possibly-live
receipt is never overwritten, and the bulk-scan wrapper _scan_read turns the
same ValueError into its existing warn-once-and-skip path, so one malformed
ticket no longer wedges _next_sequence or claim_pending_delivery (#109820's
rule for the live-delivery dir).
Co-authored-by: beardthelion <beardthelion@users.noreply.github.com>
`hermes doctor` and `hermes setup terminal` only ever looked for a `docker` binary
(`_safe_which("docker")` / `shutil.which("docker")`) and probed `docker version` by
literal name, so a podman-only machine was told Docker was missing while the docker
terminal backend was already running containers through Podman — the same mis-report
the dashboard probe had.
Both now resolve the CLI through `find_docker()` (HERMES_DOCKER_BINARY → docker →
podman → macOS Docker Desktop paths) and name the runtime they actually found, via
`docker_runtime_name()` and `docker_runtime_start_hint()` next to `find_docker()` —
shared with the dashboard probe instead of a per-module copy. The probe's version call
goes through the backend's `run_capture()`, and a Podman row no longer advises starting
a daemon: Podman is daemonless, so it points at `podman machine start`.
Tests that pinned the old resolution (`_safe_which` / the global `shutil.which`) now pin
`find_docker()`, so they no longer depend on whether the host has Docker Desktop at a
known macOS path.
_refresh_tools called self.session.list_tools unguarded. run() resets
self.session to None on every transport teardown (clean reconnect,
error backoff, park, cancel) and the parked-probe path does the same,
none of it under _rpc_lock or _refresh_lock. A tools/list_changed
refresh scheduled on the dying transport therefore wakes up behind the
lock mid-restart and crashes the background task:
ERROR tools.mcp_tool: MCP server 'treadmill': dynamic tool refresh failed
AttributeError: 'NoneType' object has no attribute 'list_tools'
once per profile that includes the server, on every gateway restart with
a slow-to-connect MCP server.
Snapshot the session only once _rpc_lock is held and return at debug
level when it is None. The snapshot sits inside the RPC lock because
that is the point at which the refresh has actually won the right to
talk to the transport; a check before acquiring it can still observe a
session that teardown nulls while the refresh waits. Skipping is
correct, not a loss: the reconnect's own discovery re-lists and
re-registers tools, and the next tools/list_changed re-arms the refresh
against the live session. The previous registration stays intact rather
than being nuked mid-restart.
Co-authored-by: Tranquil-Flow <66773372+Tranquil-Flow@users.noreply.github.com>
Co-authored-by: ildunari <ildunari@users.noreply.github.com>
Co-authored-by: Andrex Ibiza <84248988+andrexibiza@users.noreply.github.com>
Co-authored-by: ly6751 <liuyu890412@gmail.com>
Squash of the 54 commits on victor-kyriazakos:feat/user-channel-warning-suppression
(PR #112302, head f45c640e55) so the contributor's authorship survives a rebase-merge;
the commits interleave with a cron delivery-ledger rework that the salvage removes in
follow-up commits, so per-commit cherry-picks were not practical.
Adds display.suppress_warning_notifications (global + per-platform, default false):
one resolver (gateway/warning_notifications.py), BasePlatformAdapter.emit_warning /
emit_media_warning / warning_text, a notification_category classification carried
through wakes, queues and persistence, and render/present boundaries for CLI/TUI.
Replace the string-presence snippet test and the file-content e2e with two
behaviour contracts: the real bash dump must drop every gateway-bridged name
(gateway.session_context._VAR_MAP) plus HERMES_DELEGATED_CHILD_CONTEXT while
keeping ordinary exports — this is the test that would have caught the
HERMES_CRON_SESSION drift (in the regex contract, missing from the unset list)
that was live on main — and a real LocalEnvironment run where a delegated
child still sees its marker and the next ordinary command does not. Add the
marker to the regex contract and cut the WHAT comment down to the WHY.
Co-authored-by: PRATHAMESH75 <prathamesh290504@gmail.com>
_export_dump_excluding_session_vars unsets the session bridged vars,
the attribution markers, and HERMES_UI_SESSION_ID before export -p, but
not HERMES_DELEGATED_CHILD_CONTEXT or HERMES_CRON_SESSION. Both are
scope-limited: scrub_kanban_env() injects the delegated-child marker
into the SUBPROCESS env while a delegate_task child is live, and the
cron session bridge tracks the owning cron scope. A snapshot captured
in that window persists them — the marker outlives the child whose
ContextVar was correctly reset on exit, and every later `source` of the
snapshot re-asserts it. In the parent session this locks out all kanban
mutations ("delegate_task child contexts cannot mutate Kanban tasks or
boards") even though no child is running (#90782).
Add both names to the unset list in the dump snippet. Regression tests
assert the snippet unsets them and that an end-to-end command run with
the marker set leaves no trace of it in the snapshot file.
Why: for an old WebSocket client that never advertised server→client requests,
send_async already failed fast (on_result(None)) but _emit_approval_request ignored
it, so the approval wait — owned by tools.approval's queue, not server_requests —
still idled for the whole approvals.timeout (300s) with no prompt anywhere. The
same held for a -32601 error frame. on_result(None) now withdraws the queue entry
(withdraw_gateway_approval: cancelled cause, never a user deny) so
_await_gateway_decision returns at once; the agent sees a withdrawn prompt.
resolve_gateway_approval committed entry.result/reason/event.set() AFTER
releasing _lock, so _drop_entry (which reads result and leaves the queue under
the lock) could still pop-and-lose a choice the client was acked for. Every
committer (resolve, clear_session, unregister_gateway_notify) now commits inside
the same critical section that pops the entry, which is what _drop_entry's
docstring promised.
_drop_entry mapped a withdrawn wait to settle("set") — the raw poll-state token
went out as the request.cancel reason. It is now session_closed, and a choice
from another surface is resolved (both RequestCancelReason values).
Tests: restore test_server_request_error_response_fails_fast (an error frame
settles send() to None promptly); the approval fail-fast probe from the review
(silent WS peer, timeout 3 -> was 3.2s, now immediate); a lock-instrumented
resolve test; a settle-reason test. The test_protocol `server` fixture imports
server_requests before its sys.modules patch window so the module server.py binds
its sinks on is the one tests import — the PR's fail-fast test only passed in a
full-file run before (order dependency).
Part of #112548
Item 2 of #112548: a Desktop/dashboard build that predates server→client
requests has no response path, so every clarify/approval/sudo/secret/vault/
connection/bridge request sat for the full deadline (clarify: 300s). Only the
tour probed. Clients now advertise once per connection
(`client.capabilities {server_requests: true}`, sent by the shared TypeScript
channel on `gateway.ready`); `send()` / `send_async()` return the
error-response shape (None) at once when every WebSocket peer of the session
is a build that never advertised. Sessions with no client attached still wait
so the reconnect replay (`open_requests`) keeps working; the stdio TUI ships
with the backend and is not gated. The advertisement is dropped on disconnect.
Reviewer minors from #113227:
- tools/approval_gateway_wait.py: the verdict is the choice committed under
the approval lock while leaving the queue, so an /approve that lands after
the deadline check but before the entry is dropped is an answer, not a
timeout (the client was already acked "ok").
- tests/tui_gateway/test_protocol.py: the error-fails-fast test that only
restated pre-existing behaviour is replaced by the two capability
invariants (never advertised → fails fast; advertised → frame written,
waits, forgotten on disconnect).
- server_requests.send try/finally around event.wait already landed on main
(4371ed34a9); nothing to change.
Docs: programmatic-integration.md (advertise once per connection; method
list), tui_gateway/AGENTS.md; contracts regenerated.
repeat_refusal checked the counter and record_embed incremented it in separate
lock sections, so a concurrent tool batch on the same image (the incident issued
4 at once) all passed a check taken before any of them recorded and the cap of 3
let 6 embeds through. Reserve the slot inside the check's lock section and
release it when the embed then fails.
User-visible knobs need a home: the Vision feature page explains why native embeds ride
the session and what each key does (subagent-only default cap, clamp range), the
configuration reference points at it next to auxiliary.vision so the two sections are
not confused, cli-config.yaml.example carries the commented block, and tools/AGENTS.md
names vision_tools_history_budget.py as the single owner of embed-cost policy.
A delegate_task child asked to transcribe five screenshots called vision_analyze 158
times on those files (full loads alternating with region crops) until its provider
quota ran out; every native load bakes the image into history and is re-sent on each
later API call, and nothing refused the repeat (#112095).
tools/vision_tools_history_budget.py now keeps a per-(session id, resolved source)
embed counter. Region crops share their file's key. When the cap is reached
_vision_analyze_native returns a tool error that says the image has already been loaded
N times and to answer from what is visible or ask the user, instead of another embed.
Only successful embeds count, so a failed read never eats the budget.
Default: `vision.max_calls_per_image` unset caps only delegated subagents
(agent.delegation_context.is_delegated_child_process_context) at 3 — they run
unattended and CLI /steer targets the parent — while the main agent stays unlimited;
an explicit value applies everywhere and 0 means unlimited.
Co-authored-by: Evi Nova <66773372+Tranquil-Flow@users.noreply.github.com>
Co-authored-by: gumclaw <gumclaw@gumroad.com>
The native vision_analyze fast path (and the browser screenshot twins) downscaled every
embed to a fixed _EMBED_TARGET_BYTES = 256 KB. That is fine for photos but turns a
1080x2340 phone screenshot of a table into 540x1170, which the model then reads as
"unreadable" and re-requests (#112095).
The budget is now `vision.embed_target_bytes` in config.yaml (default unchanged: 256 KB,
clamped to 64 KiB..4 MiB so one setting cannot make every later request a multi-megabyte
resend), resolved in the new topical sibling tools/vision_tools_history_budget.py and read
by vision_analyze, browser_vision and browser_exec screenshots alike.
Ported from #112947 by @MohamadKanso (resolver + clamp), relocated out of the facade.
The live pass on a stock home counted 13 new WARNINGs per process: every
unconfigured core-gated tool (browser, image_gen, HA) warned on its first
probe. The actionable case is a core tool that was available earlier in
this process and then got dropped; never-configured stays INFO.
Issue #112649 atom 4: a core (`_HERMES_CORE_TOOLS`) tool whose check_fn returns
False leaves neither the schema nor the tool_search catalog (it is not
deferrable), so the model's "no such tool" is accurate and nothing in the log
points at the probe. ae5666f7fc made the False verdict INFO for optional,
unconfigured toolsets — that stays; the core-tool drop is now a WARNING that
names the dropped tool(s), once per probe per process (the 30 s TTL re-probe
would otherwise re-warn every turn), reset by invalidate_check_fn_cache().
Review minor: `catalog_provider: deepsek` was accepted silently and the typo
leaked into ModelInfo.provider_id. An alias that is neither a Hermes provider
id nor a models.dev id (loaded catalog, no network) now warns once, mirroring
the unknown-key warning, and the row keeps its own slug.
Adds the real-process invariant behind the parent-first teardown salvaged from
#111603 (#111598): a bash supervisor that reaps its own child on SIGTERM must
receive the only SIGTERM and exit 0, with the child never signalled by the
registry. Red on origin/main (the child logged a registry TERM before the parent
and the parent's own kill found no such process); green on the salvaged head.
Documents the teardown order in tools/AGENTS.md, including why scope teardown
(_stop_systemd_unit) never precedes the PID signal for a live parent — the
issue's cgroup-split concern (Electron migrating only the browser PID into its
own scope) cannot make the registry stop the scope first.
The reconnecting exemption keyed only on `_ever_connected`, so a server that
connected once and then parked on a PERMANENT error (revoked credentials,
endpoint gone: `_park_and_rearm('from parked state (permanent error)')`) looked
identical to a router-reboot park. Its self-probe fails the same way every
interval, so the job ran tool-less on every tick forever with only a gateway
WARNING and never received the one-shot blocked_config alert it used to get.
Record the revival reason `_park` was given on MCPServerTask (`_park_reason`,
cleared when a session proves healthy) and have `mcp_server_reconnecting`
return False for a permanent-error park, restoring the block and its alert.
Transient parks (network blip, rapid-drop budget exhausted) still return True.
Also dedupe the exemption's WARNING per job+server for the length of one
outage, like the one-shot blocked_config alert, instead of logging every tick;
the entry drops once the server resolves tools again so the next outage warns.
A cron job naming an MCP server in its enabled_toolsets was hard-blocked
(blocked_config, one-shot alert, no LLM call) whenever that server resolved
to zero tools — including the minute a router reboot or DNS blip left an
otherwise healthy server parked and self-probing. The preflight could not
tell "the network blinked" from "wrong name / other profile's server", and
the job lost a whole tick to a transient outage the MCP layer was already
recovering from on its own (#112871).
The MCP run task keeps that distinction: a server that connected once in
this process (_ever_connected) and is currently sessionless with a live
task is degraded/parked and will revive. tools.mcp_tool_discovery grows a
read-only mcp_server_reconnecting(name) predicate on that state (resolved
through the profile's own or adopted connection key, never connecting),
and _empty_requested_mcp_toolsets skips such servers with a WARNING that
names them — the job runs with whatever tools did resolve, as the reporter
asked. A server that never connected for this profile keeps the block,
which is the misconfiguration case #109050 built the check for. The
blocked reason now says "never connected for this profile" so the
remaining block reads as what it is.
Docs: the pre-dispatch validation section lists the MCP check and the
reconnecting exemption.
The no-keepalive proof branch ran on ANY timeout wake. With idle_timeout_seconds=60 the wait is min(180, 60); an overlapping RPC hides the recycle deadline so _recycle_if_due() is False and the session was marked proven after 60 s, clearing the #62212 rapid-drop budget early. Fix a proof_at deadline before the loop and gate the branch on it; the unproven timeout is the remaining distance to proof_at. Tests: the stdio test now shows the first (idle-limit) wake does not prove; the interval test covers an explicit keepalive_interval on stdio.
WHAT: `_wait_for_lifecycle_event` hoists `is_http = self._is_http()` (it only
reads the immutable `"url" in self._config`) and reads `keepalive_interval`
from the config once instead of `in` + `.get()`. The one-shot RPC-idle waiter
is now spawned on `not is_http and self._rpc_lock.locked()` alone, dropping
the inline `_idle_timeout_seconds is not None or _max_lifetime_seconds is not
None` clause. The waiter list is built by one comprehension per iteration
(rpc_idle_task changes between iterations) and the `finally` reuses the last
value; `_cancel_waiters` already skips done tasks. The docstring folds the
#17003 keepalive paragraph into the first paragraph instead of describing the
keepalive twice.
WHY: the dropped clause was the inverse of
`MCPServerHealthMixin._stdio_recycle_deadlines` re-derived by reaching into
health-mixin attributes. Without it, a stdio server with no limits and an
active RPC spawns one trivially-completing waiter per RPC; on lock release
the loop takes one extra iteration where `_recycle_if_due()` is False and
`_next_stdio_recycle_deadline()` is None, so it simply recomputes the timeout
and waits again — harmless.
After the stdio keepalive default became None, the lifecycle loop's
`if keepalive_interval is None: continue` skipped the only idle path to
`_mark_session_proven()`. `_session_proven` is otherwise set only on a
tool-call success or a suspect health-check pass, and each new session
resets it, so an idle default-config stdio server never became proven:
every later child death/reconnect charged the rapid-drop budget that a
180 s idle survival used to clear (#62212 regression).
While the session is unproven, keep the `asyncio.wait` timeout at
`_DEFAULT_KEEPALIVE_INTERVAL` for stdio too; when it fires with the child
still alive, mark the session proven without pinging. Once proven the
stdio timeout returns to None, so a healthy idle pipe is never probed.
Shutdown/reconnect precedence and the rpc_idle waiter are untouched.
Test: `_captured_interval` now runs two lifecycle cycles (first times
out, second shuts down) through a shared helper so the stdio test asserts
timeouts [180, None], zero pings and `_session_proven` set, instead of
copy-pasting the fake `asyncio.wait`. Docstring/comment updated to the
new rule.
The test trim dropped the headline case: text written over an EXISTING .db. Sidecar paths return at is_sqlite_sidecar before the overwrite branch, so a mutant that drops has_binary_extension from the overwrite condition survived every remaining test. Add a third parametrize case that targets the held-open WAL database file itself and asserts the binary refusal with bytes unchanged.
Same commit: the _check_binary_document_write docstring now names SQLite sidecars (always) and every BINARY_EXTENSIONS suffix (on overwrite), and the suffix is computed once at the top instead of three times.
_has_extension_in and is_sqlite_sidecar each re-implemented the same
rfind('.') / -1 / .lower() suffix extraction, and the write guard used a
third spelling (filepath[filepath.rfind("."):]) for its refusal messages
next to os.path.splitext in the overwrite branch. Add _lower_suffix()
("" when no dot) and route both predicates through it; the write guard
now uses os.path.splitext for every message's displayed extension.
_SQLITE_EXTENSIONS was `{.db,.sqlite,.sqlite3,.db3} & BINARY_EXTENSIONS`,
which silently dropped .db3 (not a binary extension anywhere in the
codebase, so `x.db3-wal` was never a sidecar). Spell the set as the three
members that actually take effect and assert the subset invariant instead
of hiding it behind an intersection. Behaviour is unchanged.
file_operations.py checked `os.path.splitext(path)[1].lower() in
BINARY_EXTENSIONS` at five sites, so a `.db-wal` / `.sqlite3-shm` sidecar
(final suffix never in the set) was read and edited as text in the sandbox
paths. Route all five through has_binary_extension(), which strips the
sidecar marker. Behaviour is otherwise identical: same lower-cased final
suffix membership (rfind vs splitext differ only on leading-dot basenames
like `.bashrc`, which are in neither set). binary_extensions imports nothing
from tools, so no cycle.
The refactor that moved sidecar detection into binary_extensions dropped the
contributor's unconditional refusal for the db family and made every binary
extension a plain overwrite guard. That is right for the MAIN .db/.sqlite
(text fixtures named *.db exist), but a -wal/-shm/-journal path is never a
legitimate text target: a checkpointed database has no sidecar on disk, so
write_file("kanban.db-wal", text) silently dropped a garbage WAL next to a
live database.
Add is_sqlite_sidecar(path) and refuse such paths in
_check_binary_document_write regardless of is_file(). Scope the marker
stripping in _has_extension_in to SQLite suffixes so report.docx-wal no
longer counts as an opaque document. Hoist the duplicated is_pdf_path call.
Parametrize the write_file WAL test over sidecar existing/absent.
Move the .db-wal/-shm/-journal detection from a write-guard-local regex
into tools/binary_extensions._has_extension_in so has_binary_extension
(read guard AND write guard) agree: a sidecar counts as its database's
extension. read_file now refuses sidecars instead of returning lossy
text, which also gives write_file the no-baseline overwrite refusal.
Behaviour change vs the contributor's commit: creating a NEW .db /
.sqlite file via write_file stays allowed (main allows it and text
fixtures named *.db exist) — only overwriting an existing binary is
refused. The generic has_binary_extension overwrite branch is kept and
merged with the PDF branch because patch has no full-read baseline
check: on main, patch on an existing .db with a matching old_string
rewrites the header in place.
Tests folded into the existing guard test classes (one write_file, one
patch refusal on a real WAL sidecar); the PR's 12-test file and its
private-regex assertions are dropped.
The text tools' read->modify->write round-trip re-encodes binary bytes
lossily: the terminal transport decodes stdout with errors=replace, so
a patch on a SQLite database reads mojibake and writes it back, silently
destroying the file (observed in the wild: a kanban board DB lost 550k
bytes / 26 days of history this way on 2026-09-15).
read_file already refuses to display binary files (extension + byte
sniffing), but write_file/patch had no write-side equivalent, and
patch_replace reads via plain cat so the read-side guard never fires
for it.
Guard rules (extends _check_binary_document_write, the existing
binary-document write guard):
- .db/.sqlite/.sqlite3 and their -wal/-shm/-journal sidecars: ALWAYS
refused (even new-file creation) — plain text can never be a valid
database, mirroring the opaque-document rule. The sidecars need a
basename regex because os.path.splitext('x.db-wal') yields '.db-wal',
which is not in BINARY_EXTENSIONS.
- Other BINARY_EXTENSIONS (.png, .zip, ...): refused when OVERWRITING an
existing file, mirroring the existing .pdf-overwrite rule — the model
can only have read mojibake, so writing back destroys it. Creating a
NEW file with such an extension stays allowed.
Tests: new test_binary_db_write_guard.py covers the regex (main names,
sidecars, non-db names), the guard function, and the write_file/patch
tool paths against real SQLite files (integrity_check still ok, bytes
untouched). 29 new+existing guard tests pass; file-tools/patch suites
green (196 tests).
The per-action schema made the delete branch `additionalProperties: false`
with no `absorbed_into`, so a schema-validating or grammar-constrained
backend could no longer emit the curator's consolidation delete and
`_curator_consolidation_delete_guard` fail-closed every consolidation.
Advertise `absorbed_into` on the delete branch and cover the batch path
that forwards it to the guard.
Also fold the two remaining patch shape checks (missing new_string,
content mixed with old_string/new_string) into `_op_shape_error`, so a
batch rejects them before applying any sibling instead of creating op[0]
and rolling it back. Drop the stale `edit` vocabulary from skills.md:440
and the curator prompt, and move the schema-diet test helpers above the
`__main__` guard.
The operations[] item schema was one flat object with four coexisting text
slots (content / new_string / file_content / file_path). A 27B local model
that had just used write_file's file_content kept emitting it on create and
patch ops; the call validated against the advertised schema, the handler
failed on "content is required", and the whole batch rolled back — eight
identical retries until the tool-loop guardrail tripped (#112677).
- items is now an anyOf of self-contained per-action op objects (create,
patch targeted, patch full-rewrite, write_file, remove_file, delete), each
with additionalProperties: false. The wire shape of a correct call is
unchanged (still a flat op with name/action/...), so transcripts, staging
and replay are untouched; grammar-constrained backends can no longer emit
another action's slot, and schema-validating providers reject it up front.
Nested (non-top-level) anyOf survives every sanitizer (schema_sanitizer,
Gemini legacy translator). Cost: parameters JSON 1352 -> 2414 bytes.
- _validate_batch_ops runs the per-op argument-shape check (_op_shape_error)
before any op is applied, so a misfiled op[1] no longer applies op[0] and
then rolls the batch back; the error carries the same misplaced-key hint.
- The hint is attached to argument-shape misses only: a patch whose real
problem is an unmatched old_string is no longer told to "move that text to
'content' (full rewrite)", the escape the patch error itself warns against.
- Shape tables (_REQUIRED_ARGS, text-slot maps, _misplaced_text_hint,
_op_shape_error) move out of the facade into skill_manager_batch.py, the
op-validation sibling; _patch_skill shares the old_string guidance text.
- Docs: skills.md Actions table states the one-slot-per-action contract.
_command_detection_variants yielded one FULL-LENGTH variant per quoted or
escaped command word. A heredoc body of quoted lines ("key": "value", …) is
hundreds of quoted command words, so both detection passes scanned
O(words * len) characters: on a 16 KB / 460-line command the hardline pass
alone took 7.8 s and the dangerous pass 15 s even with the launchctl lookahead
anchored (33 KB: 36 s hardline), all while holding the GIL on the gateway loop.
Build a single variant with every command word deobfuscated instead. The
obfuscation catch ($(echo rm), r''m, ${0/x/r}m …) is unchanged — the same
deobfuscated words appear, in one string — and the variant count on the
issue's input drops from 923 to 4 (36 s -> 0.26 s hardline, verdict
unchanged). detect_hardline_command needs no bound and keeps its YOLO
semantics: a benign heredoc is neither blocked nor prompted.
Root cause identified in #113943 by @Tranquil-Flow (variant explosion as the
second compounding factor); its cumulative-work budget is superseded by
removing the explosion.
Co-authored-by: Tranquil-Flow <66773372+Tranquil-Flow@users.noreply.github.com>
Plugin dependency installs need two things the shared installer ladder did not
expose: a constraints file (so a plugin can never move a core pin) and a dry run
(so a conflict can be detected before anything is written). Both are threaded
through _venv_pip_install; the durable-target path keeps its own core
constraints and a dry run skips syspath activation and bytecode warming.
The Desktop pool caps local `hermes serve` children and holds each child's
slot lease for its lifetime. Its renderer refreshes lastActiveAt every 60s
for every open socket, so a bot-tile-pinned resident is keepalive-fresh
forever: occupied is not busy, and the pool cannot tell the difference. The
renderer's own turn bookkeeping cannot see cron fires (HERMES_DESKTOP=1 runs
the in-process ticker), messaging turns served by a pooled backend, or a
session blocked on an approval, so it is not a safe proof either.
Add `GET /api/health/idle` (token-gated; whether a turn is running is
activity recon, unlike the public liveness route) returning
`idle: true|false|null` from `hermes_cli/web_server_idle_proof.py`. It
reuses the SSH idle-exit primitive `turn_in_flight()` (running gateway
sessions + running cron jobs) and adds the human-input ledgers: open
server->client requests (`server_requests.open_request_count`) and queued
gateway approvals (`approval.pending_gateway_approval_count`). Any ledger
that cannot be read yields `null`, which the Desktop treats as busy.
Tests: unit invariants over the fail-closed table and the real ledgers, and
a live test that boots three desktop-shaped children (HERMES_DESKTOP=1,
per-child HERMES_HOME, port 0), holds one busy in the cron running-job
ledger, and probes all three over HTTP. RED on base: every child 404s.
Folds the backend half of #113396 (#89720) into this branch so there is ONE
friendly-name resolver: `_display_name` reads the Bot Mode title, then the
profile.yaml `display_name` written by `hermes profile rename` (the Desktop's
botFriendlyNames order), then the @handle — so the renamed primary signs
`Maia (@hermes)` instead of `hermes (@hermes)`. Reaching it by `maia` /
`@maia` already works through `_resolve_local_name`, which stays the single
resolver (fail-closed on ambiguity, punctuated names accepted).
`local_alias_map` docstring now states what the code does: an exact folder
id wins by design; a friendly name colliding with another folder id never
steals it, and ambiguity is only between friendly names.
Docs: the Direct-messages bullet lists the accepted target forms (profile
name, friendly name, Desktop @-slug) and the fail-closed ambiguity rule.
Review follow-up for #113410 / #113396.
message_agent keyed local targets on the profile FOLDER id only, while the Desktop
autocompletes the bot's friendly name (profile.yaml display_name, or the Bot Mode
title) as its @-slug. "@scribe" for folder `writer`, or "Dr. Foo" for folder `foo`,
came back "No teammate named …" (a punctuated name was even "Invalid target"), and
the model went hunting for the teammate under ~/.hermes/profiles/ instead.
- tools/bot_mode_probe.py: alias_forms() mirrors the Desktop's mentionNameForms
(slug + collapsed, reserved tokens dropped); local_alias_map() builds
form → {folder ids} from every local profile's display_name / title.
- tools/bot_mode_dm.py::_resolve_local_name: exact folder id first, then a unique
alias; two bots sharing a friendly name fail closed (never the one that sorts
first). The handle-shape check no longer rejects a friendly name that resolves.
- The protocol roster line leads with a display_name that differs from the folder
id and title, so an untagged "talk to Scribe" maps to `@writer` from the prompt.
Closes#100671
ensure_message_agent_tool() returned True on the schema-present branch without
re-adding message_agent to agent.valid_tool_names. A long-lived Bot Chat whose
tool surface was rebuilt (compaction / MCP refresh republishes the allowlist from
the registry snapshot, which never contains the injected tool) then advertised
message_agent while the executor rejected every call, and the model fell back to
hermes -p shellouts that time out. Success now means both halves hold.
Re-applied onto the current any()-shaped gate; semantic change and test are the
contributor's (#96109).
Closes#96105
A stray profiles/default/ directory shadowed the reserved root-home entry when
the roster was collapsed into a dict (last-key-wins), sending bot-mode DMs to a
lifeless profiles/default/state.db. Skip the reserved name in the scan, matching
the container_boot precedent.
Fixes#108564
_should_skip_container_guards documented fail-soft (unknown or raising
backend keeps the guards on) but only provider_flag's inner lookups were
covered: a raising registry.get_provider propagated out of the approval
predicate instead of defaulting to guards-on. Wrap the lookup so any
exception means "not isolated enough" and the dangerous-command prompts
stay in place.
Review follow-up on #113257.
