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).
CI caught a missed caller-shape update. Both PRODUCTION callers of
_write_sse_chat_completion / _write_sse_responses were converted to
ThreadSafeAsyncQueue, but two pre-existing tests in
tests/gateway/test_api_server.py construct the writer's queue
themselves and still passed a stdlib queue.Queue.
The consumer now does 'await asyncio.wait_for(stream_q.get(), ...)',
which on a queue.Queue blocks the thread forever:
test_stream_cancelled_persists_incomplete_snapshot hung until
pytest-timeout killed it (CI reported the whole file as 'no tests
ran (timeout before collection)'). The sibling disconnect test only
survived because it pre-fills before the first await.
tests/gateway/test_api_server.py: 99 passed (was 1 failed + a 60s
hang); with the SSE/api_server suites: 147 passed.
Gate finding (/simplify-code pass): both cross-thread tests passed
loop=loop explicitly, but no production caller does — all six
(_on_delta, _on_tool_*) rely on the queue resolving its own
_loop_ref in __init__. The kwarg made the tests vacuous: a broken
_loop_ref still passed them.
Dropping the kwarg exercises the real path. Verified by mutation:
with self._loop_ref = asyncio.new_event_loop() (wrong loop), both
tests now FAIL; they passed before this change.
The conflict-marker strip fused test_agent_task_raises with the body of
test_failed_result_dict — restore both as separate tests (content from
the PR head, verified verbatim).
Addresses teknium1 sweeper review (2026-07-30) requiring coverage of:
1. ThreadSafeAsyncQueue.put_threadsafe() off-loop boundary: a real
daemon thread pushes into the queue from outside the owning event loop
while the consumer awaits get(), mirroring the run_conversation
worker-thread producer path. Includes a 20-concurrent-thread
no-drop regression.
2. Long-reasoning bound stability for thinkingPreview: 100k-char input
plus empty/collapsed cases must not crash and must retain the visible
tail marker inside the bounded 24k clean window.
The session event stream (api_server.py:~2236) was the one genuinely
unicode-distinct SSE writer — json.dumps(payload, ensure_ascii=False) +
.encode('utf-8'). Every other writer uses plain json.dumps. Route it
through _sse_frame(..., ensure_ascii=False) so _sse_frame is now the single
source of truth for ALL SSE frame serialization in the module (chat-
completion, responses._write_event, /v1/runs, and the session stream).
Byte-identical for non-ASCII payloads: verified against the historical
inline encoder (raw bytes preserved). The ensure_ascii=False path is now
exercised by test_sse_frame_ensure_ascii_false_reproduces_session_event_stream.
_extend _sse_frame with an explicit ensure_ascii param (default True,
byte-identical to a bare json.dumps) and route the two sibling writers
through it: _write_sse_responses._write_event and the /v1/runs event
stream. This completes the dedup PR #65009 — previously only the five
_write_sse_chat_completion sites used the helper, leaving the other two
writers on inline json.dumps with no shared shape.
No behavior change: every writer's emitted bytes are unchanged (verified
byte-for-byte, including non-ASCII payloads where the default
ensure_ascii=True matches the original inline encoders). The ensure_ascii
option is exposed so a future writer can opt into raw non-ASCII bytes
without fractalizing the format again.
Adds tests/gateway/test_sse_frame.py asserting the byte-contract
invariant between _sse_frame and the historical inline encoders.
_write_sse_chat_completion and _write_sse_responses bridged their
stream_delta_callback queue into the event loop via
`await loop.run_in_executor(None, lambda: stream_q.get(timeout=0.5))`
in a while-True poll — a thread-pool round trip on every 0.5s tick even
when idle, plus up to 500ms of tail latency between a delta landing in
the queue and it reaching the SSE response.
Add ThreadSafeAsyncQueue (asyncio.Queue + a put_threadsafe() that wraps
call_soon_threadsafe), used by both streaming producer closures
(_on_delta, tool start/complete callbacks — all invoked from the worker
thread running run_conversation via loop.run_in_executor). Consumers
now do a plain `await asyncio.wait_for(stream_q.get(), timeout=0.5)` —
woken immediately when a delta arrives, no executor hop, no poll
interval.
Updated tests/gateway/test_sse_agent_cancel.py's 7 call sites to
construct ThreadSafeAsyncQueue inside the running loop (required, since
it captures asyncio.get_running_loop() at construction) instead of a
bare queue.Queue() at test-method scope.
Offload manual /compress temporary-agent cleanup through the existing
bounded off-loop helper so a slow agent.close() cannot freeze the
gateway event loop, heartbeat, or platform polling.
Guarantee Relay transport teardown even when the runner cancels
adapter.disconnect() during go_idle: shielded finally, 2s drain-path
idle ACK budget under the 5s outer disconnect budget, and bounded
supervisor/reader/ws.close awaits.
Original commits:
- fix(gateway): offload manual /compress cleanup from the event loop
- fix(gateway): tear down Relay transport even if go_idle is cancelled
- fix(gateway): keep Relay disconnect budgets inside the runner window
By @Dannyzen (PR #78027), salvaged onto current main.
`DiscordAdapter.disconnect()` cancelled the bot task before tearing down voice
clients. `leave_voice_channel()` ends in `await vc.disconnect()`, and discord.py
sends a voice state update over the main gateway websocket and then waits for the
voice socket to close. The bot task is the loop running that gateway connection,
so cancelling it first left the handshake with no transport: it could never
complete and blocked until the caller's shutdown timeout fired.
The effect was a fixed ~5s penalty on every shutdown with a voice connection
open, ending in "discord disconnect timed out after 5.0s - forcing continue",
with the voice disconnect abandoned rather than completed.
Measured on a live gateway with a voice connection open in both cases:
before: timed out after 5.0s, all adapters disconnected at +5.29s
after: discord disconnected (0.12s), all adapters disconnected at +0.46s
Moving the voice-cleanup loop above `_cancel_bot_task()` preserves the
zombie-client protection its comment describes: the bot task is still cancelled
before `client.close()`, just after voice teardown rather than before it. Voice
teardown is the one step that still requires a live gateway.
Adds a regression test asserting the ordering. It fails on the previous ordering
at index 1 with `cancel_bot_task != leave_voice_channel:111`.
Fixes#76044
When a Discord channel message initiates a relay auto-thread, the thread does
not exist at ingest (source.thread_id is None) — the connector creates it on
its FIRST send and auto-threads any outbound carrying the reply anchor. The
final reply carries that anchor, so it lands in the thread. But the
tool-progress / status bubbles (the "Searching the web for..." updates and the
streaming preamble) were sent with _progress_metadata=None and
_progress_reply_to=None: _resolve_progress_thread_id returns None for Discord
(only slack/mattermost get a synthetic thread), so the progress send had no
anchor and the connector posted it FLAT in the parent channel. Result: the
search-status updates leaked outside the thread while the answer threaded
(staging repro 2026-08-02).
The connector now stamps prospective_thread_id on the inbound (the anchor
message id == the id of the thread it will create). Reuse it: when a
relay-delivered Discord channel-initiate carries prospective_thread_id and has
no real thread yet, carry the reply anchor (event_message_id) on both the
progress metadata (reply_to_message_id) and the progress reply_to, so the
connector routes the progress bubble into the SAME auto-thread as the final
reply. Applied to both the tool-progress path (_progress_metadata /
_progress_reply_to) and the status/interim callback path
(_status_thread_metadata). Events already arriving in a real thread, DMs, and
non-relay sources are untouched (guarded on delivered_via_upstream_relay +
prospective_thread_id + not thread_id).
Tests: two new cases in test_run_progress_topics.py — a relay Discord
channel-initiate asserts every progress send carries the anchor (reply_to +
metadata.reply_to_message_id + non_conversational), and an event already in a
real thread asserts the synthetic-anchor path does NOT engage. Full gateway
progress + relay + session suites green (228 passed).
Salvage of #26860 (hunk 2, ported \u2014 the PR's base predates the current
gateway layout by ~11.9K commits). Messaging platforms can set
gateway.platforms.<key>.skip_context_files: true to skip the
filesystem-heavy context-file discovery (SOUL.md, AGENTS.md,
.cursorrules walks) during AIAgent construction \u2014 10-100x slower
stat()/walk costs on Windows made this a real per-turn tax. Soul
identity is still loaded (single small file), so the persona survives.
The flag participates in _agent_config_signature so toggling it
rebuilds the cached agent instead of silently reusing a prompt built
under the other setting (prompt-cache correctness).
The PR's hunk 1 (mtime-caching the per-turn dotenv reload) was dropped:
df51ad797 mtime-cached load_config/read_raw_config and c2eda92fd
removed the per-turn deepcopies, capturing most of that win; the
function has since gained a multiplex early-return and managed-scope
overlay that the original whole-function skip would have bypassed.
Follow-up on the salvaged pair: the original guard's `not msg_id` arm let an
id-less internal/synthetic event erase a tracking entry a concurrently-queued
id-bearing message's drain task still needs for recall matching (id-less
events never write entries in _dispatch_inbound_event, so they must never
pop). Tests cover: normal cleanup, id-less non-erasure, overwritten-entry
ownership handoff, TTL eviction + fresh-entry survival.
Two places were using SELECT COUNT(*) when they only needed a boolean:
- has_any_sessions() called session_count() > 1 (full table scan)
- delete_session() used SELECT COUNT(*) WHERE id=? (full matching scan)
Fix:
- Add session_count_ge(n) to SessionDB — short-circuits via
SELECT 1 FROM sessions LIMIT n, returns bool
- has_any_sessions() uses session_count_ge(2) instead of session_count() > 1
- delete_session() uses SELECT 1 ... LIMIT 1 with fetchone() is None
- Add tests for session_count_ge
CI exposed the whole class: feishu tests across MANY files (thread
routing, text batching, sdk executor, ...) inject a mock _client and
skip connect(), so the deferred import leaves the request-builder
globals None. Replace the single-file setUpModule with a session-scoped
autouse conftest fixture that binds the globals once when lark_oapi is
installed; when it isn't, the affected tests already skip via their own
skipUnless guards. Full tests/gateway run: zero failures beyond main's
pre-existing baseline (sorted failure-diff).
Salvage of #57657, ported onto the plugin layout (the adapter moved
from gateway/platforms/feishu.py to plugins/platforms/feishu/adapter.py
since the PR's base). lark_oapi takes seconds to import and holds the
GIL doing it; the module-level import made every gateway boot pay that
cost even with Feishu unconfigured.
- _load_lark_oapi() with double-checked locking binds the SDK globals
on first use; connect() and _standalone_send() call it via
asyncio.to_thread so the loop never blocks on the import.
- probe_bot() also calls _load_lark_oapi() (sync context) so the SDK
probe path is preserved rather than silently degrading to the HTTP
fallback before a first connect.
- check_feishu_requirements() is install-only and no longer rebinds
globals; test_feishu.py gets a setUpModule that binds them eagerly
for tests that inject fake clients.
Includes the dedicated lazy-import test file (check-does-not-import,
connect-loads-on-worker-thread).
The runtime footer (`/footer`) shows what model ran and how full the context
is, but not how long the turn took. On a messaging platform there is no
progress bar and no shell timer — a turn that took 4 seconds and one that took
four minutes produce visually identical replies. Users comparing models,
providers, or reasoning levels have no at-a-glance signal for the one
dimension they most often care about, and "was that slow or did I imagine it?"
is unanswerable after the fact.
Adds a `latency` field to the existing footer machinery, rendering the
wall-clock duration of the agent run: `<1s`, `22s`, `1m05s`.
`gateway/run.py` measures with `time.monotonic()` immediately around the
`self._run_agent(...)` await in `_handle_message_with_agent` — the same
function that already builds the footer, so the value is the user-perceived
turn duration (monotonic, so it is immune to wall-clock/NTP adjustment).
`latency` is deliberately NOT in `_DEFAULT_FIELDS`. It is opt-in via
`display.runtime_footer.fields`. Every existing footer — and every footer a
user has today without touching config — renders byte-identically.
This is enforced by tests, not just asserted:
- `test_latency_not_in_default_fields` pins the default tuple.
- `test_resolve_footer_config_default_fields_exclude_latency` pins what
config resolution produces for an untouched config.
- `test_default_footer_renders_byte_identically` pins five exact output
strings for default-config renders **while supplying `turn_seconds`** —
proving that even when the caller measures timing, a default-configured
footer does not show it.
- `test_default_build_footer_line_ignores_turn_seconds` asserts
`build_footer_line(...) == build_footer_line(..., turn_seconds=125.0)`
under default fields.
Adding `latency` to `_DEFAULT_FIELDS` fails 11 of these tests.
No new config surface (reuses `display.runtime_footer.fields`), no new env
vars, no new core tool, no new model-facing schema. One new module-private
helper (`_format_latency`), one new keyword argument threaded through the two
existing footer functions, and 3 lines in `gateway/run.py`.
`turn_seconds` defaults to `None` and the field is skipped when it is `None`
or negative, so any call site that does not measure timing keeps working
unchanged.
`tests/gateway/test_runtime_footer.py` (+185): `_format_latency` boundary
table (sub-second, rounding at 59.4/59.6, the `m{:02d}s` zero-pad, 60m), the
render/skip/opt-in matrix, field-order placement, `build_footer_line`
threading, and the byte-stability block above.
RED-proved by mutation — each of these breaks tests:
- `latency` added to `_DEFAULT_FIELDS` → 11 failures
- dropping the `turn_seconds is not None and >= 0` guard → 2 failures
- `{sec:02d}` → `{sec}` → 6 failures
- `build_footer_line` not threading `turn_seconds` → 1 failure
51 passed in `tests/gateway/test_runtime_footer.py`; 54 passed across the
footer blast radius. `ruff check` clean.
Two consumer tests assumed a send completes synchronously within the
handler turn: the continuation-drain test polled on handler-call count
then immediately asserted on adapter.sent, and the split-brain heal
test drained with bare zero-delay yields. With ledger calls hopping to
worker threads around each send, the reply can land microseconds after
those checks. Poll for the actual sends with a bounded 2s window —
same invariants, scheduling-robust.
CI slices failed the offload tests with 0.5s witness timeouts: on a
loaded shared runner the event loop thread can take >0.5s to get
scheduled even when NOT blocked, making the probe report a false
positive. A genuinely blocked loop can never set the progress event at
any timeout (the witness coroutine can't run at all), so 5s only
absorbs scheduler flake without weakening the invariant. Mutation
re-verified: reverting the offload still fails all 4 tests.
The sweep-path test parametrizes over _runner/_adapter, which live on
TestGatewayRedeliverySweep; main later added
TestUnconnectedPlatformKeepsItsBudget at the cherry-pick anchor point and
the test landed in that class, where the helpers don't exist
(AttributeError x2). Placement-only move.
Wrap the masked-link destination in angle brackets so Discord does not
unfurl an OG-preview embed under every tool progress bubble. quote()
percent-encodes any <> inside the URL itself, so the wrapper cannot be
broken out of.
The account was written under the new pickle key before sessions were
re-pickled. The account is effectively the migration's commit marker —
once it reads under the current key, the fast path short-circuits every
later startup — so a sweep that errored or was interrupted left the
remaining legacy-key sessions stranded permanently with no retry.
Sweep first, commit the account last, and return False on sweep failure
so the migration is retried on the next start.
Also corrects the unreadable-row log: it claimed rows were being dropped
while no DELETE was ever issued. Such rows are left in place (already
unusable; deleting crypto material on a guess is not worth it) and the
message now says so.
Adds session-sweep coverage, which was previously absent: rows rewritten
under the current key, rows already current left alone, unreadable rows
left in place, and a failed sweep that leaves the account uncommitted.
The existing migration test now fakes the olm C-extension so the suite
no longer requires libolm.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Olm account pickle key is derived from the account ID plus the
configured device ID (acct:device_id). If the crypto store's account was
created before MATRIX_DEVICE_ID was set — e.g. the very first password
login, where the device ID is only known after connecting — it gets
pickled under "<acct>:default". Setting MATRIX_DEVICE_ID afterwards (a
reasonable thing to do once you know the device ID you want to pin)
changes the derived pickle key, and every subsequent unpickle attempt
fails with BAD_ACCOUNT_KEY. In optional-E2EE mode that failure is
swallowed and encryption silently stays disabled instead of surfacing an
actionable error.
_migrate_legacy_crypto_pickle() detects the BAD_ACCOUNT_KEY failure,
tries the known legacy pickle keys, and re-pickles the account (plus
every stored olm/megolm session — sessions share the same pickle key, so
migrating only the account would leave them unreadable on the next
decrypt and silently break key sharing with peers) under the current
key. It only reports failure when no known key can unpickle the
account, with a log message pointing at what changed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
connect() resolved client.device_id as `self._device_id or resolved_device_id`,
so a configured MATRIX_DEVICE_ID masked the live whoami() device. With
persisted A, configured A and a rotated token reporting B, the reset
compared A to A and never fired — exactly the token-rotation case this PR
claims to handle.
An access token is bound to one device and the homeserver only accepts key
uploads for that device, so a configured value naming a different one
cannot work. The live whoami() device now wins on conflict and logs an
error naming both. The configured value is still preferred when whoami()
reports no device.
Adds a connect()-level regression for persisted A + configured A +
whoami B, and corrects test_connect_uses_configured_device_id_over_whoami,
whose stated premise this inverts.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The crypto store is keyed by Matrix user ID, not device ID, so swapping in
a new access token (which mints a new device_id) silently inherits the
previous device's Olm account. That account's identity keys can never be
published under the new device ID, and the pickle key embeds the old
device ID anyway — the result is stale-key mismatches and cross-signing
signatures the homeserver refuses to replace, degrading E2EE in ways that
are hard to diagnose (peers silently withhold room keys).
_reset_crypto_store_if_device_changed() compares the store's persisted
device ID against the live one at connect time and wipes the store on
mismatch, so a fresh Olm account is generated for the new device instead
of reusing stale key material.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up for the salvaged #29239: regression test drives the real
gateway _reload_runtime_env_preserving_config_authority() path with a
stale .env TERMINAL_ENV=docker vs config.yaml terminal.backend=local,
and the hermes debug dump test that pinned the old stale-env-wins
symptom now pins the fixed contract (config wins, override line kept
as defense-in-depth for post-load env mutation).
A wedged adapter transport (network hang, dead websocket) previously
blocked _check_session_stalls forever: sibling candidates in the same
pass were never evaluated and the watcher stopped ticking. Wrap the
send in asyncio.wait_for (15s); on timeout log a WARNING and do NOT
latch, so the next tick retries. Regression uses a never-resolving fake
adapter and proves the pass completes, a healthy sibling candidate is
still notified in the same pass, and the watcher ticks again
(sabotage-verified against the unbounded send).
- Config docs now describe session_stall_timeout precisely: a RECOVERY
notifier for an in-process AIAgent with an adapter-queued follow-up —
not a general gateway/session stall detector — with a per-AIAgent scan
cadence (not globally coordinated per durable session).
- import_sessions documents the deliberate export-includes /
import-resets asymmetry for the activity fields (no resurrected
'working' labels on machines where no agent runs), with a regression
pinning both halves.
- Strip trailing whitespace in contributors/emails/fangliquan@qq.com
(git diff --check housekeeping).
PR #76354 review, scope/contract items + housekeeping.
The stall watchdog gathered pending/activity candidates and later sent
the recovery notification from that aging snapshot — an agent that made
progress (or drained its queue) between the scan and the send received a
false stall notice mid-recovery.
Re-read the adapter pending slot, the overflow queue, and the live
activity snapshot immediately before delivery; abort the send and re-arm
the latch (pop it) when the candidate is no longer stale, so a future
genuine episode still notifies.
Race regressions: progress between scan and send aborts delivery;
pending-drained between scan and send aborts delivery; a genuinely
still-stale candidate is still delivered exactly once.
PR #76354 review, 'watchdog can send /new using a stale snapshot' /
merge gate 8.
Activity heartbeat writes and turn-end label clears ran synchronously on
the response-critical path with the full ~20s routine write-patience
budget — under contention an otherwise-finished reply could stall for
seconds just to update observation labels, mimicking the very stall the
watchdog detects.
touch_session_activity and clear_session_activity_labels now use a
dedicated 0.5s patience budget (they are observation-only; the next
heartbeat window retries naturally), and a no-op label clear (labels
already empty) skips the write transaction entirely.
Regressions: with another connection holding BEGIN IMMEDIATE, both writes
give up well under the routine budget; the no-op clear performs zero
write transactions.
PR #76354 review, 'activity writes are synchronous on critical paths' /
merge gate 9.
Three mechanisms to detect and notify when gateway sessions stall silently:
1. Mid-turn activity heartbeats stamped to SessionDB so hermes sessions list
and hermes status show progress during long turns without new message rows.
2. Stall watchdog: when a busy session has pending inbound and the shared
activity clock is idle past agent.session_stall_timeout (default 300),
log a WARNING and notify the user once to try /new. Notify-only; does
not kill the turn.
3. Compaction timeout: fenceless compress_context callers get a progress-aware
host budget (compression.context_timeout_seconds default 120 idle,
compression.context_total_ceiling_seconds default 600 ceiling). On timeout,
cancel via commit fence, skip compaction without dropping messages, and
continue the turn.
Closes#72016 (slices 1-3; slice 4 cumulative SSE stream-retry deadline
remains a follow-up).
Cherry-picked from PR #72424 by @fangliquanflq.
Builds on the adapter list_channels() hook (cherry-picked from #43545 by
@Guoen0):
- plugins/platforms/simplex: implement list_channels() — enumerates
contacts (/contacts) and groups (/groups) over the live daemon
WebSocket into the channel directory. Returns None when the WS is
down so the directory falls back to session discovery instead of
wiping known targets.
- hermes send --list: merge configured-but-undiscovered platforms into
the listing. Previously a platform configured only via env (e.g. a
fresh SimpleX setup used for outbound sends) was silently omitted,
leaving users guessing at platform names.
- format_directory_for_display(): accept an explicit platforms view and
render empty platforms with a targeting hint instead of hiding them.
- docs: simplex hermes-send section.
Reported by Fedpostoffice on Discord (simplex missing from
hermes send --list; guessed platform names simplex-chat/simplex-relay).
The Discord semantic thread-rename lane resolved the target thread from
`_relay_auto_thread_info`, which read a single-slot-per-parent-chat cache
(`adapter._auto_thread_by_chat[chat_id]`, populated from connector
SendResult feedback). When two auto-threads spawned from the SAME parent
channel, the second send overwrote the first's slot and the title turn's
read raced the write — so only the FIRST thread in a channel ever got its
semantic rename. Staging repro 2026-08-02: message A's thread renamed to
"A Hundred Word Sword Story", sibling message B's thread stayed stuck at
the raw first-words name.
The connector now stamps `prospective_thread_id` on the inbound (the anchor
message id, which is the id of the thread it will auto-create) — shipped for
per-thread session keying. Reuse it here: it is deterministic and
per-message, so it names the EXACT thread even when several auto-threads
share one channel. `_relay_auto_thread_info` returns it directly (with an
empty initial-name marker) and never consults the collision-prone per-chat
cache; the connector's own created-name guard (`prefer_connector_created`)
still enforces no-clobber, so no initial name is needed gateway-side. The
send-result cache path stays as a fallback for older connectors that don't
stamp the field.
Tests: two new cases in test_relay_threads.py — prospective id wins over a
poisoned cache entry, and two sibling threads in one channel each rename to
their own thread id. Full gateway session + relay suites green (211 passed).
kshitij
HexLab98
brooklyn!
xxxigm
kshitijk4poor
zabih-sudo
Dannyzen
Brin Shadewater
Ben Barclay
314574126
skywind
baau
Kyzcreig
szzhoujiarui
ibaldr89
Teknium
Coffee☕️
ckaznocha
fangliquanflq
Guoen