Skip to content

Commit 0011d2c

Browse files
fix(agentex): end SSE subscriptions on terminal task / vanished topic; stop deleting shared stream (#385)
## What was broken When you open a task in the UI, the backend keeps a live stream open to push updates. That stream had **no way to end itself** — it only stopped if the browser disconnected. So when a task finished, the backend just kept listening forever, holding one Redis connection each time. These pile up until the pool is empty, at which point the pod can't serve requests (and its health check fails, so it sits stuck until a manual restart). A second bug: when one viewer left, the code deleted the task's **shared** event stream — yanking it out from under everyone else watching the same task. ## The fix - **End the stream when the task is done.** The backend already gets a `task_updated` event when a task finishes, so the stream now closes as soon as it sees a terminal status. - **Fallback for the rare cases** the event can't cover (viewer connects after the task already finished, or the producer dies silently): a lightweight check on the existing keepalive interval closes the stream then too. - **Stop deleting the shared stream** on one viewer's exit — the Redis TTL already cleans it up on its own. ## Testing Two integration regression tests (stream self-ends on terminal task; one viewer disconnecting doesn't delete the shared stream). **Verified live (A/B against `main`)** — same steps both times: create a RUNNING task → open `GET /tasks/{id}/stream` → complete the task, while watching Redis `blocked_clients`. | | on task completion | Redis connection | |---|---|---| | **`main`** | stream keeps `XREAD`-looping; kept sending `:ping` 15s+ after completion and only ended when the client was force-disconnected | `blocked_clients` stayed `1` — released only on client disconnect (**zombie**) | | **this branch** | stream delivers the terminal `task_updated` and returns immediately (`Ending SSE stream …: received a terminal task_updated event`) | `blocked_clients` `1 → 0` instantly, no client disconnect needed | On `main` the reader logged `Reading messages from Redis stream …` every ~2s indefinitely after the task was done; on this branch it stops the moment the terminal event arrives. Same reproduction the incident described, and the connection is released instead of pinned. > Note: the unit/integration suite runs in CI; the tutorial-agent integration jobs were red due to a pre-existing missing-`pytest-asyncio` issue unrelated to this change (fixed separately in scale-agentex-python). Related: the UI-side half of this leak is #383. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR updates task SSE subscription lifecycle handling. - Ends subscriptions when a terminal update is received, authoritative task state becomes terminal, or the task disappears. - Replays buffered events and closes when connecting to an already-terminal task. - Publishes failure updates and centralizes terminal/non-terminal status classification. - Preserves shared Redis streams when individual subscribers disconnect. - Adds integration coverage for terminal shutdown, late connections, reclaimed keys, transient lookups, and shared-stream retention. <details><summary><h3>Confidence Score: 4/5</h3></summary> The PR should not merge until recoverable FAILED tasks no longer terminate subscriptions that must observe their subsequent RUNNING updates. The stream returns upon receiving any FAILED task update, while the task service intentionally supports forwarding that same FAILED task again and publishing a RUNNING recovery update; viewers whose subscriptions closed on FAILED cannot receive that recovery or subsequent events. **Files Needing Attention:** agentex/src/domain/entities/tasks.py, agentex/src/domain/use_cases/streams_use_case.py, agentex/src/domain/services/task_service.py </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | agentex/src/domain/entities/tasks.py | Introduces canonical terminal and non-terminal status sets shared by state transitions and SSE termination. | | agentex/src/domain/services/task_service.py | Publishes FAILED updates through the normal update path, while existing retry behavior can still restore FAILED tasks to RUNNING. | | agentex/src/domain/use_cases/streams_use_case.py | Adds event-driven and authoritative fallback termination and stops deleting the shared Redis topic, but the previously reported recoverable-FAILED lifecycle conflict remains. | | agentex/src/domain/use_cases/tasks_use_case.py | Reuses the canonical non-terminal status set for transitions to terminal states. | | agentex/tests/integration/test_task_stream.py | Adds regression coverage for terminal shutdown, late subscribers, vanished tasks, transient lookups, reclaimed keys, and shared-stream preservation. | </details> <details><summary><h3>Sequence Diagram</h3></summary> ```mermaid sequenceDiagram participant Client participant SSE as StreamsUseCase participant DB as Task Store participant Redis Client->>SSE: Subscribe to task stream SSE->>Redis: Snapshot stream cursor SSE->>DB: Read authoritative task state alt Task already terminal SSE->>Redis: Replay buffered events SSE-->>Client: Events, then close else Task is non-terminal loop Until terminal, missing, or disconnect SSE->>Redis: Read new events alt Terminal task_updated received SSE-->>Client: Terminal event SSE-->>Client: Close stream else Status-check interval elapsed SSE->>DB: Recheck task alt Terminal or missing SSE-->>Client: Close stream end end end end Note over SSE,Redis: Subscriber exit does not delete the shared stream ``` </details> <sub>Reviews (14): Last reviewed commit: ["fix(agentex): terminate SSE task streams..."](85dfe6b) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=48824816)</sub> <!-- /greptile_comment --> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 80763f7 commit 0011d2c

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)