Skip to content

Commit 85dfe6b

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 85dfe6b

5 files changed

Lines changed: 276 additions & 22 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: 57 additions & 12 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,19 +169,31 @@ 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. Use a
190+
# fresh timestamp — the read above blocks up to timeout_ms, so
191+
# the loop-top current_time would be stale for ping timing.
147192
if message_count == 0:
148-
current_time = asyncio.get_running_loop().time()
149-
if current_time - last_message_time >= ping_interval:
193+
now = asyncio.get_running_loop().time()
194+
if now - last_message_time >= ping_interval:
150195
yield ":ping\n\n"
151-
last_message_time = current_time
196+
last_message_time = now
152197
await asyncio.sleep(0.1)
153198
else:
154199
# Small pause between batches
@@ -190,8 +235,8 @@ async def stream_task_events(
190235
)
191236
yield f"data: {TaskStreamErrorEventEntity(type='error', message=str(e)).model_dump_json()}\n\n"
192237
finally:
238+
# Don't delete the shared topic; the TTL reclaims it.
193239
logger.info(f"SSE stream for task {task_id} has ended")
194-
await self.cleanup_stream(stream_topic)
195240

196241

197242
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,

0 commit comments

Comments
 (0)