Skip to content

fix(gateway): race condition, photo media loss, flood control, stuck sessions, and STT config - #4727

Merged
teknium1 merged 6 commits into
mainfrom
hermes/hermes-b19afd39
Apr 3, 2026
Merged

fix(gateway): race condition, photo media loss, flood control, stuck sessions, and STT config#4727
teknium1 merged 6 commits into
mainfrom
hermes/hermes-b19afd39

Conversation

@teknium1

@teknium1 teknium1 commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Salvage of PR #4577 by @kshitijk4poor — cherry-picked onto current main with nit fixes.

Fixes a cluster of Telegram gateway reliability issues:

  1. Race condition in base.py_active_sessions guard set synchronously before create_task() to prevent duplicate background tasks from rapid messages
  2. Photo media loss_build_media_placeholder() creates text context for captionless photos/documents so they aren't silently dropped when dequeued
  3. Progress message flood control — Throttles progress edits to 1.5s minimum interval; degrades gracefully on Telegram flood control instead of blocking 23s+
  4. Streaming truncation — Resets _already_sent when a mid-stream edit fails so the final send path delivers the complete response
  5. Stuck sessionsasyncio.wait_for() timeout on agent execution (default 10min via HERMES_AGENT_TIMEOUT); staleness eviction auto-cleans leaked _running_agents entries
  6. Log noise — Downgrades "Handler returned empty/None response" from WARNING to DEBUG (normal for streaming and queued messages)
  7. STT config resolution_resolve_openai_audio_client_config() checks stt.openai.api_key from config.yaml before falling back to env vars
  8. Cron timeout — Wraps run_conversation in a ThreadPoolExecutor with timeout (HERMES_CRON_TIMEOUT, default 10min) so hung cron jobs don't block the ticker

Follow-up nit fixes

  • Moved _running_agents_ts class attribute below the class docstring to preserve GatewayRunner.__doc__
  • Added clarifying comment on progress throttle continue behavior

New environment variables

Variable Default Purpose
HERMES_AGENT_TIMEOUT 600 (10min) Max agent execution time per message
HERMES_CRON_TIMEOUT 600 (10min) Max cron job execution time

Test results

pytest tests/gateway/ tests/cron/ tests/tools/test_transcription.py: 1929 passed, 3 failed (pre-existing Signal phone redaction test-order issue), 42 skipped

Closes #4577. Supersedes #4650, #4653.
Credit: @kshitijk4poor

kshitijk4poor and others added 6 commits April 3, 2026 00:37
…Telegram

Three bugs causing intermittent silent drops, partial responses, and
flood control delays on the Telegram platform:

1. Race condition in handle_message() — _active_sessions was set inside
   the background task, not before create_task(). Two rapid messages
   could both pass the guard and spawn duplicate processing tasks.
   Fix: set _active_sessions synchronously before spawning the task
   (grammY sequentialize / aiogram EventIsolation pattern).

2. Photo media loss on dequeue — when a photo (no caption) was queued
   during active processing and later dequeued, only .text was
   extracted. Empty text → message silently dropped.
   Fix: _build_media_placeholder() creates text context for media-only
   events so they survive the dequeue path.

3. Progress message edits triggered Telegram flood control — rapid tool
   calls edited the progress message every 0.3s, hitting Telegram's
   rate limit (23s+ waits). This blocked progress updates and could
   cause stream consumer timeouts.
   Fix: throttle edits to 1.5s minimum interval, detect flood control
   errors and gracefully degrade to new messages. edit_message() now
   returns failure for flood waits >5s instead of blocking.
This warning fires on every successful streamed response (streaming
delivers the text, handler returns None via already_sent=True) and
on every queued message during active processing. Both are expected
behavior, not error conditions. Downgrade to DEBUG to reduce log noise.
… eviction

Three changes to prevent sessions from getting permanently locked:

1. Agent execution timeout (HERMES_AGENT_TIMEOUT, default 10min):
   Wraps run_in_executor with asyncio.wait_for so a hung API call or
   runaway tool can't lock a session indefinitely. On timeout, the
   agent is interrupted and the user gets an actionable error message.

2. Staleness eviction for _running_agents:
   Tracks start timestamps for each session entry. When a new message
   arrives and the entry is older than timeout + 1min grace, it's
   evicted as a leaked lock. Safety net for any cleanup path that
   fails to remove the entry.

3. Cron job timeout (HERMES_CRON_TIMEOUT, default 10min):
   Wraps run_conversation in a ThreadPoolExecutor with timeout so a
   hung cron job doesn't block the ticker thread (and all subsequent
   cron jobs) indefinitely.

Follows grammY runner's per-update timeout pattern and aiogram's
asyncio.wait_for approach for handler deadlines.
…llback

Three targeted fixes from user-reported issues:

1. STT config resolution (transcription_tools.py):
   _has_openai_audio_backend() and _resolve_openai_audio_client_config()
   now check stt.openai.api_key/base_url in config.yaml FIRST, before
   falling back to env vars. Fixes voice transcription breaking when
   using a custom OpenAI-compatible endpoint via config.yaml.

2. Stream consumer flood control fallback (stream_consumer.py):
   When an edit fails mid-stream (e.g., Telegram flood control returns
   failure for waits >5s), reset _already_sent to False so the normal
   final send path delivers the complete response. Previously, a
   truncated partial was left as the final message.

3. Telegram edit_message comment alignment (telegram.py):
   Clarify that long flood waits return failure so streaming can fall
   back to a normal final send.
- Fix cron ThreadPoolExecutor blocking on timeout: use shutdown(wait=False,
  cancel_futures=True) instead of context manager that waits indefinitely
- Extract _dequeue_pending_text() to deduplicate media-placeholder logic
  in interrupt and normal-completion dequeue paths
- Remove hasattr guards for _running_agents_ts: add class-level default
  so partial test construction works without scattered defensive checks
- Move `import concurrent.futures` to top of cron/scheduler.py
- Progress throttle: sleep remaining interval instead of busy-looping
  0.1s (~15 wakeups per 1.5s window → 1 wakeup)
- Deduplicate _load_stt_config() in transcription_tools.py:
  _has_openai_audio_backend() now delegates to _resolve_openai_audio_client_config()
…ment

Follow-up nits for salvaged PR #4577:
- Move _running_agents_ts class attribute below the docstring so
  GatewayRunner.__doc__ is preserved.
- Add clarifying comment explaining the throttle continue behavior
  (batches queued messages during the throttle interval).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants