Commit 62a76bd3d (feat: make smart approvals the default, #62661)
changed approvals.mode default from "manual" to "smart". The
TestBlockingApprovalE2E tests did not patch the approval mode, so
check_all_command_guards routed through _smart_approve() first —
calling call_llm() which tried all auxiliary providers, failed (no
API keys in test env), and returned "escalate" before falling through
to the gateway blocking path. The LLM failure cascade took longer
than the test's 2.5s wait window (50 × 0.05s), so the notify
callback had not fired yet when assert len(notified) == 1 ran.
Force {"mode": "manual"} via patch in setup_method/teardown_method,
matching the pattern already used by test_blocking_approval_uses_
canonical_timeout in the same class.
Reserve API requests before their first await, include every drain-owned work type in runtime status, and pause local cron dispatch while a gateway drain is active.
Closes#63529
Root cause: GatewayRunner._drain_active_agents only waited on
_running_agents + cron in-flight counts. Desktop/API sessions are
tracked solely inside APIServerAdapter (_inflight_agent_runs +
_active_run_agents), so stop/restart logged active_at_start=0 and
systemd SIGKILL'd mid-tool work.
Fix: APIServerAdapter.active_agent_work_count() plus
GatewayRunner._active_api_run_count() folded into the drain wait,
status updates, timeout result, and shutdown logs — same pattern as
_active_cron_job_count for #60432.
Verification: pytest tests/gateway/test_api_server_active_work_drain.py
tests/gateway/test_cron_active_work_drain.py -q → 19 passed
GatewayRunner._prepare_inbound_message_text's "@" context-reference
block read self._model / self._base_url to resolve the model for
get_model_context_length_async. GatewayRunner never sets either
attribute (copy-pasted from HermesCLI in da44c196b, which does carry
self.model/self.base_url). Every message containing "@" raised
AttributeError inside the try block, silently swallowed by the
surrounding except Exception at debug level, so
preprocess_context_references_async never ran and @file:/@folder:/@diff/
etc. references passed through to the model unexpanded.
Fix: resolve model/provider/base_url via
self._resolve_session_agent_runtime(source=, session_key=,
user_config=), the same session-aware resolution the hygiene
compression block already uses a few hundred lines later in this file.
Also raise the swallow log from debug to warning (with exc_info at
debug) so a future regression here is visible instead of silent.
Keep empty-tail recovery scoped to the current stream segment and bound fallback flood retries. Preserve Telegram's server retry hint without blocking final delivery through a long cooldown.
The second lock block in get_or_create_session held self._lock during six
blocking operations on every inbound message: _is_session_ended_in_db
(SQLite SELECT), _should_reset (callback), _save (SQLite write + JSON write
+ os.fsync), and _recover_session_from_db (SQLite SELECT + UPDATE).
A code comment at line 1607 claimed 'SQLite calls are made outside the
lock' -- true only for _compression_tip_for_session_id, which was moved
out in a prior fix. The remaining I/O was never addressed.
Restructure into a four-phase lock/no-lock split that mirrors the pattern
already established at the bottom of the function:
Phase 1 (lock) -- read entry + session_id
Phase 1b (no lock) -- stale check + reset policy
Phase 2 (lock) -- apply decisions to _entries, capture snapshot + flags
Phase 3 (no lock) -- recovery DB query, _save from snapshot, end/create
_save_entries(snapshot) replaces _save() to avoid dict-mutation races when
called outside the lock. _query_recoverable_session splits the DB I/O out
of _recover_session_from_db so only the _entries assignment needs the lock.
Three early returns inside the lock block are eliminated in favour of a
unified save + return path.
The sync _session_has_compression_in_flight sat on the message hot path
and blocked the event loop twice: under session_store._lock during
_ensure_loaded_locked (JSON read) and via db.get_compression_lock_holder
(SQLite SELECT). Async-ify the method and offload both sources via
asyncio.to_thread; await the call site in _handle_active_session_busy_message.
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.
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>
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.
Follow-ups on the #60987 salvage (review pass):
- _refresh_fallback_model: keep last known-good chain on transient
config.yaml read/parse failure (user mid-edit, torn write) — only a
successful read that lacks the key clears the chain. Previously a
refresh error wiped a cached agent's working fallback for the turn.
- Move the cached-agent refresh+apply OUTSIDE the agent-cache lock:
config.yaml read is disk I/O and the idle-sweep watcher contends on
that lock (same reasoning as #52197). Per-session turn serialization
keeps the post-lock apply safe.
- _apply_fallback_chain_to_agent: clear _unavailable_fallback_keys when
chain content actually changes, so an entry re-configured mid-uptime
(e.g. credentials added) is retried instead of staying suppressed for
the cached agent's lifetime; no-op refreshes keep the memo.
- Tests: cwd-independent source pin (Path(__file__) anchor), pin the
reuse-path apply call, + regression tests for last-known-good, memo
clear-on-change, memo keep-on-unchanged (mutation-verified).
Pin reload + cached-agent apply helpers for #60955 so a mid-uptime
fallback chain change reaches messaging sessions without a restart.
(cherry picked from commit fafb341035)
Completes the session-binding class on the gateway surface (#55578),
matching the TUI rules:
1. Fail-closed pinning: switch_session() re-opens ended sessions, so
pinning a completion to a spawning session that has since ENDED
(user /new, closed rotation) would resurrect a conversation the user
explicitly ended and inject into it. The injection path now checks
the pinned row's ended_at first and drops the injection with a
WARNING when the spawning session is dead or unknown - the result
stays in the delegation records.
2. /new ends the old conversation's delegations: _handle_reset_command
calls interrupt_for_session() with the expiring durable session id
(matching the parent_session_id pin stamped at dispatch) plus the
routing key as fallback, so a reset can't leave dangling subagents
whose completions have no live owner.
interrupt_for_session() gains the parent_session_id selector because a
gateway chat's session_key (the platform conversation key) survives a
reset while the session id rotates - key-based matching alone could
never sever a gateway conversation's delegations.
For air-gapped / self-hosted-IdP deploys with NO Nous Portal, let the gateway
obtain its caller-identity bearer from a generic OAuth2 client_credentials grant
against the operator's own IdP (e.g. Microsoft Entra ID) instead of only
resolve_nous_access_token(). The connector's OIDC tenant resolver reads a claim
(default tid) off that token as the tenant.
- gateway/relay: new canonical _resolve_relay_identity_token() — client_credentials
when gateway.idp.token_url (or GATEWAY_RELAY_IDP_* env) is set, else Nous Portal
(unchanged default). Wired into self_provision_relay().
- hermes_cli/gateway_enroll: _resolve_identity_token() delegates to the canonical
resolver so the enroll CLI and the runtime self-provision path share ONE impl.
Config via gateway.idp.{token_url,client_id,client_secret,scope} in config.yaml
(env override GATEWAY_RELAY_IDP_*). No behaviour change when unset.
Tests: tests/gateway/relay/test_identity_token_resolver.py (6 — mode selection,
request shape, config/env precedence, fail-closed). Relay suite 162 pass.
Validated via the cross-repo gateway<->connector live E2E (provision, managed
self-provision, inbound round-trip, /link) against a connector running the OIDC
tenant resolver with zero NAS config.
Keep #60631's get_running_job_ids() snapshot + _active_cron_job_count()
(import-guarded for minimal test doubles) as the single read path, and
retarget #60612's drain tests at it. Drops the redundant
cron_jobs_in_flight() helper so there is one surface, not two.
Cron jobs run through cron/scheduler.py's own ThreadPoolExecutor via a
standalone AIAgent (run_job/run_one_job), entirely outside
GatewayRunner._running_agents -- the dict _drain_active_agents() and
every other active-work check on that class reads. A gateway shutdown
(/update, /restart, and SIGUSR1 all funnel through the same stop())
could log active_at_start=0 and immediately kill tool subprocesses
while a cron job's terminal command was still running, with no wait
and no indication anything was interrupted.
Real-world impact (from the issue): a scheduled daily briefing cron
job was in flight during /update, its tool subprocess got killed
by the unconditional shutdown cleanup, and the job was never marked
failed -- it simply never completed or delivered, with no error
surfaced anywhere. A repro with a 30-minute `sleep` cron job in flight
during /update reproduced the same pattern: subprocess killed at
+0.22s of drain (active_at_start=0), the job's agent thread continued
in-process and produced a plausible-looking final response from the
truncated tool output, and the scheduler marked the run successful.
Root cause is layered, not a single line:
1. GatewayRunner._drain_active_agents() only waits on _running_agents.
Cron work was invisible to it, so drain returned instantly whenever
the only active work was a cron job.
2. Even with visibility, the shutdown's final tool-subprocess kill
(process_registry.kill_all()) is a global, unconditional sweep with
no per-job targeting -- a long-running cron job that outlives the
drain timeout still gets its subprocess killed.
3. cron/scheduler.py had no way to detect that a job's tool subprocess
was killed out from under it mid-run; the agent thread kept going
and its eventual (often degraded but plausible-looking) response
got reported as a normal successful completion.
Fix, three parts:
- cron/scheduler.py: expose get_running_job_ids() (thread-safe
snapshot of the existing _running_job_ids set, already used to
prevent double-dispatch) so the gateway can read cron's in-flight
state without reaching into private module internals.
- gateway/run.py: GatewayRunner._active_cron_job_count() reads that
snapshot. _drain_active_agents() now waits on
(_running_agents OR active cron jobs), so a cron-only workload gets
the same bounded wait chat sessions already get instead of an
instant active_at_start=0. Shutdown drain logging gains
cron_active_at_start/cron_active_now fields alongside the existing
ones (unchanged, for compat).
- cron/scheduler.py: mark_running_jobs_interrupted(reason), called by
gateway/run.py's _kill_tool_subprocesses() right after
process_registry.kill_all(), marks every job still in
_running_job_ids at that instant as failed/interrupted via the
existing mark_job_run() -- and records the job IDs in
_interrupted_job_ids BEFORE writing, so run_one_job()'s own
eventual completion for the same run (racing in its own thread)
checks that flag and skips its normal write instead of clobbering
the interrupted status with a false "ok" produced from the
now-truncated tool output. This does not attempt to correlate a
killed PID to a specific job ID (process_registry tracks PIDs, not
job IDs) -- any job still dispatched at the moment of a forced kill
is treated as interrupted, matching the existing coarser precedent
set by _interrupt_running_agents(), which interrupts every entry in
_running_agents on a drain timeout without per-agent correlation
either.
Deliberately out of scope (flagged in the issue as a separate,
lower-priority concern): startup-time reconciliation of cron runs that
started but never reached a terminal status.
Testing:
- tests/cron/test_shutdown_interrupt.py (12 tests): get_running_job_ids
snapshot semantics, mark_running_jobs_interrupted marking/no-op/
partial-failure behavior, and -- the core race guard -- run_one_job
skipping its own last_status write (both the success path and the
exception path) when the shutdown path already marked the run
interrupted, with a control test proving ordinary un-interrupted
completions are unaffected.
- tests/gateway/test_cron_active_work_drain.py (9 tests):
_active_cron_job_count reading cron state and failing closed (0) if
the cron module is unavailable; _drain_active_agents waiting for an
in-flight cron job the same way it waits for chat sessions, timing
out if the job outruns the window, and leaving existing chat-session
drain behavior unchanged; a full runner.stop() integration test
(drain-timeout path) proving mark_running_jobs_interrupted actually
fires with the right job ID when a tool subprocess is force-killed,
plus a no-op control when nothing cron-related is in flight.
- tests/gateway/test_shutdown_cache_cleanup.py: added
_active_cron_job_count() to that file's hand-rolled _FakeGateway test
double, which stop() now calls -- without it those 8 pre-existing
tests AttributeError (caught by fail-then-pass below, not a
production bug).
Fail-then-pass: reverted gateway/run.py + cron/scheduler.py, all 21
new tests fail (fixture/attribute errors -- the feature doesn't exist
yet); restored, all 21 pass.
Regression check: ran the full plausibly-affected surface --
tests/gateway/{test_gateway_shutdown,test_restart_drain,
test_restart_notification,test_restart_redelivery_dedup,
test_restart_resume_pending,test_restart_service_detection,
test_shutdown_cache_cleanup,test_stuck_loop,test_clean_shutdown_marker,
test_external_drain_control,test_session_state_cleanup,
test_update_command,test_update_streaming}.py plus tests/cron/ (944
tests) -- against a clean upstream/main checkout and against this
branch. Diffed the two FAILED lists: identical, 20 pre-existing
failures on both sides (Windows-locale/cp1252 file-encoding issues and
Unix-permission-bit assertions that don't apply on this Windows dev
box), zero new failures, zero fixed-by-accident. The 8
test_shutdown_cache_cleanup.py failures found mid-development were
from the _FakeGateway gap above, fixed in the same commit and
confirmed clean on the final rerun (diff against baseline: exit 0).
Fixes#60432
