_humanize_user_mentions rewrites <@UID> to @DisplayName by passing the
resolved name as re.sub's replacement, where re parses it as a template.
A display name is arbitrary user-set text, so the escapes in it are the
user's characters, not regex syntax:
dev\ops -> re.error: bad escape \o
a\1b -> re.error: invalid group reference 1
\g<0> -> expands to the whole match, silently putting the opaque
<@UID> back — the token this method exists to remove
The trigger-text call site sits in _handle_slack_message outside any try,
and both Bolt event handlers await it bare, so the raise takes the whole
inbound message down: every message mentioning that person is dropped.
Pass the replacement as a function instead — re does no template parsing
on the return value, so the name lands verbatim. Same shape the Matrix
adapter already uses for its outbound mention rewrite.
The non-streaming /v1/responses path built function_call and
function_call_output output items with no status field (and no item id),
while the SSE streaming path correctly emits status in_progress ->
completed. Spec-strict OpenAI clients reading the non-streaming output
array could interpret the status-less function_call items as pending
calls the CLIENT must execute — but these tools were already executed
server-side by the Hermes agent and are replayed for structured tool UI
only. Reported by a community user whose GPT-5.6 client concluded 'a
server should not tell an OpenAI client to execute a tool the server
already executed itself'.
- _extract_output_items now stamps status: completed and spec-shaped
item ids (fc_/fco_) on replayed items, matching the streaming path
- test updated to pin status + id shape
- docs example updated + explicit note that output tool calls are
replayed, never pending
Two defects in _normalize_empty_agent_response surfaced together during a
state.db lock-contention incident on an enterprise Slack deployment:
- the error lookup used dict.get's default, which an explicit
'error': None value bypasses, rendering 'The request failed: None' /
'unknown error';
- persistence failures fell through to the generic branch, whose 'use
/reset' advice is harmful for this failure mode (destroys conversation
context, fixes nothing).
Persistence-failed turns (failure_reason session_persistence_failed:*,
with a legacy fallback on the error text) now get a dedicated message:
storage was temporarily unavailable, the message was recorded, send it
again — with a disk-specific variant. No /reset suggestion. All other
branches unchanged.
The _standalone_send() function (used by the send_message tool for
proactive/scheduled sends) has the same bare `@<id> text` bug that
send() had before PR #44444. SimpleX's `@<x>` syntax resolves x as a
display name, not a contactId — the daemon silently drops messages
when it cannot find a contact named "6".
Use the structured `/_send @<id> json [...]` form, matching what
send_image, send_document, and the send() fix already use.
Fixes#46265
Adds regression test for the case where both disk and live start_time
are known integers but differ (e.g. stale value from a previous run).
The self-PID short-circuit must fire regardless — start_time only
guards PID reuse for *other* PIDs. Inspired by #81495's test case.
Replace the 'describe everything in thorough detail' auto image-preprocess
prompt with a concise 2-4 sentence summary prompt so image-bearing gateway
messages stop generating ~2000-char descriptions (35s+ on local models).
Prompt-only variant of #10852: the max_tokens=500 cap and the
preserve_max_tokens aux-client plumbing from the original PR are
intentionally dropped to stay compatible with the max-tokens-knob policy
direction (#75253 removes hardcoded vision caps).
Fixes#10809
Surfaced during the post-merge review pass on our own #81113 follow-up:
cached_fetch_api_models gained _cache_entry_valid (numeric-'at'
validation) but its sibling cached_provider_model_ids still did
float(entry.get('at', 0)), which raises ValueError/TypeError on a
hand-edited or corrupted provider_models_cache.json row and propagates
uncaught into the /model picker call sites. Same fix, same helper:
corrupt rows are now a cache miss (live fetch), never an exception.
Both wrappers now share the identical validity predicate, closing the
divergence the 'mirrors' docstring promised away.
Also two test nits from the same review: unused OrderedDict import
dropped and the drain-order assertion strengthened to pin LRU-first
FIFO order in tests/gateway/test_agent_cache_pressure.py.
Mutation-checked: restoring the raising float() form makes the new
corrupt-at tests fail.
Two small config-gated features:
1. Kanban orphaned-card reconciliation (kanban.reconcile_orphans, default
true, config.yaml): a running card with broken claim bookkeeping
(claim_lock or claim_expires NULL — crash mid-claim, manual SQL, DB
restore) is invisible to all existing recovery paths
(release_stale_claims requires claim_expires NOT NULL,
detect_crashed_workers requires host-local lock + pid,
detect_stale_running is config-disabled by default) and shows Running
forever. New reconcile_orphaned_running() pass in kanban_db.py runs
each dispatch_once tick: requeues orphans to ready with an explanatory
comment, closes any leaked run, emits a 'reconciled' event, and defers
when the recorded PID is still alive on this host (never requeue
beside a live worker). Surfaced via DispatchResult.reconciled_orphans.
2. Per-server MCP identity header (mcp_servers.<name>.identity_header,
config.yaml): optional {name, value_from: static|profile, value}
mapping; the header is attached to that server's HTTP/SSE transport
requests. 'static' sends the config value; 'profile' resolves the
active Hermes profile name once at connect time (no per-call
mutation). Explicit per-server headers of the same name (any casing)
win. Invalid blocks warn-and-ignore; stdio servers warn-and-ignore.
Tests: tests/gateway/test_kanban_reconcile_orphans.py (9),
tests/tools/test_mcp_identity_header.py (13), all written first (RED)
then implemented (GREEN). No new HERMES_* env vars.
Inspired by: openai/symphony tracker reconciliation (Apache-2.0) +
Poke per-user MCP identity (idea-level).
- Drain the eviction plan (pop + del) before trim_memory: the batch
thread previously held every evicted agent in its local list while
gc.collect + malloc_trim ran, so the in-pass trim freed almost
nothing, the next tick re-read a still-high RSS, and the valve
over-evicted an extra batch of warm prompt caches per cycle.
- Clear _db_flush_scan_prefix in _release_evicted_agent_soft: it is a
shallow copy of the flushed transcript (stamped on every successful
flush) sharing every message dict — and pressure-evictable agents
have flushed by definition, so it pinned the multi-MB content strings
on exactly the agents the valve targets.
- Config-read failure now falls back to resolve_agent_cache_bounds({})
instead of bare AgentCacheBounds(): the dataclass default disables
the pressure pass, but an absent config section means 'auto' — a
transient read failure must not permanently switch off the OOM valve.
- protect_recent: false (YAML bool; False == 0) keeps the default MRU
protection instead of silently disabling it.
- 'No evictable session' warning now distinguishes sessions blocked on
un-flushed persistence (e.g. session DB never initialized — NFS
HERMES_HOME) from mid-turn agents, so operators can diagnose why the
valve isn't shedding instead of being pointed at running turns.
- _cgroup_limit_bytes checks the process's own cgroup (via the existing
gateway.cgroup_cleanup._own_cgroup_path) before the root files, so a
systemd unit's MemoryHigh=/MemoryMax= is detected — the root
memory.high/max read 'max' on those deployments.
- Tests: 5 new guards; drain-before-trim and scan-prefix-clear
mutation-checked (revert each fix -> its guard fails).
The per-session agent cache is capped at 128 entries with a 1h idle TTL, and
neither bound knows how many bytes it holds. Each cached agent pins
_session_messages -- the full transcript including tool output, tens of MB on
a session with 100+ tool calls -- so a gateway serving many chats keeps every
warm transcript resident: agents that took a turn inside the TTL are never
idle-swept, and the idle sweep additionally defers finalizable sessions until
they expire. RSS climbs until the cgroup throttles and SIGTERM can no longer
flush inside systemd's stop timeout.
Add the missing bound. Each session-expiry watcher tick compares the process's
anonymous RSS against a budget and, when over, sheds LRU agents through the
same soft-eviction path the cap enforcer uses, then runs malloc_trim so the
freed arenas actually return to the OS. Evicted sessions rebuild their
transcript from the persisted session on the next turn.
Three classes of session are never shed: agents mid-turn, the most recently
used ones, and any session whose transcript has not finished reaching disk
(_last_flushed_db_idx vs len(_session_messages) -- the same divergence the FTS
write-corruption guard reacts to when it preserves live history).
memory_high_mb defaults to "auto", deriving the budget from the cgroup limit
the gateway runs under, so a MemoryHigh/MemoryMax on the unit is respected
without a second number to keep in sync. The two existing bounds become
configurable alongside it under agent.agent_cache.
protect_recent is clamped to half the cache: a couple of sessions can exhaust
the budget on their own, and a fixed MRU guard would then protect everything
and leave the gateway climbing with nothing it would shed.
Fixes#80764
Follow-up to the #80376 salvage:
- TurnLeaseToken.degraded is dead code since acquire() started raising
TurnLeaseTimeoutError: the only constructor site passes the default and
repo-wide there are zero external readers. Remove the field, its ctor
param, the repr segment, and the always-False guards in rebind()/release();
the class docstring's 'retained for compatibility' claim described
consumers that do not exist.
- Revert pure black-rewrap churn in test_config_env_bridge_authority.py and
test_turn_lease.py (hunks on functions this change does not touch), keeping
the functional Windows-env/encoding additions.
- Turn the default-value test into an invariant: config default must equal
gateway.turn_lease.DEFAULT_LEASE_WAIT instead of pinning the 1800 literal.
- Rewrap the dangling 'Released' comment line in gateway/run.py.
Verified: 20/20 targeted gateway tests, ruff clean; mutation check — with
gateway/turn_lease.py reverted to pre-fix main the module fails, restored it
passes.
_handle_media_message had the same gap as _handle_text_message: it didn't
set user_id/user_name or any reply_to_* fields on MessageEvent. A user
replying to a photo/video/audio in Matrix got null sender metadata and
null reply context — the exact bug PR #80293 fixes for text messages.
Mirrors the text handler's reply fallback parsing + sender propagation
into the media handler, and adds a test covering a media reply event.
Also fixes test env mutation: _make_adapter now accepts monkeypatch
and uses monkeypatch.setenv() instead of bare os.environ assignment,
preventing env var leakage across tests in the same xdist worker.
The Matrix adapter built MessageEvent from inbound room events but dropped
the sender's MXID and display name on the event itself -- only 'source'
carried them. Other adapters (signal/slack/telegram/discord/mattermost/irc)
have the same gap; this PR fixes matrix and adds the supporting
top-level MessageEvent fields so the rest can follow.
Downstream effects for matrix specifically:
- gateway prompt assembly can now read event.user_name (or source)
without having to dig into source per platform
- reply context (reply_to_text / reply_to_author_id /
reply_to_author_name) is parsed from the inline > <@user:server> ...
Matrix fallback format before stripping, instead of discarded
- the gateway's existing [Replying to: "..."] renderer can now show
who the user was replying to (was always anonymous for matrix)
MessageEvent gains two optional top-level fields (user_id, user_name,
both default None) so non-IM producers (cron/webhook/autonomous) remain
unaffected. Source still carries the same values for callers that
already read from there.
Tests cover:
- non-reply message carries sender user_id/user_name on MessageEvent
- different senders (alice, bob) both propagate
- reply message carries reply_to_message_id + reply_to_text +
reply_to_author_id + reply_to_author_name, parsed from the
> <@carol:example.org> original question\n\nactual reply shape
- non-reply message does NOT spuriously set reply_to_* fields
Sibling matrix tests (148 across test_matrix*.py) remain green.
Authored by WintleChoung <cwt@users.noreply.github.com>
Salvaged from PR #80293.
Review follow-up for the salvaged #79881/#63963 stack: the shutdown
interrupt fires exactly once, but work can materialize AFTER that one
shot on BOTH sibling paths:
- a /v1/runs task admitted before the drain populates
_active_run_agents only once _create_agent returns
(queued-before-agent window);
- a _running_agents entry claimed as _AGENT_PENDING_SENTINEL is
promoted to the real agent by track_agent() on its own schedule,
after the one-shot walk skipped the sentinel.
Either way the settle loop waited on work nothing signaled, and the
turn went straight to the post-interrupt tool-subprocess kill — the
exact amputation the fix exists to avoid, in a rarer window.
If any work is still live when the settle loop exits, re-invoke
_interrupt_running_agents (which already skips sentinels and folds in
the API-server helper) so late-materializing agents on either path get
the cooperative interrupt. Regression test drives the real stop() path
with an accelerated loop clock and asserts exactly two interrupt
signals.
The shutdown drain ACCOUNTS for API-server work but never INTERRUPTS it.
`_drain_active_agents()` folds `_active_api_run_count()` into both its wait
loop and its `timed_out` verdict, while `_interrupt_running_agents()` iterates
`self._running_agents` only -- a dict no API turn ever enters, because the
API server owns its own agent lifecycle. `gateway/run.py` states the gap
against itself: "API-server / desk sessions have the same structural gap
(#63529)."
The user-visible result is that every gateway restart with a live API or
desktop turn burns the full drain timeout and then runs
`_kill_tool_subprocesses("post-interrupt")`, which amputates the turn's tool
subprocesses with no cooperative interrupt and no resume marker.
There are seven API agent-entry points. Six funnel through `_run_agent()`
(both session-chat routes, and `/v1/chat/completions` + `/v1/responses` in
streaming and non-streaming form) and are counted by `_inflight_agent_runs`;
the seventh, `/v1/runs`, runs its own lifecycle and is counted through
`_active_run_tasks`. None of the six has a run_id, so the run_id-keyed
`_active_run_agents` cannot reach them, and only two pass `agent_ref` -- which
lands in a caller-local list, not a registry.
So register once at the single unconditional creation site inside
`_run_agent`, beside the existing `_publish_turn_process_ownership()` call,
and unregister in the same `finally` that already clears it. That one
symmetric pair covers all six callers. The registry is adapter-owned and
keyed by object identity, kept separate from `_active_run_agents` because
that dict is run_id-keyed and scoped to the public `/v1/runs` stop API.
`interrupt_active_runs()` then walks both registries, deduped by identity, so
the interrupt set matches the set the drain waits on. The settle window after
the interrupt now polls API work as well: the interrupt is cooperative, and
without this the window closes the instant `_running_agents` is empty -- which
it always is for API turns -- and the tool kill lands on a turn that was asked
to stop microseconds earlier.
/retry truncates the live transcript to before the last user message
and persists it via SessionStore.rewrite_transcript, which calls
replace_messages() with the default active_only=False. That DELETEs
every row for the session, including the soft-archived
active=0/compacted=1 rows that in-place compaction keeps on disk
(#38763), so any /retry after a compaction permanently wiped the
archived history. #57803 named this call site as a residual gap after
its global-default approach was rejected; the TUI sibling was fixed
in #80195.
The handler now probes has_archived_messages() (new SessionStore
wrapper, auto-exposed through AsyncSessionStore) and passes
active_only=True when archives exist, so only the live rows are
replaced. rewrite_transcript gains an active_only parameter that
defaults to False, keeping the destructive semantics yuanbao recall
redaction depends on. Also corrects the rewrite_transcript docstring,
which still listed /undo as a caller even though /undo soft-archives
via rewind_session.
The regression test drives _handle_retry_command against a real
SessionStore and SessionDB seeded with archived compaction rows and
asserts the archives survive.
- matrix/dingtalk: extract deps-only installers (ensure_matrix_deps,
ensure_dingtalk_deps) and register THOSE as ensure_deps_fn — the prior
check_*_requirements combined credential env checks with the install,
so a platform configured via PlatformConfig.extra (which is_connected
accepts) would pass enablement, reach create_adapter(), and have the
'installer' veto on env-var grounds before installing anything —
re-creating the #79812 deadlock for extra-configured setups. The
combined deps+credentials functions remain for setup/status callers.
- matrix/feishu passive probes: use the existing lazy_deps.is_available()
instead of hand-rolling 'not feature_missing(...)' (reuse finding).
- teams: module docstring no longer recommends bare system pip (the
PEP 668 trap purged everywhere else); docs troubleshooting row updated
to match the new hint text.
- wecom_callback: drop dead 'global ET, DEFUSEDXML_AVAILABLE'
(ensure_and_bind mutates the module dict directly; nothing assigns).
- tests: parametrized wiring contract for all 8 lazy-installable
platforms — ensure_deps_fn present and distinct from check_fn
(behavior contract, not identity snapshot, so renames don't churn it).
- gateway/config.py: rewrite the stale enablement-pass header comment that
still described check_fn as 'the single source of truth for are-my-env-
vars-set' / 'lazy-installs it' — both false under the new contract.
- teams: check_requirements docstring wrongly claimed credential checks
(body checks only SDK/aiohttp presence); derive install_hint from the
canonical LAZY_DEPS pins + sys.executable instead of hardcoding
'~/.hermes/hermes-agent/venv/bin/pip' and version pins (wrong under
HERMES_HOME overrides / profile installs; pins go stale on CVE bumps);
connect() fatal-error hints now point at the venv pip instead of bare
system pip (the PEP 668 trap the docs warn about).
- teams docs: drop exact version pins from the two manual-install commands
(LAZY_DEPS is the source of truth; unpinned installs still work and the
text can't go stale).
- hermes_cli/status.py: per-entry exception guard around check_fn so one
raising probe can't abort the listing of all remaining plugin platforms
(aligns with the other three call sites).
- tests: rename test_register_check_fn_is_active_lazy_installer ->
test_register_splits_passive_probe_from_active_installer (name said the
opposite of what it verifies).
PlatformEntry.check_fn served three contradictory roles: adapter-creation
gate, config auto-enablement gate, and status display. Plugins had to pick
one function for all three:
- Active installer as check_fn (discord/slack/telegram/matrix/dingtalk/
feishu): every status display could pip-install SDKs as a side effect
(the desktop 94% boot-loop class).
- Passive probe as check_fn (teams, wecom_callback): create_adapter()
returned None before connect() could lazy-install, so the SDK never
installed (#79812 deadlock; wecom_callback's platform.wecom_callback
LAZY_DEPS entry was dead code).
The split makes both call sites correct by construction:
- check_fn is now contractually PASSIVE (probe only, never installs).
- New optional PlatformEntry.ensure_deps_fn is the ACTIVE installer;
create_adapter() runs it exactly when check_fn is False — the platform
is enabled+configured and the gateway is about to connect it.
- Config enablement keeps a configured platform whose deps are missing
but installable; the install itself is deferred to create_adapter().
- Status surfaces (_platform_status, hermes status) read only the
passive probe and can never trigger pip.
Migrated all lazy-installable platform plugins to the split; platforms
with no optional deps (irc/ntfy/buzz/simplex/line/a2a/...) are unchanged
— no ensure_deps_fn means a False check_fn stays a hard block.
wecom_callback gains a working installer for the first time.
Builds on @xxxigm's #79812 (both commits cherry-picked with authorship
preserved), reworking the check_fn swap into the two-field split so the
Teams fix doesn't reintroduce install-on-status.
Platform registry create_adapter() gates on check_fn before the adapter
exists, so wiring the passive probe permanently blocked connect() and
the existing check_teams_requirements() lazy-install never ran.
PATCH /api/sessions/{id} only accepted title and end_reason, so the
`pinned` flag the desktop sends was rejected as an unsupported field —
and the client swallows that error. Pins lived in one app's localStorage
and never reached state.db, which also meant the server-side auto-archive
sweep was free to hide the chats a pin exists to keep.
Accept pinned and archived as booleans, route them to the SessionDB
setters that already existed, and include both in the serialized session
so clients can reconcile against server truth.
Follow-up to c0d974b19 (#79741). Three review findings against that commit,
none of which change the escalation behaviour it shipped.
1. The recovery decision lived inline in `_handle_message_with_agent`, a
~2000-line async method, so the only way to pin it was a test that read
`inspect.getsource(...)` and asserted on substrings. AGENTS.md bans reading
source in tests outright and names this exact situation: "if the logic lives
inline in a god-file (gateway/run.py) and extracting it feels disruptive:
that's the actual signal to do the extraction, not to regex around it."
Those tests were not merely stylistically wrong, they were actively harmful.
One asserted the substring `_new_tokens < _approx_tokens` was PRESENT -- so
it passed while the gate had the bug that substring represents, and had to be
edited when the gate was fixed. It failed on correct code and passed on
broken code, in one assertion.
Extracted `hygiene_compaction_recovered()` as a module-level pure predicate
and replaced the three source-reading tests with eight direct unit tests.
The extraction immediately earned itself: the new tests caught a `NameError`
(the predicate called `compression_made_progress` while the module bound it
under an alias) that a source-text assertion cannot see, because the symbol
is spelled correctly in the source and only fails at runtime.
2. The gate inferred "did the transcript actually get rewritten" from a numeric
side effect -- the degenerate "did not rotate or compact in place" path
(#21301) reuses the pre-compression counts -- when the booleans
`_hyg_rotated` / `_hyg_in_place` were already in scope and explicitly set
False on that path. The predicate now takes them directly, so a future edit
that re-estimates instead of reusing the old counts cannot silently defeat
the escalation.
3. `_record_hygiene_cooldown` passed no `error` to
`record_compression_failure_cooldown`, which writes `compression_failure_error`
unconditionally -- so a hygiene failure clobbered to NULL whatever reason the
in-conversation path had recorded, and readers then show the user "unknown
error" (agent/manual_compression_feedback.py, gateway/slash_commands.py). The
reason was already in hand at both call sites. Pre-existing from #74136 but
amplified by escalation: a blank reason on a 45-minute cooldown is far more
user-visible than on a 5-minute one.
Also: the ladder docstring described the compressor's absolute 60/300/900s
ladder while the constant is multipliers (1, 3, 9); the config docs still
described `hygiene_failure_cooldown_seconds` as a flat interval rather than the
first rung of a capped ladder; and `PersistentState.hygiene_failure_streak` now
documents that it is process-local by design -- keying on `session_key` is what
survives compaction rotation, which the persisted `compression_*_streak`
columns cannot express since they key on the rotating `session_id`. Making it
durable is a schema change, tracked on #79624 rather than smuggled in here.
Also replaces the file's hand-written `_Runner` stub with
`object.__new__(GatewayRunner)` (already the idiom elsewhere in the same file).
The stub reimplemented `_session_state` and `_peek_session_state`, so the tests
exercised copies that could drift from production; using the real class
immediately made one assertion stronger -- on a fresh runner `_sessions` does not
exist at all until something materialises it, so the reset provably did not even
create the map.
A review pass on this follow-up then caught that the CALL SITE was still
unbound: deleting the whole `if not _hyg_aborted: if
hygiene_compaction_recovered(...)` block left every ladder test green, because
the unit tests prove the predicate correct without proving it is wired in. The
merged commit had the same gap and its only cover was the banned source-reading
test. `test_session_hygiene_forces_in_place_compaction_with_bound_session_db`
now spies the reset on a genuine in-place compaction, so deleting the wiring
fails. Two earlier attempts at this test did NOT close the gap -- asserting on
streak VALUES passes either way, since the streak is 0 whether or not the gate
ran; only a positive spy assertion on a recovering run detects the deletion.
Same pass also corrected an overstatement: point (2) is hardening, not a live
bug. The degenerate path also sets `_new_count = _msg_count` and `_new_tokens =
_approx_tokens`, and `compression_made_progress(n, n, t, t)` is always False, so
the merged code already declined to reset there. A 200k-trial fuzz over the
reachable state space found zero behavioural disagreements between the merged
gate and this one. The guard's value is surviving a future edit that stops
reusing those counts.
Tests: 28 in tests/gateway/test_hygiene_failure_cooldown_ladder.py (8 new unit
tests for the predicate, 3 for reason forwarding, 3 source-reading tests
deleted). All 5 mutations caught -- including one that restores the hand-rolled
comparison and one that removes the rotated/in_place guard. Two mutations
initially SURVIVED and exposed vacuous tests of my own: the no-rewrite test used
counts the progress predicate already rejects, so it passed without binding the
guard at all; it now passes counts that read as progress on their own, proving
the guard is what rejects them. gateway hygiene + session-state + agent
compression-progress suites: 54 passed; ruff clean.
Refs #79624
A gateway session whose summary model keeps timing out no longer retries
compaction on the same fixed interval forever.
The in-agent compressor already escalates repeat summary timeouts
60 -> 300 -> 900s (ContextCompressor.record_timeout_failure), but that ladder
reads the in-memory _consecutive_timeout_failures counter and
bind_session_state() zeroes it (context_compressor.py:1645). Session hygiene
constructs a FRESH AIAgent for every run (gateway/run.py:16820) and re-binds
state each time, so from the gateway that streak is structurally always 0 --
only the flat hygiene_failure_cooldown_seconds (300s) could ever be recorded.
Issue #79624 reported exactly that steady state: an oversized session
(1053 messages, ~119.5k tokens) whose aux model always timed out, re-attempting
compaction every 300s across five days until the reporter deleted the session
by hand.
Track the streak on PersistentState instead, which outlives the per-run agent
and is not cleared by turn/boundary resets, so consecutive hygiene failures
climb 300 -> 900 -> 2700s and then saturate. Both failure sites (progress
timeout and aborted compression) feed it; a real compression resets it, so a
session that recovers starts from the first rung again. The ladder multiplies
the configured base, so operators who tuned
hygiene_failure_cooldown_seconds keep their first rung. Per-session, so one
wedged chat cannot penalize other conversations.
Deliberately NOT changed, since each is a maintainer policy call rather than a
defect (all three are written up on #79624):
- no durable failure-streak column, so escalation still resets on restart
- the gateway 30s / in-agent 120s / aux-client 300s-floor timeout mismatch
- no `hermes doctor` check or `hermes sessions list` marker for a session
stuck in a compression-failure cooldown
Note the reported exit(1) is NOT a crash: it is the deliberate
_signal_initiated_shutdown path (gateway/run.py:26746-26751, #5646) that lets
systemd Restart=on-failure revive the gateway after a bare SIGTERM, and it
fires on every `systemctl restart` independently of compaction. The compaction
log lines appear after the shutdown line because the gateway-owned executor is
torn down with shutdown(wait=False, cancel_futures=True) (run.py:21164), so an
in-flight turn keeps logging during teardown. Full analysis on the issue.
Post-review hardening (Phase 2c + /simplify-code found five real defects in the
first cut):
- the recovery gate hand-rolled `_new_tokens < _approx_tokens` when a canonical
predicate already existed: `compression_made_progress` (agent/turn_context.py,
#39548). They disagree on 3 of 5 cases -- the hand-rolled form misses a
row-count win when the summary keeps the token estimate flat, misses one
where the summary is slightly MORE verbose (so a genuinely recovered session
would keep escalating forever), and counts a sub-5% wobble as recovery. Now
reuses the shared predicate, promoted from `_compression_made_progress` to a
public name with the old private name kept as a back-compat alias so the
existing importer (tests/agent/test_protected_tail_pressure_61932.py) and any
patcher of that symbol keep working.
- the reset was gated on "not aborted", but the degenerate "did not rotate or
compact in place" branch (#21301) is NOT aborted and yields zero reduction,
so a session wedged there reset its streak every run and could never
escalate -- silently defeating the fix. Now gated on real progress.
- no absolute ceiling: base * 9 reaches 9h at an operator base of 3600s,
indistinguishable from "compaction switched off". Added
_HYGIENE_COOLDOWN_MAX_SECONDS = 3600, mirroring the in-file
_RECONNECT_BACKOFF_CAP precedent.
- the reset used the get-or-create accessor to write a 0 that was already 0,
materialising a _sessions entry (never evicted). Now peeks.
- the abort verdict was probed twice, leaving the reset/record mutual
exclusion implicit; a future await between the probes would have broken it
silently. Computed once into _hyg_aborted.
Tests: 19 new in tests/gateway/test_hygiene_failure_cooldown_ladder.py --
ladder escalation, saturation, the absolute cap, per-session isolation,
reset-on-recovery, custom/zero base, PersistentState scoping (a mutation moving
the field to TurnState fails), degraded runners, the progress gate, the exact
progress-predicate semantics the gate depends on, and end-to-end that the
escalated value is what reaches the state DB. All 12 mutations caught, including
ones that restore the flat cooldown (the original bug), ungate the reset, swap
the canonical predicate back for the hand-rolled comparison, remove the cap, and
share the streak globally; the harness hard-errors when a mutation cannot be
applied, since a silently no-op mutation check is worse than none -- an earlier
version of it WAS silently no-opping after a refactor. The gate's contract test
slices by AST node span rather than a fixed character count, which had already
truncated once as the block grew. gateway hygiene + session-state + the three
touched agent compression suites: 50 passed; ruff clean.
E2E with real imports demonstrates the premise rather than asserting it:
bind_session_state zeroes the in-agent counter, and the recorded deadlines go
300 -> 900 -> 2700 -> 2700 -> 2700s where they were previously a flat 300s.
Reported by @yucezerey (#79624), whose state.db column dump and
"deleting the session fixed it" datapoint made the real mechanism findable.
Follow-up to #79669. That PR routed the three fallback recorder sites through
_record_turn_final_payload so a split turn would record the unsplit ledger
instead of a tail-only payload. For two of them that is right. For
_send_empty_fallback_final it is wrong, and it reintroduces the #78541 swallow
at the one site that was supposed to be fixed.
_send_empty_fallback_final is a *replacement* recovery: it sends the completed
text as a fresh message and deletes every tracked segment preview -- which on an
overflow split includes the sealed head chunks. After it runs, the only thing
on screen is the message it just sent. Recording the ledger there claims
delivery for text the same function just removed, so delivered_final_matches()
returns True, the gateway suppresses its own send, and the user is left with a
fraction of the answer.
Observed with a probe driving the real run() loop (543-char reply, 475-char head
sealed then deleted, 67-char tail committed):
before this fix recorded=543 matches=True -> suppressed, 67/543 on screen
after this fix recorded=67 matches=False -> gateway sends the full answer
Record final_text verbatim here instead. The sibling site in
_send_fallback_final keeps the recorder: its delete is gated on
`continuation == final_text` and targets only the single active partial, never
the sealed heads, so the ledger correctly describes what survives.
The distinction is whether a recovery ADDS to what is on screen or REPLACES it.
Additive paths may record the ledger; replacing paths must record only what they
leave behind. _try_fresh_final is the same shape and #79669 handled it by
refusing the route on split turns.
Test drives the real seal-then-delete sequence and asserts the mismatch, so the
gateway is required to re-send. Mutation-checked: restoring the recorder call
turns it red.
The salvaged fix changed only the gateway's verdict: a payload-less
multi-message split stopped inheriting legacy trust. But six code paths set
_turn_split_delivery, and only one of them was taught to record a payload, so
the remaining five swapped the swallow for the opposite defect.
Fix the producers instead of only distrusting them at the boundary:
- _send_or_edit failed-final-edit branch: record the visible payload on split
turns too. It deliberately skipped recording, which now reads as a mismatch
and re-sends an answer already on screen -- reintroducing the duplicate
#45517 fixed (#36965 / #25349).
- _send_fallback_final (x2) and _send_empty_fallback_final: route through
_record_turn_final_payload instead of assigning _delivered_final_text
directly. On a split turn their final_text is only the trailing chunk, so a
fully delivered heads+tail reply recorded a tail-only payload and was
re-sent in full.
- _try_fresh_final: refuse the fresh-final route once a head chunk is sealed.
It replaces every tracked preview with one message, which only holds the
whole answer on a single-message turn. After a split it deleted the sealed
heads while sending just the tail, so the complete reply was still lost --
on Telegram, the default finalize route and the shape #78541 reports.
- Set _turn_split_delivery at seal time rather than after the tail send, so
the tail's own finalize sees the split state. The sibling overflow path
already did this; the divergence is what let fresh-final delete the heads.
- run.py stale-finalize reconciliation: skip the in-place edit on a split
delivery. message_id is only the LAST chunk there, so editing it with the
complete response repeated every sealed head's text inside the tail
message. Fall through to the normal final send.
Also drop a dead `or "".join(chunks)` fallback (all growth funnels through
_append_accumulated, so the ledger is never empty at that call site, and joined
chunks carry injected fence markers that could never match final_response), and
document that _record_turn_final_payload intentionally ignores its argument on
split turns.
Tests: four end-to-end cases driving the real overflow-split loop instead of
hand-setting private flags -- complete split still suppresses (no duplicate),
split missing a tail does not suppress, fresh-final keeps sealed heads, and a
flood-controlled final edit after a split stays suppressed. Each was
mutation-checked: reverting any individual fix turns its test red.
The pre-existing gateway-boundary test asserted the recovery *route* (the
reconcile edit) rather than the guarantee. Relaxed to the real contract: either
_run_agent puts the complete text on the wire, or it declines to claim delivery
so the caller's normal final send does.
Co-authored-by: HexLab98 <liruixinch@outlook.com>
* fix(discord): reject empty outbound messages
* test(discord): cover empty final reply backfill state
Missed-message backfill decides what to replay from discord_messages, so
a dropped final reply must be recorded as failed by the new guard the
same way the exception path records one — otherwise the reply is both
never sent and never retried.
Co-authored-by: Jony <619963502@qq.com>
* chore: map 619963502@qq.com to zyz619963502zyz for PR #73449 salvage
---------
Co-authored-by: Jony <619963502@qq.com>
Lock in backend preference, wire-name aliasing, and normalize mapping
so configured non-xai search providers stay on the Hermes client path.
Also init conflict-recovery generation on the telegram bare-adapter
helper so CI polling progress tests do not AttributeError.
#75017: Telegram polling conflict retry used drop_pending_updates=False,
starting a new getUpdates session that immediately got 409'd by the
previous still-expiring session — creating the very conflict it was
trying to recover from. Switch to drop_pending_updates=True so Telegram
terminates stale sessions. Also add a recovery-generation guard so the
first transient getUpdates success after a retry doesn't reset the
conflict counter back to 0 (defense-in-depth from PR #75096).
#75153: The WAL-reset warning always said 'hermes update can repair'
even for git/pip/system Python installs where it can't. Now uses
detect_install_method() + recommended_update_command_for_method() to
give a context-appropriate hint (hermes update for git, docker pull for
docker, nix message for nix, generic install hint as fallback).