Skip to content

fix(gateway): force reconnect stale Signal SSE streams#71098

Open
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/signal-sse-stale-force-reconnect
Open

fix(gateway): force reconnect stale Signal SSE streams#71098
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/signal-sse-stale-force-reconnect

Conversation

@fangliquanflq

Copy link
Copy Markdown

What does this PR do?

Fixes a Signal gateway bug where inbound messages could stop permanently while the adapter still looked connected. When the SSE receive stream stalled for more than 120 seconds, the health monitor refreshed the idle clock if the daemon HTTP check returned 200 instead of forcing a reconnect.

Bug Cause

In gateway/platforms/signal.py, _health_monitor() treated a 200 from GET /api/v1/check as proof the receive path was healthy and reset _last_sse_activity. After #3316, signal-cli sends SSE : keepalive comments about every 15 seconds, so crossing HEALTH_CHECK_STALE_THRESHOLD (120s) means the stream itself is hung, not merely quiet. Resetting the idle clock left the dead SSE connection open and _sse_listener() never entered its reconnect/backoff loop.

Reproduction Steps

  1. Start the Hermes gateway with Signal enabled and confirm inbound messages work.
  2. Stall the SSE stream to /api/v1/events (for example proxy drops further bytes, half-open TCP, or a stuck daemon event thread) while GET /api/v1/check still returns 200.
  3. Wait more than 120 seconds without SSE keepalives or data events.
  4. Send a Signal message to the bot.

Expected: the health monitor forces an SSE reconnect and inbound messages resume.
Before fix: logs show repeated "daemon healthy, SSE idle" debug lines, _last_sse_activity keeps getting refreshed, and inbound messages never resume until process restart.

Fix

When SSE activity is stale, call _force_reconnect() immediately. The daemon /check probe runs afterward for diagnostics only and no longer refreshes _last_sse_activity. Added TestSignalSseHealthMonitor to lock reconnect-on-stale behavior, call order (reconnect before probe), and the no-clock-refresh invariant.

Related Issue

No issue - discovered via bug-hunter analysis of Signal SSE health monitoring.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/platforms/signal.py - force reconnect on stale SSE before the diagnostic daemon health probe
  • tests/gateway/test_signal.py - add TestSignalSseHealthMonitor coverage for stale/fresh/unhealthy/error paths

How to Test

  1. Manual: reproduce the stalled SSE scenario above and confirm the gateway logs a stale SSE warning, calls _force_reconnect(), and inbound messages resume without restarting the process.
  2. Automated (already run locally, 141 passed):
scripts/run_tests.sh tests/gateway/test_signal.py -q

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run scripts/run_tests.sh on relevant tests and they pass
  • I've added tests for my changes
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation - or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys - or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - or N/A
  • I've considered cross-platform impact - or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior - or N/A

When the SSE receive path stalls for >120s without keepalives or events,
always call _force_reconnect() instead of resetting the idle clock on a
healthy daemon HTTP /check response. The /check probe remains diagnostics only.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #69328 and #53696. #71098 treats stale SSE activity as a hung receive path and reconnects before probing; #69328 treats a healthy HTTP probe as sufficient, while #53696 replaces SSE entirely. Please choose the intended receive-liveness policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants