Skip to content

Commit 2eed3af

Browse files
fix(agentex): end SSE subscriptions on terminal task; stop deleting shared stream
Task event SSE subscriptions had no terminal condition. stream_task_events ran a `while True` loop whose only exits were client disconnect (CancelledError) and fatal errors. Once a task finished its producer stopped writing, but the reader kept blocking on the topic forever — issuing an XREAD every couple of seconds and pinning one connection from the shared per-process Redis pool. Because the stream key carries a sliding TTL, a finished task's key eventually disappears while readers keep blocking on it, turning each subscription into a permanent zombie. Accumulated zombies exhaust the pool, which is shared with the readiness probe, so /readyz fails while the dependency-free /healthz keeps returning 200 — the pod goes Unready and is never restarted. Termination: - Primary is event-driven. Every terminal transition funnels through task_service.transition_status (and delete via update_task), which XADDs a task_updated event carrying the new status onto the task's own stream. The reader already delivers those events, so it returns as soon as it forwards one whose task is in a terminal status — no DB lookup and no ordering race, since the terminal event is the last message produced. - Fallback runs on the keepalive-ping cadence for the cases the event path cannot see: a client that connects after the task already finished, a producer that dies without emitting a terminal event, or a task_updated with no task payload. It ends the stream when the task is terminal or when a topic that had existed is reclaimed. stream_ever_existed is seeded from the tail snapshot so a mid-flight subscriber correctly arms the vanished-topic check. Shared stream: - Drop the per-subscriber cleanup_stream in finally. The topic is keyed only by task id and shared by every viewer, so deleting it on one subscriber's exit tore the stream out from under the others. The sliding TTL already reclaims it. Also adds stream_exists to the stream port + Redis adapter (EXISTS) for the vanished-topic fallback, and two integration regression tests: the stream ends on its own once the task is terminal, and a single subscriber disconnecting does not delete the shared topic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 32a9acc commit 2eed3af

4 files changed

Lines changed: 190 additions & 6 deletions

File tree

agentex/src/adapters/streams/adapter_redis.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,14 @@ async def read_messages(
229229
logger.error(f"Error reading from Redis stream {topic}: {e}")
230230
raise
231231

232+
async def stream_exists(self, topic: str) -> bool:
233+
"""Whether the Redis stream key exists (EXISTS)."""
234+
try:
235+
return bool(await self.redis.exists(topic))
236+
except Exception as e:
237+
logger.error(f"Error checking existence of Redis stream {topic}: {e}")
238+
raise
239+
232240
async def cleanup_stream(self, topic: str) -> None:
233241
"""
234242
Clean up a Redis stream.

agentex/src/adapters/streams/port.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ async def get_stream_tail_id(self, topic: str) -> str:
6060
"""
6161
raise NotImplementedError
6262

63+
@abstractmethod
64+
async def stream_exists(self, topic: str) -> bool:
65+
"""
66+
Report whether a stream topic currently exists, to tell a reclaimed
67+
topic ("gone") apart from one that is still live.
68+
"""
69+
raise NotImplementedError
70+
6371
@abstractmethod
6472
async def cleanup_stream(self, topic: str) -> None:
6573
"""

agentex/src/domain/use_cases/streams_use_case.py

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,28 @@
1212
TaskStreamConnectedEventEntity,
1313
TaskStreamErrorEventEntity,
1414
TaskStreamEventEntity,
15+
TaskStreamTaskUpdatedEventEntity,
1516
convert_task_stream_event_to_entity,
1617
)
18+
from src.domain.entities.tasks import TaskStatus
1719
from src.domain.services.task_service import DAgentTaskService
1820
from src.utils.logging import make_logger
1921
from src.utils.stream_topics import get_task_event_stream_topic
2022

2123
logger = make_logger(__name__)
2224

25+
# Statuses after which no more events are produced; a subscription must end.
26+
_TERMINAL_TASK_STATUSES = frozenset(
27+
{
28+
TaskStatus.CANCELED,
29+
TaskStatus.COMPLETED,
30+
TaskStatus.FAILED,
31+
TaskStatus.TERMINATED,
32+
TaskStatus.TIMED_OUT,
33+
TaskStatus.DELETED,
34+
}
35+
)
36+
2337

2438
class StreamsUseCase:
2539
def __init__(
@@ -119,6 +133,13 @@ async def stream_task_events(
119133
# client's read fails on each cycle; without backoff this turns into a
120134
# log-ingestion firehose (one failure per client per cycle, ~once/sec).
121135
consecutive_errors = 0
136+
# A subscription must end once no further events can arrive, or it
137+
# blocks on the topic forever, pinning a Redis connection (a "zombie").
138+
# Primary signal is event-driven (a terminal task_updated below); the
139+
# keepalive-cadence check is a fallback. stream_ever_existed gates the
140+
# fallback's vanished-topic check; a concrete tail ID means the stream
141+
# already had data at connect, so it counts as having existed.
142+
stream_ever_existed = last_id != "0-0"
122143
try:
123144
# Application-level control loop
124145
while True:
@@ -132,21 +153,46 @@ async def stream_task_events(
132153
# Update the last_id for the next iteration
133154
last_id = new_id
134155
message_count += 1
156+
# Topic exists; a later disappearance now means "gone",
157+
# not "not yet created".
158+
stream_ever_existed = True
135159
# Send the data to the client
136160
data_str = f"data: {data.model_dump_json()}\n\n"
137161
yield data_str
138162
last_message_time = asyncio.get_running_loop().time()
163+
# A terminal task_updated is always the last event
164+
# produced, so nothing follows it — end the stream here.
165+
if (
166+
isinstance(data, TaskStreamTaskUpdatedEventEntity)
167+
and data.task is not None
168+
and data.task.status in _TERMINAL_TASK_STATUSES
169+
):
170+
logger.info(
171+
f"Ending SSE stream for task {task_id}: received "
172+
"a terminal task_updated event"
173+
)
174+
return
139175
await asyncio.sleep(0.02)
140176

141177
# A read cycle completed without raising — the stream is
142178
# healthy again, so reset the backoff/error counter.
143179
consecutive_errors = 0
144180

145-
# If we didn't get any messages, add a small pause
146-
# to prevent tight loops and send keepalive ping if needed
181+
# No messages this cycle: on each keepalive interval, run the
182+
# fallback done-check (for finishes the event loop never
183+
# sees: late connect, silent producer death, a payload-less
184+
# task_updated) and, if still live, send a keepalive ping.
147185
if message_count == 0:
148186
current_time = asyncio.get_running_loop().time()
149187
if current_time - last_message_time >= ping_interval:
188+
if await self._stream_is_finished(
189+
task_id, stream_topic, stream_ever_existed
190+
):
191+
logger.info(
192+
f"Ending SSE stream for task {task_id}: "
193+
"fallback check found it finished"
194+
)
195+
break
150196
yield ":ping\n\n"
151197
last_message_time = current_time
152198
await asyncio.sleep(0.1)
@@ -190,8 +236,42 @@ async def stream_task_events(
190236
)
191237
yield f"data: {TaskStreamErrorEventEntity(type='error', message=str(e)).model_dump_json()}\n\n"
192238
finally:
239+
# No cleanup_stream here: the topic is shared by all viewers of the
240+
# task, so deleting it on one subscriber's exit would break the
241+
# others. The sliding TTL reclaims it instead.
193242
logger.info(f"SSE stream for task {task_id} has ended")
194-
await self.cleanup_stream(stream_topic)
243+
244+
async def _stream_is_finished(
245+
self,
246+
task_id: str,
247+
stream_topic: str,
248+
stream_ever_existed: bool,
249+
) -> bool:
250+
"""
251+
Fallback terminal check. True if the task is terminal, or the topic
252+
was reclaimed after having existed (covers hard-deleted tasks whose
253+
lookup fails). A not-yet-written topic is not treated as finished. A
254+
transient lookup failure keeps the stream open.
255+
"""
256+
try:
257+
task = await self.task_service.get_task(id=task_id)
258+
except Exception as e:
259+
logger.warning(
260+
f"Terminal-state lookup failed for task {task_id}; "
261+
f"keeping stream open: {e}"
262+
)
263+
task = None
264+
# Normal finish: the DB says the task is done, so no more XADDs.
265+
if task is not None and task.status in _TERMINAL_TASK_STATUSES:
266+
return True
267+
# Vanished topic: the key existed before but its sliding TTL reclaimed
268+
# it, so a reader is now blocking on a key that can never return data
269+
# (also covers a hard-deleted task whose status lookup failed above).
270+
if stream_ever_existed and not await self.stream_repository.stream_exists(
271+
stream_topic
272+
):
273+
return True
274+
return False
195275

196276

197277
DStreamsUseCase = Annotated[StreamsUseCase, Depends(StreamsUseCase)]

agentex/tests/integration/test_task_stream.py

Lines changed: 91 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,9 +588,7 @@ async def test_event_xadded_after_connected_is_delivered(
588588

589589
# Emit a delta while the generator is suspended at the yield.
590590
sentinel = "after-connected-sentinel"
591-
await repo.send_data(
592-
stream_topic, {"type": "error", "message": sentinel}
593-
)
591+
await repo.send_data(stream_topic, {"type": "error", "message": sentinel})
594592

595593
# The delta must be delivered; a silent stream means it was dropped.
596594
received = False
@@ -656,3 +654,93 @@ async def collect_stream_data():
656654
)
657655

658656
print(f"✅ Stream sent {ping_count} keepalive pings during idle period")
657+
658+
async def test_stream_ends_when_task_reaches_terminal_status(
659+
self, test_agent_and_task, tasks_use_case, streams_use_case
660+
):
661+
"""
662+
Regression for the zombie-subscription leak: the stream must end on its
663+
own once the task is terminal (no client-side cancellation), instead of
664+
blocking on the topic forever and pinning a Redis connection.
665+
666+
Completing via TasksUseCase emits a terminal task_updated event, so this
667+
exercises the event-driven termination path.
668+
"""
669+
_agent, task = test_agent_and_task
670+
671+
async def drain_until_end():
672+
# Returns normally only if the generator terminates by itself.
673+
async for _event_data in streams_use_case.stream_task_events(
674+
task_id=task.id
675+
):
676+
pass
677+
678+
reader_task = asyncio.create_task(drain_until_end())
679+
680+
# Let the stream connect and enter its loop, then finish the task.
681+
await asyncio.sleep(0.2)
682+
completed = await tasks_use_case.complete_task(id=task.id)
683+
assert completed.status == TaskStatus.COMPLETED
684+
685+
# The generator should now end by itself.
686+
try:
687+
await asyncio.wait_for(reader_task, timeout=10)
688+
except TimeoutError as err:
689+
reader_task.cancel()
690+
try:
691+
await reader_task
692+
except asyncio.CancelledError:
693+
pass
694+
raise AssertionError(
695+
"SSE stream did not terminate after the task reached a terminal "
696+
"status — the subscription is a zombie and will pin a Redis "
697+
"connection forever."
698+
) from err
699+
700+
print("✅ Stream ends on its own once the task is terminal")
701+
702+
async def test_cleanup_does_not_delete_shared_stream_on_disconnect(
703+
self, test_agent_and_task, streams_use_case
704+
):
705+
"""
706+
Regression for the shared-stream deletion bug: the topic is shared by
707+
every viewer of a task, so one subscriber disconnecting must not delete
708+
it (the old per-subscriber cleanup_stream did). The sliding TTL handles
709+
reclamation instead.
710+
"""
711+
from src.utils.stream_topics import get_task_event_stream_topic
712+
713+
_agent, task = test_agent_and_task
714+
stream_topic = get_task_event_stream_topic(task_id=task.id)
715+
repo = streams_use_case.stream_repository
716+
717+
# Ensure the topic exists.
718+
await repo.send_data(stream_topic, {"type": "error", "message": "seed"})
719+
assert await repo.stream_exists(stream_topic), "precondition: topic exists"
720+
721+
# Run one subscriber briefly, then disconnect it (simulating one viewer
722+
# of a task that other viewers are still watching).
723+
async def brief_reader():
724+
try:
725+
async for _event_data in streams_use_case.stream_task_events(
726+
task_id=task.id
727+
):
728+
pass
729+
except asyncio.CancelledError:
730+
pass
731+
732+
reader_task = asyncio.create_task(brief_reader())
733+
await asyncio.sleep(0.3)
734+
reader_task.cancel()
735+
try:
736+
await reader_task
737+
except asyncio.CancelledError:
738+
pass
739+
740+
# The shared topic must survive one subscriber leaving.
741+
assert await repo.stream_exists(stream_topic), (
742+
"Topic was deleted when a single subscriber disconnected — other "
743+
"live viewers of this task would lose their stream."
744+
)
745+
746+
print("✅ Shared stream survives a single subscriber disconnecting")

0 commit comments

Comments
 (0)