Without this UA tag the Feishu server does not push group @mention events
over the WebSocket transport. The "channel" tag tells the server to use
the Channel protocol which enables group-message routing in addition to P2P
direct messages.
Root cause: FeishuWSClient was created without any UA signaling tag, so the
server defaulted to the basic DM-only push mode. Group @mention events were
silently dropped before reaching Hermes.
Fixes https://github.com/NousResearch/hermes-agent/issues/50656
Also adds a regression test verifying the UA tag is present in the
FeishuWSClient constructor call.
Sibling of the salvaged #53196 read-path fix: setdefault() does not
replace a present-but-null key, so saving platform tools with
known_plugin_toolsets: null in config.yaml crashed on indexing None.
When config.yaml has known_plugin_toolsets set to null (or any value
mapped to None by the YAML loader), config.get returns None (dict.get
only falls back to the default when the key is absent, not when its
value is None). The subsequent set(known_map.get(platform, [])) then
crashes with TypeError: NoneType object is not iterable and the gateway
fails to start, even though no plugin toolsets are configured.
Add or-empty-dict and or-empty-list guards so a null/None value is
treated as empty instead of crashing the platform-tools resolver.
`_load_web_config()` is typed `-> dict` but returned `load_config().get("web",
{})`, which is `None` when the config has a present-but-null `web:` section
(YAML `web:` with no body). Every caller then does
`_load_web_config().get(...)` and raises `AttributeError: 'NoneType' object
has no attribute 'get'` — this hits `_get_backend`, `check_web_api_key`, and
the extract-char-limit reader.
Separately, `check_web_api_key()` read the backend as
`.get("backend", "").lower()`; a null `web.backend` value yields `None` (the
`""` default only applies when the key is absent), so `None.lower()` raised.
`check_web_api_key` is the `check_fn` gate for `web_search`/`web_extract`, so
this surfaced as an exception during tool-availability checking.
- Make `_load_web_config()` honor its `-> dict` contract (`... or {}`), fixing
the null-`web:`-section crash at every call site.
- Guard the backend value in `check_web_api_key` with `or ""`, mirroring the
existing guard in `_get_backend`.
Adds regression tests for both the null-backend-value and null-web-section
cases.
* test: deflake CI and dev-machine flaky tests in bulk
Fixes ten distinct flake sources found by mining recent CI failures and
running the full suite on a dev machine with real user state:
CI-observed races:
- tests/conftest.py live-system guard: allow signal 0 (pure liveness
probe) through _guarded_kill/_guarded_killpg. psutil.pid_exists()
probes a just-killed grandchild reparented to init; the subtree check
fails for it and the guard RuntimeError'd
test_entire_tree_is_sigkilled_not_just_parent intermittently on
unrelated PRs.
Hermeticity flakes (fail on dev machines with real state, pass on CI):
- agent/coding_context.py: _marker_root() now skips the shared temp
root (tempfile.gettempdir()) like it skips $HOME — a stray
/tmp/package.json flipped every tmp_path test into the coding
posture (9 failures in test_coding_context.py).
- test_agent_guardrails.py: pin MAX_CONCURRENT_CHILDREN=3 via autouse
monkeypatch instead of freezing the user's real config value at
import time (import-time vs call-time config mismatch).
- test_web_tools_config.py: TestCheckWebApiKey now neutralizes the
ddgs package probe and registry providers — the optional ddgs
package in a dev venv lit up the fallback backend.
- test_credential_pool.py: block claude_code/hermes-oauth credential
autodiscovery in the two pool-merge tests that assert exact id
lists (a real ~/.claude/.credentials.json seeded an extra entry).
- test_modal_sandbox_fixes.py: clear _permanent_approved /
_session_approved — the user's real command_allowlist silently
approved the guard-escalation commands under test.
- test_setup_irc.py: stub prompt_checklist to select only the IRC row;
the non-TTY cancel fallback re-ran the real configured platforms'
interactive setup_fn, which hit input() under captured stdin.
- test_doctor.py: TestGitHubTokenCheck now patches the module-level
HERMES_HOME constant (the file's established pattern) instead of
only setenv — doctor was running PRAGMA integrity_check against the
real multi-GB state.db and blowing the 300s per-file budget.
Latent atexit-duplication (same _enter_buffered_busy class as #34217):
- test_undo_command.py: drop importlib.reload(tui_gateway.server) in
fixture teardown; reload re-registers the module's atexit hooks.
- test_session_platform_resolution.py: drop per-test reload of
tui_gateway.server; every resolver reads env at call time.
* test: sentinel model value in ignore-user-config fallback assertion
With HERMES_IGNORE_USER_CONFIG=1, load_cli_config() falls back to the
repo-root cli-config.yaml (untracked, gitignored). On a dev machine that
file can legitimately set the same popular model the test hardcoded
(anthropic/claude-sonnet-4.6), flipping the != assertion locally while
CI (no cli-config.yaml) stayed green. Use an impossible sentinel model
name instead.
Sibling sites of the salvaged #47334 fix: xai/openai/elevenlabs/gemini/
mistral/piper/neutts subsection reads in transcription_tools.py and
tts_tool.py used .get(key, {}) which passes a present-but-null value
through as None. All provider-subsection reads now use .get(key) or {}.
Providers without a DEFAULT_CONFIG entry (e.g. stt.xai) were still
receiving None even after the load_config() deep-merge fix, since the
merge can only fill sections that have defaults.
When stt.local, tts.edge, or other config subsections are explicitly set
to null in config.yaml (which happens by default on a fresh --voice
setup), stt_config.get('local', {}) returns None instead of {} because
YAML null preserves the key. The chained .get('model') then crashes
with 'NoneType' object has no attribute 'get'.
Apply the defensive (x or {}) pattern to every place a config subsection
is read via .get('xxx', {}). Covers local, edge, openai, mistral, and
elevenlabs subsections in both transcription_tools.py and tts_tool.py.
Closes#47318
skills: null crashed with AttributeError, and a bare scalar
disabled: my-skill was split into a set of characters. Both now
normalize the same way agent.skill_utils._normalize_string_set does:
null -> empty set, scalar -> single-item set. Non-dict skills
sections are ignored.
Closes#13026.
_load_context_cache() returned None when context_length_cache.yaml
contained 'context_lengths:' (no value) — YAML parses this as
{'context_lengths': None} and dict.get(key, default) only returns
the default when the key is absent, not when the value is None.
This caused AttributeError in every downstream caller (issue #47135).
Fix: use 'or {}' instead of default= so both absent key and
None value return an empty dict.
Fixes#47135
Addresses Copilot review on #61348: the HTML-escaped role, while safe from
injection (quotes are escaped), still contains whitespace when a crafted role
is supplied, which splits the class attribute into several unintended CSS
classes. Keep the escaped role for the display badge, and reduce the raw role
to a single safe CSS token (alnum/-/_) for the class name. Real roles
(user/assistant/system/tool) are unchanged, so the existing .message-<role>
rules still match.
The gateway reconnect watcher forwards is_reconnect=True to every
adapter.connect() call on every retry. Adapters whose signature omits
the kwarg raise TypeError at every reconnect attempt and stay silently
disconnected — the exact bug that shipped for QQAdapter and only
surfaced after messages stopped flowing on the QQ channel for hours.
This test statically parses every adapter.py under gateway/platforms/
and plugins/platforms/ (via AST, so third-party SDKs like slack_sdk,
matrix-nio, aiohttp, telegram, etc. are NOT required in the test env)
and asserts every *Adapter class with an async connect() accepts
is_reconnect — either as a keyword-only argument or absorbed by
**kwargs.
Also fixes plugins/platforms/wecom/callback_adapter.py:WecomCallbackAdapter,
which the new test caught as a second offender. Same class of bug: bare
'async def connect(self)' signature would die on the first reconnect.
Companion to #59429 (which fixed the original QQAdapter offender).
The base adapter's signature was updated to include
, which the reconnect watcher passes as
during reconnection. All other platform adapters were
updated, but QQAdapter was missed, causing:
TypeError: QQAdapter.connect() got an unexpected keyword argument 'is_reconnect'
This leads to an infinite retry loop since every reconnect attempt fails
immediately with the same TypeError.
Fix: add to QQAdapter.connect()'s signature.
QQBot has no server-side update queue, so the flag is accepted only for
interface conformance.
Test: new test_connect_accepts_is_reconnect_param verifies both
adapter.connect() and adapter.connect(is_reconnect=True) succeed without
raising.
The streaming fallback path read yaml_cfg.get("gateway", {}).get("streaming") when top-level streaming was absent or malformed. If a user accidentally set gateway to a scalar value, config loading crashed with AttributeError instead of ignoring the malformed block and using defaults.
Read the gateway block once, verify it is a mapping before accessing nested streaming, and keep the existing gateway.platforms fallback using the same checked value.
Adds a regression test for config.yaml containing gateway: disabled.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
GatewayConfig.from_dict(), PlatformConfig.from_dict(), SessionResetPolicy.from_dict(), and StreamingConfig.from_dict() assumed their input sections were mappings. A malformed scalar from legacy gateway.json or an internal caller could crash config loading with AttributeError before env overrides/defaults had a chance to recover.
Coerce non-mapping sections to empty dicts, skip malformed platform entries, and keep valid sibling platform configs loading normally.
Tests cover scalar platform blocks, scalar nested reset/streaming sections, and malformed PlatformConfig home_channel/extra values.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Sibling site of the load_cli_config fix (#58277): _deep_merge treated a
YAML-null section (terminal: with no value) as an override, replacing
the entire DEFAULT_CONFIG dict for that section with None. Every
downstream consumer expecting a mapping was a latent crash, and default
sub-keys were silently lost. A None override of a dict default is now
ignored, matching the CLI loader's behavior. Scalar-null overrides are
unchanged.
Three tests for the #61099 salvage: OpenRouter attribution headers
present after switching to openrouter.ai, Kimi User-Agent sentinel
present after switching to api.kimi.com, and stale headers cleared
when switching to a provider with no URL-specific headers.
2/3 fail on unpatched main (DID NOT ATTACH), confirming the bug.
switch_model() rebuilds _client_kwargs from scratch (api_key + base_url)
but does not call _apply_client_headers_for_base_url(), so provider-
specific headers like OpenRouter HTTP-Referer and X-Title are lost.
Subsequent requests show "Unknown" in OpenRouter dashboard logs.
Call _apply_client_headers_for_base_url() after rebuilding _client_kwargs
and before creating the new client.
Fixes#61099
Follow-ups for salvaged PR #43819: the registry key was
str(Path(db_path).expanduser()) — a symlinked or relative path to the
same DB file got its own connection, silently reintroducing the exact
multi-writer contention the registry prevents. Key on Path.resolve()
(OSError-tolerant fallback). Adds a symlink regression test and the
AUTHOR_MAP entry for adambiggs.
Every MemoryStore instance opened its own SQLite connection guarded by
its own RLock. Several providers coexist in one process (the main agent
plus every delegate_task subagent), so instances pointing at the same
memory_store.db raced as independent WAL writers. Combined with writes
that were not rolled back on error, one connection could leave an open
write transaction that pinned the write lock and made every other
connection's writes fail with "database is locked" for the full busy
timeout.
Instances for the same database now share ONE process-wide connection
and ONE re-entrant lock, so access is fully serialized and
cross-connection contention is impossible. The shared connection is
refcounted: closing one instance never tears it out from under a live
sibling, and the last close releases it. The connection runs in
autocommit (isolation_level=None) so a write that raises mid-method can
never leave a dangling transaction holding the write lock; the existing
explicit commit() calls become harmless no-ops.
The provider's shutdown() now calls the refcount-guarded close() instead
of just dropping the reference: leaving finalization to GC kept the
connection (and its write lock) alive indefinitely on long-running
gateways, prolonging the exact contention this fix removes. The last
provider now releases the connection deterministically while siblings
stay live; regression tests fail without the wiring.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Flip the two tests that pinned the old buggy behavior (rewrite_transcript
called after in-place compaction) to assert the corrected invariant from
#61145: archive_and_compact() already persisted, so the handler must NOT
call rewrite_transcript — its replace_messages(active_only=False) would
DELETE the just-archived rows.
E2E-verified against a real SessionDB: 6 soft-archived rows are wiped by
replace_messages' default path, confirming the data-loss premise.
When gateway session-hygiene auto-compression fires with in-place
compaction, the flow was:
1. _compress_context() calls archive_and_compact() — soft-archives old
rows (active=0, compacted=1) and inserts compacted messages as the
new active set. This is the non-destructive, durable path.
2. The hygiene handler then called rewrite_transcript() — which calls
replace_messages(active_only=False) — DELETEing ALL rows including
the just-archived turns. Silent permanent data loss (#61145).
The interactive /compress handler had the same bug.
Fix: only call rewrite_transcript() when session rotation produced a new
session id (legacy path). When in-place compaction succeeded, skip the
rewrite — archive_and_compact() already handled persistence.
Closes#61145.
test_accepted_at_every_position spawned 11 separate
'python -m hermes_cli.main' subprocesses, each cold-importing the full
CLI module tree under a 15s TimeoutExpired deadline. On a loaded CI
worker the import alone can exceed that (slice 2/8 flaked exactly here
on PR #61726's run, TimeoutExpired at subprocess.py:1253), failing PRs
that never touched the CLI.
Replace with ONE driver subprocess that imports hermes_cli.main once
and parses all 11 argvs in-process (catching SystemExit per argv),
reporting JSON results. Same assertions per argv, identical semantics
(verified the --help-before-unknown-flag exit behavior matches the old
method), ~11x less import work, and the 180s timeout only trips on a
genuine hang.
Structural completion of the malformed-job freeze fixes (#61382 id-less,
#61525 non-dict schedule, #61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.
Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the #61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.
E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
One bad next_run_at value in jobs.json aborts the due-jobs scan with
ValueError from fromisoformat, before any save_jobs, so siblings lose
progress (fast-forwards etc).
Early normalization in _get_due_jobs_locked + defensive parses in
compute_next_run / _recoverable_oneshot_run_at.
Added test_bad_next_run_at_does_not_crash_or_block_sibling_jobs.
A job record in jobs.json can have a non-dict 'schedule' value (null, string,
etc.) from direct edit or old writers.
In _get_due_jobs_locked:
schedule = job.get('schedule', {})
kind = schedule.get('kind')
This (and direct schedule['kind'] in compute_next_run etc.) raises and
aborts the entire due-jobs scan before save_jobs() or advancing next_run_at
for healthy jobs. Exactly the same failure mode as the id-less job P1.
Fix: normalize non-dict schedules to {} early (before any use), matching the
defense added for id-less records. Also added defensive guards in compute
functions.
Added regression test that a bad schedule does not crash and healthy sibling
is still returned.
Refs similar pattern in #61382.
A cron record authored by a direct jobs.json edit that bypassed
add_job() can lack an "id" key (older writers used "job_id"). Every
site in _get_due_jobs_locked indexes job["id"] eagerly — both the
logging helpers (job.get("name", job["id"]) evaluates the default
argument unconditionally) and the 'for rj in raw_jobs: if rj["id"] ==
job["id"]' persistence loops. A single malformed record therefore
raised KeyError mid-tick, aborting the entire scan before save_jobs()
ran. Result: healthy jobs' fast-forwarded next_run_at was computed in
memory then discarded on the exception unwind, freezing the whole
profile's scheduler in a per-minute loop (observed dormant for weeks).
Fix: normalize id-less records at the top of _get_due_jobs_locked before
anything keys off job["id"] — recover the id from a drifted "job_id"
key when present, else synthesize one via uuid4, and persist. This
repairs the whole bug class at the source rather than guarding each of
the ~12 downstream index sites.
Adds a regression test that fails with KeyError on the current code and
passes with the fix, asserting a healthy sibling job is still returned
when an id-less record shares the store.
The salvaged fix sets HERMES_YOLO_MODE in main()'s dispatch path before
_prepare_agent_startup(); this follow-up also sets it inside
_prepare_agent_startup() itself so every launcher that triggers plugin/tool
discovery (incl. the Termux fast-CLI path) gets the same ordering guarantee
before tools.approval freezes _YOLO_MODE_FROZEN (#60328).
The HTML session export interpolated the tool-call name into the page
without escaping, while every sibling field went through _escape_html. A
tool-call name is attacker-influenced, so a prompt-injected model can emit
a name containing HTML that executes when the export is opened in a browser.
Escape the tool-call name like the other fields.
switch_model() unconditionally set agent.provider but only set
agent.base_url when the resolved value was truthy. When a real
provider change resolved an empty base_url (e.g. minimax after
copilot), the agent ended up with provider="minimax" but
base_url still pointing at api.githubcopilot.com. That incoherent
pair then got snapshotted into agent._primary_runtime, so it kept
re-applying on every subsequent turn via restore_primary_runtime()
until the process restarted.
try_activate_fallback() and _swap_credential() were audited and
confirmed unaffected: both always derive base_url from an actually
constructed client, never from a possibly-empty resolver hint.
Fix: when base_url is empty AND the provider is genuinely changing,
raise ValueError instead of silently keeping the old provider's URL.
This routes through switch_model()'s existing snapshot/rollback
path, and callers (tui_gateway/server.py's _apply_model_switch)
already catch and surface a clean "switch failed, staying on X"
message. Re-selecting the SAME provider with an empty base_url
(credential-only refresh) still keeps the current URL, unchanged.
Fixes#47828
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Port from openai/codex#31188: a parse failure in a policy-bearing config
file must not silently replace the effective policy with an empty/default
one. Codex's load_exec_policy_with_warning replaced the whole exec policy
with Policy::empty() when a .rules file failed to parse, silently dropping
managed prompt/forbidden rules; the fix preserves the managed policy while
still warning.
Hermes had the same bug shape in load_config(): a YAML parse error made
_load_config_impl() fall through to DEFAULT_CONFIG, dropping every user
override — including approvals.deny rules, which are documented to block
commands even under --yolo. In a long-running gateway, a user mid-editing
config.yaml into broken YAML silently disarmed their own deny rules on the
next load.
Now, when the process has a last successfully loaded config for that path
(_LAST_EXPANDED_CONFIG_BY_PATH), a parse failure keeps serving it (cached
under the corrupt file's signature so the broken file isn't re-parsed) and
the warning says edits are being ignored until the YAML is fixed. Fresh
processes with no last-known-good keep the existing DEFAULT_CONFIG
fallback and warning.
E2E-verified: deny rule 'curl*evil.com*' still blocks after mid-process
corruption; fixed file reloads normally; fresh-process fallback unchanged.
The earlier fix gated _resolve_use_tui, but the EARLY launcher
(_wants_tui_early) decides TUI from display.interface before cmd_chat
runs — so a `display.interface: tui` default still booted the Ink UI for
headless spawns (kanban workers), whose no-TTY bail-out exits 0 →
"protocol violation". Gate the early resolver on a real TTY: headless
stdio never boots the TUI regardless of config; explicit --tui still does.
Dragging a task done→ready did nothing: the respawn guard saw a run that
completed within the success window and deferred forever, unable to tell a
deliberate operator re-run from a status flap. Now a re-queue event
(status change, promote, unblock, reclaim) AFTER the completion bypasses
the recent_success guard, so an explicit done→ready runs again.
A retried task (→ running) kept showing "crashed Nx": the in-flight run
has no outcome yet, so the trailing crash scan skipped it and kept
counting the prior streak, and the consecutive_failures counter lingers.
Exempt `running` from both repeated_failures and repeated_crashes so a
fresh attempt clears the banner until it itself resolves (re-fires if the
new run also fails).
An inherited HERMES_TUI=1 or a `display.interface: tui` config default sent
kanban workers into the Ink TUI, whose no-TTY bail-out exits 0 without doing
the task — every attempt ended in "protocol violation". Two layers:
- _default_spawn pins `--cli` (highest-precedence interface flag) and strips
HERMES_TUI from the child env (covers older builds on PATH).
- _resolve_use_tui gates ambient TUI prefs (env/config) behind a real TTY;
an explicit --tui still wins so the informative bail-out stays reachable.
A manual done (dashboard/desktop drag) runs complete_task but ends no
run, so a trailing crashed/crashed run history never gains the
'completed' outcome that breaks the repeated_crashes streak — the card
kept flagging "needs attention" forever after being finished.
repeated_failures had the same hole via a stale counter. Terminal
statuses are now exempt from both: done means done; the history stays
on the event log for audit. Regression test included.
Adding the 6 gpt-5.6 slugs to DEFAULT_CODEX_MODELS grew the curated codex
catalog to 11, above the test's max_models=10 cap. That truncated the
picker list to 10 while total_models reported 11, breaking the
total_models == len(models) assertion. The cap was an implicit
change-detector on catalog size; raise it to 100 so the list is never
truncated and the count-consistency invariant stays meaningful as new
gpt-5.x slugs land.
