Skip to content

Commit 6716712

Browse files
fix(agentex): terminate SSE task streams deterministically; stop leaking Redis connections
Task event SSE subscriptions had no terminal condition: stream_task_events ran a `while True` loop that only exited on client disconnect or a fatal error. Once a task finished, its producer stopped writing but the reader kept blocking on the Redis stream (XREAD) forever, pinning a connection from the shared per-process pool. Because stream keys carry a sliding TTL, a finished task's key eventually expires while readers keep blocking on it — a permanent zombie. Accumulated zombies exhaust the pool, which is shared with the readiness probe, so /readyz fails while the dependency-free /healthz stays 200: the pod goes Unready and is never restarted. Termination now has three independent, deterministic checks: - Connect-time: read task status once at connect (after snapshotting the cursor, so a racing terminal event still lands after the cursor). If already terminal, replay buffered events and end — handles late connects. - In-stream: end on a terminal task_updated event (the last event a task emits). - Periodic authoritative recheck: at the top of every loop iteration, on an interval, re-read task status and end if terminal. Runs busy or idle and even after a read failure/backoff, so a dropped/lost terminal event or a failing read cannot keep the stream open. A hard-deleted task (ItemDoesNotExist) is treated as terminal; other lookup errors fall through to transient retry. Also: - AgentTaskService.fail_task now publishes task_updated via update_task — it was the only terminal write that didn't emit, which could strand a live viewer. - Drop the per-subscriber cleanup_stream in `finally`: the topic is shared by all viewers, so deleting it on one exit broke the others. The sliding TTL reclaims. - Canonical NON_TERMINAL/TERMINAL task-status sets on the TaskStatus entity, referenced by both the status state machine and SSE termination. Adds integration regression tests: event-driven termination, late-connect termination, a running task surviving a reclaimed key, and shared-stream survival on disconnect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 80763f7 commit 6716712

5 files changed

Lines changed: 271 additions & 20 deletions

File tree

agentex/src/domain/entities/tasks.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ class TaskStatus(str, Enum):
2929
DELETED = "DELETED"
3030

3131

32+
# Canonical status partition (state machine + SSE termination).
33+
# Non-terminal: RUNNING or INTERRUPTED (resumable); terminal is the rest.
34+
# New statuses are terminal unless added to the non-terminal set.
35+
NON_TERMINAL_TASK_STATUSES = frozenset({TaskStatus.RUNNING, TaskStatus.INTERRUPTED})
36+
TERMINAL_TASK_STATUSES = frozenset(TaskStatus) - NON_TERMINAL_TASK_STATUSES
37+
38+
3239
class TaskEntity(BaseModel):
3340
id: str = Field(
3441
...,

agentex/src/domain/services/task_service.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@ async def forward_task_to_acp(
166166
async def fail_task(self, task: TaskEntity, reason: str) -> None:
167167
task.status = TaskStatus.FAILED
168168
task.status_reason = reason
169-
await self.task_repository.update(task)
169+
# Publish task_updated so streaming viewers see the failure and end.
170+
# Every terminal write must emit; SSE termination relies on it.
171+
await self.update_task(task)
170172

171173
async def get_task(
172174
self,
@@ -374,7 +376,11 @@ async def cancel_task(
374376
new_status=TaskStatus.CANCELED,
375377
status_reason="Task canceled by user",
376378
)
377-
return updated if updated is not None else await self.task_repository.get(id=task.id)
379+
return (
380+
updated
381+
if updated is not None
382+
else await self.task_repository.get(id=task.id)
383+
)
378384

379385
async def interrupt_task(
380386
self, agent: AgentEntity, task: TaskEntity, acp_url: str

agentex/src/domain/use_cases/streams_use_case.py

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@
55
from fastapi import Depends
66
from pydantic import ValidationError
77

8+
from src.adapters.crud_store.exceptions import ItemDoesNotExist
89
from src.adapters.streams.adapter_redis import DRedisStreamRepository
910
from src.api.schemas.task_stream_events import TaskStreamEvent
1011
from src.config.dependencies import DEnvironmentVariables
1112
from src.domain.entities.task_stream_events import (
1213
TaskStreamConnectedEventEntity,
1314
TaskStreamErrorEventEntity,
1415
TaskStreamEventEntity,
16+
TaskStreamTaskUpdatedEventEntity,
1517
convert_task_stream_event_to_entity,
1618
)
19+
from src.domain.entities.tasks import TERMINAL_TASK_STATUSES
1720
from src.domain.services.task_service import DAgentTaskService
1821
from src.utils.logging import make_logger
1922
from src.utils.stream_topics import get_task_event_stream_topic
@@ -101,15 +104,21 @@ async def stream_task_events(
101104
task_id = task.id
102105

103106
stream_topic = get_task_event_stream_topic(task_id=task_id)
104-
# Snapshot the read cursor BEFORE yielding "connected". "connected" is
105-
# the client's cue to send its message, which makes the agent start
106-
# XADD-ing deltas. Snapshotting after the yield lets a congested relay
107-
# fall behind far enough that those deltas land before the snapshot and
108-
# are never read. Snapshotting first resolves to "0-0" (stream is empty
109-
# until the client sends), so we read from the beginning.
107+
# Cursor before status read: catches a racing terminal event.
110108
last_id = await self.stream_repository.get_stream_tail_id(stream_topic)
109+
task = await self.task_service.get_task(id=task_id)
111110
# Send initial connection data
112111
yield f"data: {TaskStreamConnectedEventEntity(type='connected', taskId=task_id).model_dump_json()}\n\n"
112+
# Already terminal: replay buffered events and end (late connect).
113+
if task.status in TERMINAL_TASK_STATUSES:
114+
async for _id, data in self.read_messages(topic=stream_topic, last_id="0"):
115+
yield f"data: {data.model_dump_json()}\n\n"
116+
await asyncio.sleep(0.02)
117+
logger.info(
118+
f"Ending SSE stream for task {task_id}: already terminal at connect"
119+
)
120+
return
121+
113122
last_message_time = asyncio.get_running_loop().time()
114123
ping_interval = float(
115124
self.environment_variables.SSE_KEEPALIVE_PING_INTERVAL
@@ -119,10 +128,34 @@ async def stream_task_events(
119128
# client's read fails on each cycle; without backoff this turns into a
120129
# log-ingestion firehose (one failure per client per cycle, ~once/sec).
121130
consecutive_errors = 0
131+
last_status_check = last_message_time
122132
try:
123133
# Application-level control loop
124134
while True:
125135
try:
136+
# Authoritative status recheck on an interval. Runs at the
137+
# TOP of every iteration — even after a read failure/backoff —
138+
# so a terminal task ends even if its event publish was lost
139+
# or Redis reads keep erroring.
140+
current_time = asyncio.get_running_loop().time()
141+
if current_time - last_status_check >= ping_interval:
142+
last_status_check = current_time
143+
try:
144+
task = await self.task_service.get_task(id=task_id)
145+
except ItemDoesNotExist:
146+
# Row permanently gone (e.g. retention) — end, don't retry.
147+
logger.info(
148+
f"Ending SSE stream for task {task_id}: "
149+
"task no longer exists"
150+
)
151+
return
152+
if task.status in TERMINAL_TASK_STATUSES:
153+
logger.info(
154+
f"Ending SSE stream for task {task_id}: "
155+
"terminal on status recheck"
156+
)
157+
return
158+
126159
# Process yielded messages one by one
127160
message_generator = self.read_messages(
128161
topic=stream_topic, last_id=last_id
@@ -136,16 +169,25 @@ async def stream_task_events(
136169
data_str = f"data: {data.model_dump_json()}\n\n"
137170
yield data_str
138171
last_message_time = asyncio.get_running_loop().time()
172+
# Terminal event is the last one — end here.
173+
if (
174+
isinstance(data, TaskStreamTaskUpdatedEventEntity)
175+
and data.task is not None
176+
and data.task.status in TERMINAL_TASK_STATUSES
177+
):
178+
logger.info(
179+
f"Ending SSE stream for task {task_id}: received "
180+
"a terminal task_updated event"
181+
)
182+
return
139183
await asyncio.sleep(0.02)
140184

141185
# A read cycle completed without raising — the stream is
142186
# healthy again, so reset the backoff/error counter.
143187
consecutive_errors = 0
144188

145-
# If we didn't get any messages, add a small pause
146-
# to prevent tight loops and send keepalive ping if needed
189+
# Idle: send keepalive ping so proxies don't reap us.
147190
if message_count == 0:
148-
current_time = asyncio.get_running_loop().time()
149191
if current_time - last_message_time >= ping_interval:
150192
yield ":ping\n\n"
151193
last_message_time = current_time
@@ -190,8 +232,8 @@ async def stream_task_events(
190232
)
191233
yield f"data: {TaskStreamErrorEventEntity(type='error', message=str(e)).model_dump_json()}\n\n"
192234
finally:
235+
# Don't delete the shared topic; the TTL reclaims it.
193236
logger.info(f"SSE stream for task {task_id} has ended")
194-
await self.cleanup_stream(stream_topic)
195237

196238

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

agentex/src/domain/use_cases/tasks_use_case.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@
33
from fastapi import Depends
44

55
from src.adapters.crud_store.exceptions import ItemDoesNotExist
6-
from src.domain.entities.tasks import TaskEntity, TaskRelationships, TaskStatus
6+
from src.domain.entities.tasks import (
7+
NON_TERMINAL_TASK_STATUSES,
8+
TaskEntity,
9+
TaskRelationships,
10+
TaskStatus,
11+
)
712
from src.domain.exceptions import ClientError
813
from src.domain.services.task_service import DAgentTaskService
914
from src.utils.logging import make_logger
@@ -132,10 +137,8 @@ async def update_mutable_fields_on_task(
132137

133138
return task_entity
134139

135-
# Non-terminal statuses a task can be transitioned to a terminal status from.
136-
# RUNNING is the normal case; INTERRUPTED is also valid so an interrupted
137-
# (paused, still-continuable) task can still be canceled/completed/etc later.
138-
_TERMINAL_TRANSITION_SOURCES = (TaskStatus.RUNNING, TaskStatus.INTERRUPTED)
140+
# Statuses a task can transition to terminal from (the non-terminal set).
141+
_TERMINAL_TRANSITION_SOURCES = NON_TERMINAL_TASK_STATUSES
139142

140143
async def _transition_to_terminal(
141144
self,

agentex/tests/integration/test_task_stream.py

Lines changed: 196 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,198 @@ 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 bool(await repo.redis.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 bool(await repo.redis.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")
747+
748+
async def test_stream_ends_on_late_connect_to_terminal_task(
749+
self, test_agent_and_task, tasks_use_case, streams_use_case
750+
):
751+
"""
752+
Connect-time termination path: a viewer that connects *after* the task
753+
has already reached a terminal state never receives a terminal
754+
task_updated event via the read loop — the event is behind the snapshot
755+
cursor, so the in-loop check can't fire. The authoritative connect-time
756+
check must end the stream (replay buffered events, then return) instead
757+
of blocking on the topic forever.
758+
"""
759+
_agent, task = test_agent_and_task
760+
761+
# Finish the task BEFORE anyone subscribes. The terminal task_updated is
762+
# XADDed to the stream now, so a later subscriber snapshots past it and
763+
# the read loop never surfaces it.
764+
completed = await tasks_use_case.complete_task(id=task.id)
765+
assert completed.status == TaskStatus.COMPLETED
766+
767+
async def drain_until_end():
768+
# Returns normally only if the generator terminates by itself.
769+
async for _event_data in streams_use_case.stream_task_events(
770+
task_id=task.id
771+
):
772+
pass
773+
774+
reader_task = asyncio.create_task(drain_until_end())
775+
776+
# The connect-time check should end it near-immediately.
777+
try:
778+
await asyncio.wait_for(reader_task, timeout=10)
779+
except TimeoutError as err:
780+
reader_task.cancel()
781+
try:
782+
await reader_task
783+
except asyncio.CancelledError:
784+
pass
785+
raise AssertionError(
786+
"SSE stream to an already-terminal task did not self-close — the "
787+
"connect-time terminal check is not firing, so late-connect "
788+
"subscriptions leak as zombies."
789+
) from err
790+
791+
print("✅ Stream self-closes when connecting to an already-terminal task")
792+
793+
async def test_running_task_stream_survives_reclaimed_key(
794+
self, test_agent_and_task, streams_use_case
795+
):
796+
"""
797+
A still-RUNNING task whose idle stream key is reclaimed by the sliding
798+
TTL must NOT have its stream closed — a later XADD recreates the key.
799+
Termination is driven by the terminal task_updated event (and the
800+
connect-time terminal check), never by a vanished key, so a live task's
801+
stream stays open regardless of key reclamation.
802+
"""
803+
from src.utils.stream_topics import get_task_event_stream_topic
804+
805+
# Short keepalive so several idle cycles elapse within the test window.
806+
streams_use_case.environment_variables.SSE_KEEPALIVE_PING_INTERVAL = 1
807+
808+
_agent, task = test_agent_and_task # created in RUNNING state
809+
stream_topic = get_task_event_stream_topic(task_id=task.id)
810+
repo = streams_use_case.stream_repository
811+
812+
# Seed the topic, subscribe, then reclaim the key mid-stream.
813+
await repo.send_data(stream_topic, {"type": "error", "message": "seed"})
814+
assert bool(await repo.redis.exists(stream_topic)), "precondition: topic exists"
815+
816+
async def reader():
817+
try:
818+
async for _event_data in streams_use_case.stream_task_events(
819+
task_id=task.id
820+
):
821+
pass
822+
except asyncio.CancelledError:
823+
pass
824+
825+
reader_task = asyncio.create_task(reader())
826+
827+
# Let the reader connect and go idle, then simulate the sliding TTL
828+
# reclaiming the key while the task is still RUNNING.
829+
await asyncio.sleep(0.5)
830+
await repo.cleanup_stream(stream_topic)
831+
assert not bool(await repo.redis.exists(stream_topic)), (
832+
"precondition: key reclaimed"
833+
)
834+
835+
# Give the fallback several cycles to (wrongly) close the stream.
836+
await asyncio.sleep(4)
837+
838+
try:
839+
assert not reader_task.done(), (
840+
"SSE stream for a still-RUNNING task was closed after its idle key "
841+
"was reclaimed — the fallback must not treat a reclaimed key as "
842+
"terminal when the task is confirmed non-terminal."
843+
)
844+
finally:
845+
reader_task.cancel()
846+
try:
847+
await reader_task
848+
except asyncio.CancelledError:
849+
pass
850+
851+
print("✅ Running task's stream stays open when its idle key is reclaimed")

0 commit comments

Comments
 (0)