Call the existing keep-newest vision retirement on the per-call
api_messages clone after sanitization so OpenAI-style tool-result
screenshots are not re-uploaded on every later turn.
`_record_streamed_assistant_text` grew the turn's visible text with `+=`
on an attribute. Python only grows a string in place when the target is a
local variable, so this copied the whole text on every delta.
The loop runs once per streamed token, so a reply of length N costs about
N squared in copying. A 200 KB answer arriving in 4-character deltas
moves several billion characters and burns seconds of CPU in the loop the
file itself calls the hottest one in the agent.
The text is now kept as a list of pieces and joined when read. Reading
happens at turn end and on interrupt, not per delta, so the whole turn is
linear in the length of the reply. `_fire_stream_delta` used to join on
every token just to ask if the text was empty. That check now looks at
the parts list.
`_current_streamed_assistant_text` becomes a property over that list, so
the seven readers and the call sites that clear it between turns keep
working unchanged. Reading does not collapse the pieces, because a delta
landing between the join and the write back would be lost.
Measured with 8-character deltas: adding 20k deltas to an already long
text took 3.1 times as long as the first 20k before, and 0.9 times after.
Review follow-up, and a real platform bug. On Windows npm's `.bin` holds three
siblings per binary — the extensionless sh script, `<name>.cmd` and
`<name>.ps1`. Spawning the sh one from a Windows process fails, and
`os.access(X_OK)` there is effectively an existence check, so the previous
candidate test could not tell them apart: it would have picked a file that
cannot run and broken server startup that works today via npx.
Select by extension instead (`.cmd`, then `.exe`), the same precedence
hermes_constants._candidate_node_command_names already uses for npm/npx/node,
and fall back to npx when no launcher is present.
The platform branch moved into `_npx_bin_candidates(..., windows=...)` so it is
testable by injection. Patching `os.name` instead took pytest's own traceback
formatting down with an INTERNALERROR — a test that cannot report its own
failure is worse than no test.
Also from review: an `npx pkg -y` shape (flag AFTER the spec) now falls back to
npx, since those args are forwarded verbatim and would hand the server a flag
npx would have consumed. And the structural OSV-ordering guard reports a rename
explicitly instead of raising a bare ValueError that reads like a broken test.
`npx` resolves a package and then FORKS: it stays alive as the real MCP
server's parent for the whole process lifetime while doing no work. Measured
on a 4-agent host, that is ~48 MB of private memory per stdio MCP server —
and it buys nothing here, because Hermes already wraps the child in its own
parent-death watchdog, so npx's supervision is a second parent nobody reads.
The process tree for one server is watchdog -> `npm exec <pkg>` -> node.
When the package is already in npx's cache, spawn its binary directly and
drop the middle process. A cache miss changes nothing: the command stays
`npx`, so a cold machine still installs on first run.
The swap deliberately happens AFTER the OSV malware preflight.
`_infer_ecosystem` keys off the command basename being `npx`/`uvx`/`pipx`, so
rewriting first makes `check_package_for_malware` return None and silently
turns the gate into a no-op. Two tests pin that ordering — one behavioural,
one structural — because a future edit that moved the swap earlier would
disable the malware check without failing any test of either piece alone.
This is also why doing it in code beats the config workaround of pointing
`command` straight at the cached path: that loses the preflight.
Conservative by construction — falls back to npx for a version-pinned spec
(npx owns that resolution), an ambiguous or absent `bin` map, a missing or
non-executable binary, an unreadable cache entry, or no cache at all.
Measured on a Mac mini running four agent gateways plus a dashboard, ten
stdio MCP servers total: resident `npm exec` parents 5 -> 0, tracked
footprint 1531 MB -> 1324 MB across 30 -> 25 processes, free memory
464 MB -> 697 MB on a host that was actively swapping. Every MCP server kept
working; tool calls verified against a live Linear server afterwards.
Clear file-operation trackers when a terminal task ends and remove per-path
locks after the last holder or waiter exits. This prevents long-lived gateway
processes from retaining file state indefinitely while preserving sibling-write
tracking and path-level serialization.
Fixes#86514
test_no_locked_readers_gate.py (#97676) parses hermes_state.py's SessionDB
class body with ast and flags any method that holds the writer lock
around a pure-read query — Pattern C, where every concurrent turn's
persistence convoys behind an unrelated read. #97676 converted 39 such
methods and closed detection blind spots for alias/variable-SQL readers.
But SessionDB is declared as
`class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)`,
and the gate only ever opened hermes_state.py — it never parsed the three
mixin files those base classes are defined in, so a locked reader
declared there was structurally invisible to the scanner regardless of
how good the alias/variable-SQL detection got.
Applying the gate's exact scanning logic to the three mixin files
directly turns up 9 genuine pure-read methods still holding the writer
lock, none in #97676's converted list:
- hermes_state_search.py: _fts_teardown_trash_step, fts_optimize_available,
optimize_fts_storage, list_recent_user_messages
- hermes_state_portability.py: distinct_session_cwds, list_cron_job_runs,
_get_session_rich_rows_batch, list_skill_scaffolded_sessions,
get_first_assistant_text
_get_session_rich_rows_batch is a hot path: it backs list_sessions_rich's
compression-tip resolution and the web server's session-search hydration
across every gateway install — its own docstring already claims "same
read-your-writes guarantee as list_sessions_rich", but list_sessions_rich
was already using _read_ctx() (its guarantee comes from flush_token_counts()
before the read, not from holding the writer lock) while this method's
implementation never caught up to match.
Converted all 9 to `with self._read_ctx() as conn:`, the exact pattern
#97676 used, verified each is a genuine pure read with no hidden writes
by tracing every helper call it makes.
Extended the gate itself (_ALL_STATE_SOURCES) to scan all three mixin
files under their own class names, plus hermes_state.py, so this blind
spot can't silently reopen. Added a regression test
(test_scan_all_state_sources_visits_every_mixin_file) that plants a
synthetic violation in a mixin-shaped file and asserts the scanner still
catches it — a change that reverts the file list back to one file passes
the existing sabotage test but fails this one.
Mutation-verified: with the gate's new scope but the old (unconverted)
mixin sources, test_no_locked_pure_readers fails and names all 9 real
violations with correct file/line. Restored the fix; it passes clean.
The salvaged #95487 adds an expression index over sessions.last_activity_at.
Two web_server tests emulate a legacy store with ALTER TABLE ... DROP COLUMN
last_activity_at, which SQLite refuses while an index references the
column ('error in index ... after drop column'). Drop the index first —
the same adjustment the PR already made in test_schema_read_probe.py. A
real pre-column store has neither the column nor the index, so the healed
path under test is unchanged.
Preselect indexed recent candidates before rich hydration, cap and deduplicate compression lineage traversal, and interrupt sustained SQLite work through a cooperative progress deadline. Fail closed when the bounded browse API is unavailable and cover legacy-schema reconciliation plus malformed lineage cases.
The referenced WAL-index-lock docstring on _live_writer_holds_db was removed
by the extraction; describe hermes_state_holders.live_writer_holds_db instead.
Follow-up on the #97330 salvage (#97329): main grew `_canonical_sqlite_path`
after the PR's base; the holder module re-implemented it as a closure. Export
`canonical_sqlite_path` from hermes_state_holders and alias the legacy name in
hermes_state. Drop the unused `_read_proc_argv` / `_looks_like_hermes`
re-exports (no in-tree callers).
Move live-holder inspection into a bounded helper and make repair fail closed when a process still owns the state database, including deleted WAL/SHM descriptors and ambiguous procfs reads.
Preserve the contributor lineage from the original four-commit review train while presenting one coherent release object on current main.
Co-authored-by: fangliquanflq <fangliquan@qq.com>
Co-authored-by: ciabata <296402666+ciabata-git@users.noreply.github.com>
Each TUI instance spawned three stdio MCP server copies: one in the
CLI wrapper, one in tui_gateway.entry, one in the slash worker. The
wrapper's copy is dead weight — _launch_tui blocks in subprocess.call
until the TUI exits, so its registered MCP tools are never invoked,
yet the server process (35-85 MB) lives for the whole session.
Root cause: _is_tui_chat_launch() only detected --tui / HERMES_TUI=1,
so bare `hermes` with display.interface: tui fell through to
background MCP discovery in the wrapper while the TUI gateway
(spawned moments later) ran a second discovery.
Fix: _is_tui_chat_launch() now consults _resolve_use_tui() — the exact
TUI-vs-classic decision cmd_chat makes — for chat commands only
(command in {None, "chat"}), leaving mcp serve / gateway / acp / cron
discovery behavior untouched.
Verified: unit tests (RED->GREEN); E2E with a canary stdio MCP server
in a scratch HERMES_HOME counted 2 spawned copies pre-fix vs 1
post-fix (gateway's only), and the wrapper's RSS dropped ~43 MB.
Related: #71928 (same per-process duplication class), #11115 (lazy
non-core discovery).
Follow-up on the #101437 salvage (#101415). `_own_live_lease_ids()` snapshots
the server's session records under `_sessions_lock`, but the registry file
lock is taken afterwards, and a sibling session attaches its lease to its
record only after `try_acquire_active_session` has already written the entry.
A finalize racing that acquire would read the brand-new lease as an orphan
and drop it. Entries this process wrote inside a 30 s grace window are kept
regardless of the snapshot; real orphans are minutes old.
Follow-ups on the #101406 salvage (#101391):
- The issue's measured incident was a healthy connect followed by silent
polling loss; nothing republished for 11 h. `_schedule_polling_recovery`
(the single entry to the recovery ladder) now publishes "retrying" while
the adapter is running; `_record_polling_progress` already flips it back.
- `BasePlatformAdapter.send_path_degraded` property (default False) replaces
the runner's `getattr(adapter, "_send_path_degraded")` reach into a
plugin-private attribute; Telegram overrides it. `_mark_connected()` reads
the property instead of taking a `degraded=` kwarg, and `_mark_degraded()`
+ `DEGRADED_STATUS_MESSAGE` replace three copies of the same write/string.
- The startup connect stamp (sibling of the reconnect stamp) honours the
same flag.
- Tests: mid-session death publishes retrying; pre-connect death does not;
progress never flips while not running / fatal; runner reconnect stamp
honours the adapter flag.
connect() intentionally returns True when Telegram polling starts in
degraded mode (or a reconnect's require_progress=False skips the strict
readiness gate), so background recovery can retry without failing
gateway startup. But _mark_connected() published platform_state
"connected" unconditionally, and the reconnect watcher in gateway/run.py
stamped "connected" again right after -- so gateway_state.json was
indistinguishable from a healthy adapter for as long as recovery took
(observed ~11h on one seat).
_send_path_degraded already tracks exactly this (set at polling
generation start, cleared on the first confirmed getUpdates round-trip),
so:
- BasePlatformAdapter._mark_connected() takes a `degraded` flag and
publishes "retrying" instead of "connected" when set.
- TelegramAdapter passes its current _send_path_degraded into
_mark_connected().
- The gateway/run.py reconnect watcher checks the same flag before
overwriting the adapter's own status write.
- _record_polling_progress() republishes "connected" the moment
polling actually proves progress, so the state does not stay wedged
at "retrying" until the next disconnect.
Fixes#101391
_sanitize_ws_text now delegates to agent.message_sanitization._sanitize_surrogates
(the same U+FFFD replacement gateway/run.py, oneshot.py, and the chat helpers
use) instead of a second encode/decode round-trip implementation.
Starlette encodes send_text as UTF-8. A lone UTF-16 surrogate in a
status payload used to UnicodeEncodeError and latch the whole Desktop
connection closed. Sanitize the frame and skip only that send.
Final-diff review measured the worst text the 4 MiB walk budget still
admitted at ~20 s of shlex (cost is linear in admitted bytes, ~7 lex passes
per line, so the line cap was not the binding knob). 1 MiB brings it to
~4 s while a 200-script wrapper graph (~5 KB) and a ~1 MB build script are
still allowed.
lifecycle_scan_root_within_budget: state that it uses a fresh budget,
independent of the full guard's walk (review point on #83821).
- one _capped_read_limit() for local and remote reads (the file's history
is local/remote divergence bugs: #76762, #77703)
- line cap 256 KiB -> 64 KiB: shlex is quadratic on a single token, so the
worst text the budget still admits drops from ~60 s to ~17 s of lexing;
real shell lines are far below either
- fix a comment that described an encode the code does not do; trim the
header comment to the non-obvious WHY
The referenced-script walk in cron/lifecycle_guard.py capped each file
(1 MiB) and the recursion depth (8) but not the walk: a command referencing
hundreds of scripts, or one enormous shlex token, held the GIL for minutes
on every gateway terminal call (#78398).
Add a per-walk _LifecycleScanBudget (bytes, lines, longest line, unique
paths, remote reads) charged BEFORE any text reaches shlex, and cap each
referenced read at the remaining byte budget so an oversized file is never
read whole. Exhaustion fails closed (the existing contract for one oversized
file) and is logged at WARNING so operators can tell it from a genuine
lifecycle block. Limits are sized so real wrapper graphs never hit them:
a 200-script benign graph is allowed and a restart hidden behind it is
still caught.
tools/terminal_tool.py gates its optional launchctl pre-scan (which also
tokenizes) on the same budget; the full guard still runs afterwards.
Redesigned from #83821 by @Riccardo-Vecchi, which introduced the budget
idea but blocked benign wide graphs (64-path cap) and bundled a suffix
classification change that is left out here.
Refs #78398
Follow-up on the #42278 salvage (#98222): `a && b & &>/dev/null c` is valid
bash but rewrote to `a && { b & } &>/dev/null c`, which binds the redirect to
the brace group and orphans `c`. Treat a suffix starting with `&>` as command
text and insert the `;` separator. Adds bash -n coverage for that shape, for
the `;;` case-arm terminator (must NOT gain a separator), a multi-line mixed
form, and separator idempotence.
`_rewrite_compound_background` rewrites `A && B &` into `A && { B & }` to
avoid the subshell-wait process leak. When another statement follows the
backgrounded compound on the same line (`A && B & C`), the trailing `&`
was the only separator between the compound and `C`. The rewrite consumed
that `&` into the brace group and produced `A && { B & } C`. A brace group
must be terminated by `;`, `&`, `|`, a newline, or `)`/`}` before the next
command, so the result is a bash syntax error and the entire command fails
to run — neither `A`, `B`, nor `C` execute. The rewrite is applied by
default to every foreground command, so a valid command is silently
mangled into one that errors out.
This restores a `;` separator after the closing `}` when the suffix
resumes with command text. Only spaces and tabs are stripped before the
check; a newline already terminates the group, and an existing separator
(`;`, `&`, `|`, `)`, `}`) is left untouched, so previously-correct rewrites
are unchanged.
## What does this PR do?
Fixes a rewrite in `_rewrite_compound_background` that turned a valid
foreground command of the form `A && B & C` into the bash syntax error
`A && { B & } C`, causing the whole command to fail. A `;` is now inserted
after the brace group whenever a further statement follows on the same
line, while leaving commands that already end in a separator/newline
untouched.
## Related Issue
N/A
## Type of Change
- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
- [ ] ✨ New feature (non-breaking change that adds functionality)
- [ ] 🔒 Security fix
- [ ] 📝 Documentation update
- [ ] ✅ Tests (adding or improving test coverage)
- [ ] ♻️ Refactor (no behavior change)
- [ ] 🎯 New skill (bundled or hub)
## Changes Made
- `tools/terminal_tool.py`: in `_rewrite_compound_background`, insert a `;`
after the rewritten `{ ... & }` brace group when the trailing suffix
begins with command text rather than a separator/terminator.
- `tests/tools/test_terminal_compound_background.py`: add
`TestTrailingStatementSeparator` for the string-level rewrites and
`TestRewriteIsValidBash`, which runs the rewriter output through
`bash -n` for parse validity plus one end-to-end execution check.
## How to Test
1. Run `pytest tests/tools/test_terminal_compound_background.py -q`.
2. Before the fix, `_rewrite_compound_background("echo hi && sleep 5 & echo done")`
returns `echo hi && { sleep 5 & } echo done`; `bash -n -c` on that string
exits 2 with `syntax error near unexpected token 'echo'`.
3. After the fix it returns `echo hi && { sleep 5 & } ; echo done`, which
parses and runs, and the trailing statement executes.
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains only changes related to this fix
- [x] I've run `pytest tests/tools/test_terminal_compound_background.py -q` (50 passed)
- [x] I've added tests for my changes
- [x] I've tested on my platform: macOS 15.5
### Documentation & Housekeeping
- [x] I've updated relevant documentation — N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` — N/A
- [x] I've considered cross-platform impact: the rewrite is
platform-independent; the `bash -n` test is skipped when bash is absent
- [x] I've updated tool descriptions/schemas — N/A
The plausibility gate excluded stub rows by a hard-coded title literal that
session_lost_and_found builds inline at two sites; rewording either would have
turned every stub into a mis-mapped row. One STUB_TITLE_PREFIX constant.
Follow-ups on the #101423 salvage (#101409):
- `stub_missing_parent_sessions` legitimately writes `started_at = 0.0` when
no timestamped message survived; a salvage where only stubs remain is
depleted, not mis-mapped. Stub rows leave the sessions denominator.
- Reuse `_EPOCH_LOW` from session_lost_and_found instead of a second
2001-epoch constant; collapse the two per-table blocks into one loop.
- Tests: a real upgraded-layout source (started_at physically appended) with
page 1 zeroed goes through the real `.recover` lane and the report comes
back `verified: False`; a stub-only output is not flagged; a mapped row
with a NULL title (the mis-mapped shape) still counts as mapped.
The lost_and_found salvage lane maps cells positionally onto the
destination template's declared column order, but a source upgraded
via ALTER TABLE has its columns in the order they were added, which
differs from SCHEMA_SQL whenever a column was inserted mid-definition
(#101409). Every row still inserts, so integrity/FK/FTS/count checks
stay green and the report ends up verified: true — while all 1,875
sessions in the reporter's DB got started_at = 0.0 with counters and
URLs shifted into the wrong columns.
Add a semantic plausibility gate to the salvage lane: when every
salvaged sessions.started_at or messages.timestamp is NULL or before
2001-09, the cells were mapped onto the wrong columns and the
recovery is reported with errors and healthy: false, so verified no
longer claims a mis-mapped output. A partially damaged column (torn
cells on some rows) does not trip the gate — only a systematic
violation does.
Fixing the positional mapping itself needs a historical-layouts
table derived from schema-version history; that design decision is
left to maintainers (suggested fix 2 in the issue).
Follow-ups on the #101586 salvage (#101356):
- `_is_same_auth_store` uses `Path.samefile` instead of a hand-rolled
st_dev/st_ino compare.
- The same-store check now runs after the mtime fingerprint short-circuit and
records the clean mark, so a symlinked-profile process pays the resolve +
stat pair once per file change instead of on every `load_pool()` call.
- A profile `.anthropic_oauth.json` aliased to root's singleton is one shared
grant too; the singleton block no longer self-compares and unlinks it.
The forked-grant heal treated a profile auth.json that is an *alias* of the
root store (symlink, hardlink, bind-mount) as a forked copy: it loaded the
same file as both the profile store and the root store, matched every OAuth
row against itself, stripped the profile rows / providers.<id> block, and
_save_auth_store() wrote that strip back through the alias — deleting the
shared credential (openai-codex reported).
#100339 / PR #100929 fixed *copied* stores; a shared store has no other side
to consolidate. Guard the heal with _is_same_auth_store(), which reuses the
resolving _same_path() (covers symlinks and equivalent paths) and falls back
to device+inode identity (covers hardlinks), and return None without writing.
Regression tests cover an openai-codex pool row plus providers block behind a
symlinked and a hardlinked profile auth.json; the copied-fork heals from
#100339 are unchanged.
Fixes#101356
A search_files hit inside a serialized dump (multi-MB single-line JSON,
minified bundle) made rg/grep emit the entire matched line into stdout:
head -n counts lines, so a 40MB match line crossed the exec transport
untruncated and was buffered whole into Python before the per-match
[:500] clamp ran. Measured on main: 42MB transport payload and ~180MB
peak Python allocation for a single match.
Fix at the engine layer, all three pipelines:
- rg: --max-columns 2000 --max-columns-preview (preview keeps the match
visible instead of omitting it)
- grep fallback + darwin pruned-grep fallback: | cut -c1-2000
- files_only/count modes skipped (lines are paths/counts, never giant)
2000 cols exceeds the existing 500-char content clamp, so no
previously-visible content changes.
Adapted from cline/cline#13525 (search_codebase RangeError OOM crash on
giant single-line files) — hermes buffers in Python rather than a JS
string, so the failure mode is memory/transport blowup rather than an
uncaughtException, but the class is identical.
content and structuredContent are now alternatives — never both forwarded
to the model. Spec-following servers render their data into content (the
verbatim dual-emit SHOULD or a faithful reorganisation), so forwarding
both sent the same information twice per tool call. structuredContent
fills in only when content blocks render effectively empty (whitespace-only
text counts as empty), preserving structuredContent-only servers. _meta
passthrough is unchanged.
Also surfaces unsupported content-block drops to the MODEL as
'[MCP content dropped: unsupported block (...)]' notices carrying
type/mime/uri/size handles (kimi-code#3227) instead of a log-only warning;
drop notices do not count as usable content for the arbitration.
Live E2E: real stdio MCP server (mcp 2.x structured_output tool) through
register_mcp_servers + _make_tool_handler — origin/main forwarded the
payload twice, branch forwards content only; text-only path unchanged.
/simplify-code pass on the salvage stack. The hand-rolled module-global
probe cache ignored its binary argument (stale verdict survives a binary
swap) and logged per launch; switch to the repo's established
functools.lru_cache capability-probe pattern (cua_backend, browser_tool),
probe stdout+stderr, share the --http-cache-dir literal via a module
constant, tighten the probe timeout to 3s, and cover the probe's
except branch in tests. Mutation-checked: gate tests fail with the
gate removed and with the probe hardcoded True.
Salvage follow-up for PR #100269. The flag landed upstream in 0.3.x;
binaries before it (e.g. 0.2.8, verified locally) fatally reject the
flag with 'unknown argument', breaking every Browser Use launch.
Probe 'lightpanda help' once per process and omit the flag when the
binary predates it. Also soften the unverified concurrency claim in
the _http_cache_dir docstring and tell docs readers to stop sessions
before deleting the live sqlite cache.
Lightpanda's HTTP cache is opt-in (`--http-cache-dir`, off by default), and
the launcher never passed it, so every Browser Use navigation re-fetched
every asset.
Point all Hermes-spawned instances at one shared cache under
$HERMES_HOME/cache/browser-use/lightpanda/http-cache. Sharing it across
sessions keeps assets warm through session churn; Lightpanda stores it in
sqlite (WAL), so a write that loses a race degrades to a cache miss rather
than a failed load, and --http-cache-entry-limit (default 1000) bounds the
directory without Hermes managing eviction.
Measured over 25 navigations across 5 sites, median warm navigation drops
from 0.40s to 0.18s on news.ycombinator.com and 0.14s to 0.10s on
wikipedia; total navigation time 7.0s -> 5.9s.
The flag has existed since Lightpanda 0.3.x (April 2026), so this needs no
minimum-version bump.
With servers configured but none selected by -t/--toolsets, discovery still
paid the ~260ms mcp SDK import before discovering it had nothing to spawn.
Filter first; the test now asserts the SDK probe is not reached.
Royalaid