The pre-send normalization pass re-canonicalized every historical tool
call's argument JSON on every API-call iteration — quadratic in session
tool-call count. Route it through a bounded value-keyed memo (the
_MSG_TOKENS_CACHE idiom from agent/model_metadata.py): per-iteration
cost is now proportional to new tool calls, not all of them.
Measured (simulated growing session, repo venv): session-total
canonicalization cost 1056 ms -> 58 ms at 500 tool calls x 2 KB args
(18.3x), 5744 ms -> 79 ms at 500 x 16 KB (72.5x). Byte-parity with the
pre-fix logic is asserted at every iteration (unicode, nested,
malformed, empty, non-string args), and a call-count test proves
json.loads invocations went from K(K+1)/2 to K per session.
Review follow-up on the #76142 salvage: MemoryStore path-resolves and
shares one process-wide connection per file, so MemoryStore(":memory:")
creates a literal ./:memory: FILE whose state leaks across test runs —
the second run of the file failed all three spy tests because the
NULL-vector test had permanently wiped hrr_vector in the leaked db.
tmp_path isolates each run; verified two consecutive runs green + full
tests/plugins/memory/ green.
FactRetriever.search() re-encoded the query vector once per candidate,
related() re-encoded both role atoms once per fact row, and probe()
re-encoded the role-content atom once per row. All three encoders are
deterministic (SHA-256 counter blocks), so the hoisted vectors are
bit-identical to the per-iteration values they replace.
Measured (300-fact store, dim=1024, median of 30 calls): search()
11.62 -> 1.46 ms/call (8.0x; encode_text 30 -> 1 per call), related()
63.08 -> 16.17 ms/call (3.9x; encode_atom 601 -> 3 per call), probe()
431.93 -> 389.36 ms/call (1.1x; dominated by per-fact content encoding,
which is inherent to the algorithm and unchanged).
Tests: call-count regression tests for each hoist plus a bit-exact
parity test of search() against the pre-fix per-candidate loop.
Every ORDER BY id query on the messages table sorted or scanned the
whole session: get_messages_around's window seek, latest_message_row_id
(LIMIT 1), and get_messages' full-load ordering all paid O(session
history) per call — hot mid-turn via session_search and reactions.
messages.id is an original column (INTEGER PRIMARY KEY AUTOINCREMENT),
so the index lives in SCHEMA_SQL next to idx_messages_session — no
legacy-column migration hazard (the kanban lesson from #28776 does not
apply).
Measured (real schema, one 20k-message session, median of 30):
get_messages_around 7.08 -> 0.22 ms (32x), latest_message_row_id
3.37 -> 0.011 ms (307x), get_messages full load 111.6 -> 98.6 ms
(1.13x — remaining cost is row deserialization, not the sort).
Window results byte-identical at probe points across the session.
Tests: VM-step pin (get_messages_around bounded work, calibrated
~12 vs ~855 handler calls, threshold 300 — fails without the index)
and window parity with/without the index. No EXPLAIN/plan text
(behavior contracts, AGENTS.md).
Review follow-ups on the #76287 salvage:
- scheduler.py no longer calls advance_next_run after the batch switch;
keeping the import invites a future test to patch the wrong seam.
- advance_next_run returns >= 1 instead of == 1 so a corrupted jobs file
with duplicate ids (both records advanced by the batch) still reports
True after advancing and saving.
The scheduler's pre-dispatch loop called advance_next_run per due job —
one full load_jobs() + one full save_jobs() of the jobs file each — so
N due jobs cost N reads + N writes of the whole file (gateway-restart
catch-up or co-scheduled bursts). advance_next_runs() does one load +
at most one save for the whole due set with identical per-job semantics;
advance_next_run() is now a thin wrapper over it.
Measured (50 due recurring jobs, real jobs file): 107.9 ms -> 2.5 ms
(45x; 50 loads + 50 saves -> 1 + 1).
Tests: batch advances recurring and skips one-shots, single load + save
I/O pin (fails pre-fix — no such function), no save when nothing
advances, and per-job wrapper semantics unchanged. Related: #60946 and
#75833 both restructure this loop's call site for correctness — neither
addresses the I/O cost, and this batch primitive composes with either
dispatch design; happy to rebase onto whichever lands first.
Review follow-ups on the #76357 salvage:
- _message_reference_from_ids + _reply_reference_for_send collapse the
3x duplicated MessageReference construction (naming mirrors telegram's
_reply_to_message_id_for_send).
- The overflow elif's comment claimed PartialMessage has no to_reference;
discord.py 2.7.1's PartialMessage does (message.py L1901) — the branch
is belt-and-suspenders for duck-typed priors, now labeled as such.
Every reply paid one extra Discord API round trip: the text send path,
the voice send path, and the edit path each called fetch_message() just
to obtain a reference or an editable handle. Discord resolves
message_reference payloads from ids alone, and PartialMessage.edit()
works without a fetch — so build MessageReference directly (with
fail_if_not_exists=False, preserving the deleted-target behavior the
existing send-side 10008 retry already covered) and use
channel.get_partial_message() for edits. Overflow continuations keep
threading via an ids-built reference fallback for PartialMessage.
Measured by call count (deterministic): reply sends and edits now make
ZERO fetch_message calls where they made 1 per reply and 1 per edit
(including every streaming edit tick).
Tests: pin that first-mode replies construct the reference without any
fetch, deleted-target retry test updated to assert fetch await_count==0
(retry now happens purely send-side), overflow/edit mocks retargeted
from fetch_message to get_partial_message (any fetch regression breaks
all five). Note: 4 discord-suite failures are pre-existing ordering
flakes — identical with the change stashed on clean main.
Architecture fix for the bug class behind the Termux --version NameError
(live on main since eb4040242): version-printing kept being reimplemented
as *_fast() copies at the top of hermes_cli/main.py, each duplicating
canonical logic (project-root resolution, container detection, profile
detection). The copies drift silently — eb4040242 edited the canonical
output and referenced the PROJECT_ROOT module constant inside the fast
function, which doesn't exist yet at the fast exit point.
- hermes_cli/_startup_fast.py: THE implementations, stdlib-only. main.py's
*_fast() names become thin delegates (kept for test/back-compat), and
PROJECT_ROOT itself derives from the same helper — the constant and the
fast path can no longer disagree.
- Fast output now includes the .install_method stamp (one cheap file read)
and a 'Run hermes version for update status' pointer, so globalizing the
fast path doesn't silently drop slow-path info.
- Guard tests: (1) import-weight — subprocess-imports _startup_fast and
fails if any heavy module (config/yaml/argparse/cli/run_agent/httpx)
lands in sys.modules; (2) subprocess parity on+off Termux — the test
that would have caught eb4040242 the day it landed; (3) install-method
stamp surfacing.
hermes --version: ~3.8s cold / 0.2-0.4s warm -> 0.01-0.02s everywhere.
- _print_fast_version_info referenced the PROJECT_ROOT module constant,
which is defined AFTER the ultrafast exit point. On current main this
is a LIVE latent bug: the Termux fast path NameErrors on --version
(eb4040242 changed the print to use PROJECT_ROOT without noticing the
constant doesn't exist yet on that path). Compute the root locally.
- PR tests asserted the old 'Project:' label; main renamed it to
'Install directory:' (eb4040242). Expectations updated.
Exact parity with utils.atomic_replace: its target fsync is wrapped in
try/except OSError. A failed fsync after a successful copy must not
surface the already-completed write as an error (Windows can raise on
fsync of a read-only handle).
- agent_import.dump_yaml_file now calls utils.atomic_yaml_write instead
of hand-rolling safe_dump + atomic_write_text — same temp+fsync+atomic
rename and symlink preservation, plus mode/owner preservation a
0600-secured config.yaml needs
- openclaw script: the EXDEV/EBUSY copy fallback gains copystat + target
fsync so the docstring's 'mirrors utils.atomic_replace' durability
claim is true on cross-device deployments
- trim load_yaml_file's docstring to the behavior contract
The inlined temp-file + os.replace in openclaw_to_hermes.dump_yaml_file
replaced a symlinked config.yaml with a regular file, silently detaching
managed deployments that symlink ~/.hermes/config.yaml into a dotfiles repo or
profile package. The bare path.write_text it replaced followed the link, and
utils.atomic_replace -- which the hermes_cli twin reaches through
atomic_write_text -- resolves the link for exactly this reason (#16743).
Mirror that here: resolve the symlink before creating the temp file so the
rename lands on the real file, and fall back to copyfile on EXDEV/EBUSY now
that the target can live on another device. Covered by a regression test that
fails when the resolution is removed.
Also guard the permission-denied test for Windows: os.geteuid does not exist
there and chmod-based denial is unreliable, so skip on non-POSIX.
agent_import.py carries a private load_yaml_file/dump_yaml_file pair that
returned {} for an absent file AND for a present file it could not read or
parse. Three importers -- import_permission_allowlist, import_permission_denylist
and import_mcp_servers -- read config.yaml through it, merge one section into
the result, and write the whole mapping straight back. So a YAML syntax error,
a permission problem or a broken mount meant the importer replaced every
setting the user had with only the one to three keys it merged, and still
reported the item as "imported". The write was a bare path.write_text(), so an
interrupted import truncated the file instead.
Distinguish the two cases at the read. Absent, or present but empty, still
yields {} so first-time creation works. Present but unreadable, unparseable, or
not a mapping raises ConfigReadError; the three sites funnel through a new
load_target_config() that records the refusal as a per-item error and leaves the
file byte-identical. Dry-run refuses too, rather than previewing an "imported"
that would destroy the config. dump_yaml_file now writes through
utils.atomic_write_text, which the module already imports and uses for the
memory store.
This is the invariant hermes_cli/config.py already enforces for its own writers
via require_readable_config_before_write / atomic_config_write, whose docstring
names this exact root cause and calls itself "the single chokepoint every
config-update path should use". agent_import.py has its own helper pair and so
was never covered; it was the last config.yaml writer without the guard.
The identical helper pair lives in openclaw_to_hermes.py, the script this module
was ported from, where twelve config.yaml read-modify-write sites share the same
defect; fixed there too. Its refusal is recorded at the run_if_selected dispatch
point, which flips the existing _config_apply_blocked flag so the remaining
config-mutating options short-circuit instead of each rediscovering the same
unreadable file. The atomic write is inlined with tempfile + os.replace because
that script runs standalone with only the stdlib on its path.
kill_started_since duplicated kill_all's collect-under-lock/kill-outside-lock
loop line for line; it is now a thin delegate through new kill_all kwargs
(exclude_ids, source, consume_output). Public signatures unchanged — existing
callers and test monkeypatch seams keep working. kill_process's docstring now
names the deliberate consume_output=True exception for abandoned-turn reaping
so the deviation isn't 'fixed' later.
The API server intentionally lets concurrent runs share a client-provided
session_id (= process task_id), so the SSE-disconnect reap could kill a
process a still-live concurrent run spawned after the disconnecting run's
baseline — the same stale-reaper bug class the gateway path gates via
run_generation.
- Per-task-id run epochs (monotonic counter): each run claims the epoch at
publish; a reaper holding a superseded epoch declines to kill. A missing
entry (the run's own clear pruned it) still reaps, so the leak fix isn't
silently disabled.
- _publish_turn_process_ownership / _clear_turn_process_ownership helpers
replace the copy-pasted marker set/clear blocks, so attribute names and
epoch bookkeeping can't drift between surfaces.
- /v1/runs — the third own-lifecycle surface — now records ownership and
reaps on POST /v1/runs/{id}/stop and on server-side SSE cancellation,
closing the remaining sibling paths of #76115.
Two follow-ups from review of the salvaged fix:
- _reap_gateway_turn_processes now returns 0 for a blank task_id.
ProcessSession.task_id defaults to empty for sessionless callers, so a
blank turn id would match (and kill) every unrelated empty-task process
instead of the turn's own.
- The asyncio poll loop checks executor completion BEFORE the watchdog's
timeout flag. When both race in the same window, the completed run has
already persisted its real reply to session history; surfacing the
'agent inactive' diagnostic would contradict the stored transcript.
This matches _abandon_timed_out_gateway_turn's own worker-done-wins
tiebreak.
dbbb10d39 shipped without direct test coverage for its own new logic
— the same gap teknium's review flagged on the competing PR. Close it:
- _reap_gateway_turn_processes: skips when is_still_current() is
False, proceeds when True, fails open (reaps) if the check itself
raises rather than silently disabling the leak fix.
- _abandon_timed_out_gateway_turn: still marks the turn abandoned
(interrupt fires) even when the reap itself is skipped.
- api_server._reap_disconnected_agent_processes: reaps the
baseline-diff for an owned turn, no-ops when the agent never
recorded ownership markers.
- APIServerAdapter._run_agent: markers are populated with the right
task_id/baseline during the turn and cleared once it completes,
closing the same race window fixed in gateway/run.py for this
separate agent-lifecycle surface.
Addresses the hermes-sweeper review on #76188:
1. task_id is session-scoped (task_id == session_id), not turn-scoped,
and the reap runs on a detached thread. A replacement turn could
claim the same session and spawn a legitimate process before the
previous turn's reaper thread actually enumerates its targets,
killing that new process by mistake.
Fixed by gating the reap on the existing run_generation mechanism
(_is_session_run_current) instead of inventing a new ownership
token: the timeout path captures its own run_generation at turn
start, the interrupt path captures the generation immediately after
invalidating it. If a newer turn has since claimed the session, the
reap is skipped — that newer turn owns its own baseline, so nothing
is left permanently unreaped.
2. gateway/platforms/api_server.py's SSE handlers for chat-completions
and the /api/sessions responses endpoint run their own agent
lifecycle via _run_agent() and never passed through TurnRunner, so
client-disconnect abandonment there had no baseline and no reap —
contradicting the PR's stated disconnect coverage. Both disconnect
handlers now snapshot/reap through the same
tools.process_registry primitives, via a small
_reap_disconnected_agent_processes() helper shared by both call
sites.
An agent turn can spawn a long-running background subprocess (e.g.
`next build`) and later be abandoned via inactivity timeout, /stop,
/new, or a client disconnect. Before this fix the gateway interrupted
the agent loop but never touched the subprocess: it kept running
inside the gateway's cgroup, unbounded, until memory pressure starved
the event loop and made every platform/cron look hung (#76115).
The process registry already knew how to kill a process tree — the
missing piece was per-turn ownership: nothing distinguished a process
that predates the turn (must survive), a process the turn started and
finished successfully (must survive), and a process an abandoned turn
left running (must be reaped).
- tools/process_registry.py: snapshot_running_ids() captures a turn's
starting baseline; kill_started_since() reaps only IDs created after
it, scoped to one task_id.
- gateway/turn_context.py: TurnContext carries process_task_id +
process_baseline so the timeout/interrupt paths can reach them.
- gateway/run.py: baseline is snapshotted right before the turn's
executor task starts; the inactivity-timeout path and the explicit
/stop|/new|disconnect interrupt path both reap via the same helper.
A daemon-thread watchdog backs up the asyncio-based timeout poll,
since a starved event loop is exactly the failure mode this bug
causes. The turn's own worker clears its ownership markers the
instant it finishes, closing a race where a /stop landing right
after normal completion could reap a background process the turn
deliberately left running.
Related but insufficient on their own: #37454 (cgroup ExecStopPost
reaper only fires on service restart) and #68915 (orphaned-pipe
grandchild detection, a registry bug not a turn-lifecycle gap).
Neither ties process cleanup to turn abandonment.
- logger.warning when the 6h ceiling stops the heartbeat (matches the
delegate_task stale-stop precedent) so the eventual watchdog reap is
explainable from logs instead of silent
- new mutation-checked test: past the ceiling the heartbeat stops while
the job still completes
- clearer assertion messages (surface res on failure)
- heartbeat loop continues past a raising activity callback instead of
silently stopping (matches delegate_task / touch_activity_if_due
swallow-and-continue semantics) — one transient error must not drop
watchdog protection for the rest of a long job
- hard 6h elapsed ceiling so a wedged job under HERMES_CRON_TIMEOUT=0
(unlimited child watchdog) cannot mask the gateway watchdog forever
- public get_activity_callback() accessor in tools/environments/base.py
instead of importing the private _get_activity_callback cross-module
- tests: deterministic heartbeat test (event-gated, no timing sleep),
no-callback test now asserts the thread is truly never created, new
exception-survival guard; dead started event removed
- fix comment: delegate_task heartbeat cadence is 30s, not 10s
/simplify-code review found _poll_for_token has a second caller:
web_server._nous_poller (dashboard/desktop device login), which surfaces
str(e) as the UI error_message — so wrapping only in
_nous_device_code_login left the dashboard showing the bare timeout.
Move the enrichment into _poll_for_token's deadline raise so every
caller inherits the guidance, and drop the now-redundant try/except
wrap in the CLI login. Add a source-level regression test driving the
real poll loop (authorization_pending stub client) to the deadline.
A bare 'Timed out waiting for device authorization' gives the user
nothing to act on. The most common cause is Portal sign-in failing in
the opened browser tab (including the server-side CAPTCHA loop from
issue #20605), so point at the Portal login page and the hermes portal
retry command.
Salvaged from PR #75290 by @HexLab98 (timeout-guidance kernel only).
The URL-rewrite portion of that PR was dropped: the live Portal has no
/device route (verified 404 with a real user_code), so rewriting the
manage-subscription verification URL would break login entirely.
Guidance text reworded to reference only real URLs.
/simplify-code findings on the salvage stack:
- extract _category_skill_dirs() as the single category detector; the
install guard and hermes_cli._existing_categories() now share it
(third copy of the heuristic eliminated)
- filter rglob hits through is_excluded_skill_path so vendored /
support-dir SKILL.md files (node_modules, references/pkg) no longer
misclassify a plain directory as a category and block install
- fix inaccurate WHY comment (lock-file check only guards hub-installed
skills, not hand-authored dirs), drop underscore prefixes on locals,
fold the file-collision guard under the single exists() check
Follow-up to the salvaged #76000 guard:
- refuse installing a skill INTO an existing skill directory (hybrid
skill-plus-category dirs whose later update/uninstall rmtree would
destroy the nested skill — sibling case of #75983)
- refuse a stray regular file at the install path with the caller's
ValueError contract instead of an uncaught NotADirectoryError
- regression tests: nested-only category (skills at depth >= 2),
category-inside-skill, file collision
The salvaged hydrate_profile_secret_sources (#74549) seeded its
profile-local env from <home>/.env only, but the documented 1Password
bootstrap flow puts OP_SERVICE_ACCOUNT_TOKEN in the gitignored
<home>/.op.env (mirrored from load_hermes_dotenv). A cold profile using
that flow still failed 1Password hydration — the one unaddressed item
from the sweeper review on #74549. Seed .op.env via setdefault so .env
values win; never touches os.environ. Two regression tests.
Follow-ups on the #75382 salvage (review findings):
- _wenv/_get_wsecret now catch UnscopedSecretError and fall back to
os.getenv for the DEFAULT profile's adapter, which constructs and sends
outside any _profile_runtime_scope under multiplexing — a bare
get_secret would crash its WhatsApp path (fixing one profile by
breaking another). Same pattern as Slack SLACK_APP_TOKEN (#59739) and
the Matrix recovery key. Scoped misses still return the default — no
cross-profile borrow.
- bridge_env overlay extended to the full WHATSAPP_* set bridge.js
consumes (DEBUG, FORWARD_OWNER_MESSAGES, REPLY_PREFIX,
MAX_MESSAGE_LENGTH, CHUNK_DELAY_MS, SEND_TIMEOUT_MS).
- Removed the always-true conditional on WHATSAPP_MODE injection.
Fix#75349
Root cause:
Under multiplex_profiles, secondary profiles run inside
_profile_runtime_scope which installs a per-profile secret scope via
set_secret_scope. The WhatsApp adapter (and the shared
WhatsAppBehaviorMixin + Cloud API adapter) read WHATSAPP_MODE,
WHATSAPP_DM_POLICY, etc. via raw os.getenv(), bypassing the secret
scope. Since os.environ doesn't contain secondary profile .env values,
the bridge silently falls back to 'self-chat' and rejects all inbound
messages with self_chat_mode_rejects_non_self.
Fix:
- Add _wenv() helper in adapter.py that reads WHATSAPP_* vars through
get_secret() (agent.secret_scope), which honors the active scope.
- Replace all os.getenv('WHATSAPP_*') calls in adapter.py,
whatsapp_common.py, and whatsapp_cloud.py with get_secret()-based
equivalents.
- Inject resolved WHATSAPP_* values into the bridge subprocess
environment so the Node.js bridge (which reads process.env) sees the
profile's own configuration.
Changes:
- plugins/platforms/whatsapp/adapter.py: 37 lines (+ helper, bridge_env
injection, 2 os.getenv→_wenv)
- gateway/platforms/whatsapp_common.py: 13 lines (6 os.getenv→_get_wsecret)
- gateway/platforms/whatsapp_cloud.py: 21 lines (9 os.getenv→_get_wsecret)
- New regression test: 6 test cases covering scope isolation, fallback,
and cross-profile non-leakage.
The Matrix adapter read MATRIX_RECOVERY_KEY via os.getenv, so under
gateway.multiplex_profiles every profile resolved the default profile's
key. That produced "recovery key verification failed: Key MAC does not
match" and broke E2EE for secondary profiles (#69090).
Route the read through agent.secret_scope.get_secret, which honors the
active profile's scope, with an os.getenv fallback for an unscoped read
under multiplex (default-profile startup loop) — mirroring the Slack
app-token pattern (#59739). Applied to both the startup verification
site and the status diagnostic.
Fixes#69090
Multiplexed gateways resolve credentials through the fail-closed
per-profile secret scope (agent/secret_scope.py, Workstream A): any
get_secret() read outside a set_secret_scope(...) block raises
UnscopedSecretError. The agent turn installs the scope via _run_agent's
profile-scoping wrapper, but slash-command dispatch does not — so manual
/compress reached provider resolution unscoped and every invocation on a
gateway.multiplex_profiles: true deployment failed with:
Manual compress failed: get_secret('OPENROUTER_BASE_URL') called with
no profile secret scope active while multiplexing is on.
Same bug class as the cron scheduler (#57692) and the /v1/runs agent
path — an un-migrated call site the fail-closed design is meant to catch.
Two changes, both required:
- _handle_compress_command becomes a profile-scoping wrapper around the
existing handler (renamed _handle_compress_command_inner), mirroring
_run_agent: gated on multiplex_profiles, resolves the source profile's
home and runs the whole handler inside _profile_runtime_scope. Covers
the coroutine-side read (_resolve_session_agent_runtime).
- The compressor call switches from a bare loop.run_in_executor(None, …)
to the existing _run_in_executor_with_context helper, so the scope
contextvar survives the thread hop into _compress_context, where the
aux-client provider resolution reads credentials.
Single-profile gateways take the pass-through branch — zero behavior
change (pinned by test).
Tests: 2 added (scoped read inside the executor under fail-closed
multiplexing reproduces the field failure pre-fix; single-profile
pass-through). 162 gateway compress/multiplex-scope tests green.
The Playwright suite fails identically on every PR regardless of diff
(verified on a Python-only PR and a docs-only PR): the mock-backend
Electron window never gets a title, so boot/chat/setup/interim specs all
fail; only the dead-backend boot-failure path still passes. Breakage
window matches the Aug 1 night engines/npm churn (#76499/#76562/#76575).
Gated with 'false &&' in the job condition — delete that to re-enable.
Root-fix + re-enable tracked in #76627 (Ari).
- Merge _aux_free_only() + _aux_openrouter_model() into single
_aux_openrouter_settings() that reads config once via
load_config_readonly (avoids double deepcopy).
- Remove 15-line block comment and 5-line inline comment that
restated what the code already says.
- Trim module docstring from 10 lines to 3.
- Update test patches to target load_config_readonly.
Desktop edit / regenerate / restore-checkpoint send truncate_before_user_ordinal
on prompt.submit. The handler rewrote session['history'] first, then called
replace_messages, and on failure only printed to stderr and still started the
turn. When the durable write fails, in-memory history is already truncated
and history_version is bumped while state.db still holds the pre-edit tail.
The agent flush is append-only for history-dict identities, so the new
exchange is appended on top of the 'undone' turns — durable zombie history.
Fix: persist first; only then mutate memory. On failure return 5008 and
leave memory/DB unchanged.
Based on #72876 by @necoweb3. Adapted: the handler moved from
tui_gateway/server.py to tui_gateway/methods_prompt.py since the PR's base.
Start from main's 13 tests (renamed test_openai_available_reflects_key
to test_openai_available_reflects_audio_key_resolution, added 4 new
tests for xai oauth, elevenlabs secret resolver, openai configured
key, stream cap). Append 12 new regression tests from PR #71084 for
the prefetch pipeline, PCM misalignment, and PortAudio resilience.
Patch platform.system in stream-path tests for main's macOS guard.