Skip to content

Commit 438d828

Browse files
refactor(agentex): terminate SSE via connect-time check + terminal event, drop the poll
Collapse stream termination into two deterministic checks and remove the recurring keepalive fallback: - Connect-time: if the task is already terminal, replay buffered events and end. Handles late connects deterministically (the fallback's main job). - In-stream: end on a terminal task_updated event (unchanged). Every real terminal transition funnels through transition_status, which emits a task_updated, so the event path covers finishes while streaming and the connect-time check covers finishes before subscribing. The idle branch is now just the keepalive ping. Removes _stream_is_finished, the StreamRepository.stream_exists port method and its Redis impl, and the stream_ever_existed bookkeeping. This also eliminates the 'expired key closes a live stream' edge (there is no vanished-key check anymore): a still-running task's stream simply stays open until the client disconnects, which is correct. Tests updated: stream_exists assertions use redis.exists directly; the reclaimed-key test now asserts a running stream is never closed by key reclamation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 43bb3c1 commit 438d828

4 files changed

Lines changed: 35 additions & 92 deletions

File tree

agentex/src/adapters/streams/adapter_redis.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -229,14 +229,6 @@ 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-
240232
async def cleanup_stream(self, topic: str) -> None:
241233
"""
242234
Clean up a Redis stream.

agentex/src/adapters/streams/port.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,6 @@ 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-
7163
@abstractmethod
7264
async def cleanup_stream(self, topic: str) -> None:
7365
"""

agentex/src/domain/use_cases/streams_use_case.py

Lines changed: 22 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,29 @@ async def stream_task_events(
115115
task_id = task.id
116116

117117
stream_topic = get_task_event_stream_topic(task_id=task_id)
118-
# Snapshot the read cursor BEFORE yielding "connected". "connected" is
119-
# the client's cue to send its message, which makes the agent start
120-
# XADD-ing deltas. Snapshotting after the yield lets a congested relay
121-
# fall behind far enough that those deltas land before the snapshot and
122-
# are never read. Snapshotting first resolves to "0-0" (stream is empty
123-
# until the client sends), so we read from the beginning.
118+
# Authoritative connect-time check. If the task is ALREADY terminal, any
119+
# terminal event sits behind the live tail and the read loop below would
120+
# never surface it — so replay whatever is buffered and end here. This
121+
# handles the late-connect case deterministically (no polling needed);
122+
# the in-loop check below handles a task that finishes while streaming.
123+
task = await self.task_service.get_task(id=task_id)
124+
# Snapshot the read cursor BEFORE yielding "connected". "connected" is the
125+
# client's cue to send its message, which makes the agent start XADD-ing
126+
# deltas; snapshotting after the yield could let those first deltas advance
127+
# the tail and be skipped. Resolves to "0-0" (from the beginning) when the
128+
# stream is empty. Only used by the live path below.
124129
last_id = await self.stream_repository.get_stream_tail_id(stream_topic)
125130
# Send initial connection data
126131
yield f"data: {TaskStreamConnectedEventEntity(type='connected', taskId=task_id).model_dump_json()}\n\n"
132+
if task.status in _TERMINAL_TASK_STATUSES:
133+
async for _id, data in self.read_messages(topic=stream_topic, last_id="0"):
134+
yield f"data: {data.model_dump_json()}\n\n"
135+
await asyncio.sleep(0.02)
136+
logger.info(
137+
f"Ending SSE stream for task {task_id}: already terminal at connect"
138+
)
139+
return
140+
127141
last_message_time = asyncio.get_running_loop().time()
128142
ping_interval = float(
129143
self.environment_variables.SSE_KEEPALIVE_PING_INTERVAL
@@ -133,12 +147,6 @@ async def stream_task_events(
133147
# client's read fails on each cycle; without backoff this turns into a
134148
# log-ingestion firehose (one failure per client per cycle, ~once/sec).
135149
consecutive_errors = 0
136-
# End the sub once no more events can arrive, else XREAD blocks forever,
137-
# pinning a Redis connection ("zombie"). Ends via a terminal task_updated
138-
# (primary) or the keepalive fallback. A non-"0-0" tail means data existed
139-
# at connect; stream_ever_existed gates the fallback so a not-yet-created
140-
# topic isn't mistaken for a reclaimed one.
141-
stream_ever_existed = last_id != "0-0"
142150
try:
143151
# Application-level control loop
144152
while True:
@@ -152,9 +160,6 @@ async def stream_task_events(
152160
# Update the last_id for the next iteration
153161
last_id = new_id
154162
message_count += 1
155-
# Topic exists; a later disappearance now means "gone",
156-
# not "not yet created".
157-
stream_ever_existed = True
158163
# Send the data to the client
159164
data_str = f"data: {data.model_dump_json()}\n\n"
160165
yield data_str
@@ -177,21 +182,11 @@ async def stream_task_events(
177182
# healthy again, so reset the backoff/error counter.
178183
consecutive_errors = 0
179184

180-
# No messages this cycle: on each keepalive interval, run the
181-
# fallback done-check (for finishes the event loop never
182-
# sees: late connect, silent producer death, a payload-less
183-
# task_updated) and, if still live, send a keepalive ping.
185+
# No messages this cycle: send a keepalive ping on the
186+
# configured interval so proxies don't reap an idle stream.
184187
if message_count == 0:
185188
current_time = asyncio.get_running_loop().time()
186189
if current_time - last_message_time >= ping_interval:
187-
if await self._stream_is_finished(
188-
task_id, stream_topic, stream_ever_existed
189-
):
190-
logger.info(
191-
f"Ending SSE stream for task {task_id}: "
192-
"fallback check found it finished"
193-
)
194-
break
195190
yield ":ping\n\n"
196191
last_message_time = current_time
197192
await asyncio.sleep(0.1)
@@ -240,41 +235,5 @@ async def stream_task_events(
240235
# others. The sliding TTL reclaims it instead.
241236
logger.info(f"SSE stream for task {task_id} has ended")
242237

243-
async def _stream_is_finished(
244-
self,
245-
task_id: str,
246-
stream_topic: str,
247-
stream_ever_existed: bool,
248-
) -> bool:
249-
"""
250-
Fallback terminal check. True if the task is terminal, or the task's
251-
status could not be confirmed AND its topic was reclaimed after having
252-
existed (covers a hard-deleted task or a failing lookup). A confirmed
253-
non-terminal task is treated as live even if its idle key was
254-
reclaimed, and a not-yet-written topic is not treated as finished.
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"treating status as unknown: {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, but only when the status is unknown (lookup failed /
268-
# hard delete). A confirmed non-terminal task is still live even if its
269-
# idle key was reclaimed by the sliding TTL — a later XADD recreates the
270-
# key, so we must not close its stream on the expired key alone.
271-
if (
272-
task is None
273-
and stream_ever_existed
274-
and not await self.stream_repository.stream_exists(stream_topic)
275-
):
276-
return True
277-
return False
278-
279238

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

agentex/tests/integration/test_task_stream.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ async def test_cleanup_does_not_delete_shared_stream_on_disconnect(
716716

717717
# Ensure the topic exists.
718718
await repo.send_data(stream_topic, {"type": "error", "message": "seed"})
719-
assert await repo.stream_exists(stream_topic), "precondition: topic exists"
719+
assert bool(await repo.redis.exists(stream_topic)), "precondition: topic exists"
720720

721721
# Run one subscriber briefly, then disconnect it (simulating one viewer
722722
# of a task that other viewers are still watching).
@@ -738,7 +738,7 @@ async def brief_reader():
738738
pass
739739

740740
# The shared topic must survive one subscriber leaving.
741-
assert await repo.stream_exists(stream_topic), (
741+
assert bool(await repo.redis.exists(stream_topic)), (
742742
"Topic was deleted when a single subscriber disconnected — other "
743743
"live viewers of this task would lose their stream."
744744
)
@@ -796,26 +796,24 @@ async def test_running_task_stream_survives_reclaimed_key(
796796
self, test_agent_and_task, streams_use_case
797797
):
798798
"""
799-
Regression for the review finding "expired key ends live stream": a
800-
still-RUNNING task whose idle stream key is reclaimed by the sliding TTL
801-
must NOT be closed. A later XADD recreates the key, so the fallback must
802-
only treat a vanished topic as terminal when the task status is unknown,
803-
never when the task is confirmed non-terminal.
799+
A still-RUNNING task whose idle stream key is reclaimed by the sliding
800+
TTL must NOT have its stream closed — a later XADD recreates the key.
801+
Termination is driven by the terminal task_updated event (and the
802+
connect-time terminal check), never by a vanished key, so a live task's
803+
stream stays open regardless of key reclamation.
804804
"""
805805
from src.utils.stream_topics import get_task_event_stream_topic
806806

807-
# Fast fallback cadence so the vanished-key check gets a chance to (wrongly)
808-
# fire within the test window if the regression is present.
807+
# Short keepalive so several idle cycles elapse within the test window.
809808
streams_use_case.environment_variables.SSE_KEEPALIVE_PING_INTERVAL = 1
810809

811810
_agent, task = test_agent_and_task # created in RUNNING state
812811
stream_topic = get_task_event_stream_topic(task_id=task.id)
813812
repo = streams_use_case.stream_repository
814813

815-
# Seed the topic so it has existed (arms the vanished-topic fallback), then
816-
# subscribe.
814+
# Seed the topic, subscribe, then reclaim the key mid-stream.
817815
await repo.send_data(stream_topic, {"type": "error", "message": "seed"})
818-
assert await repo.stream_exists(stream_topic), "precondition: topic exists"
816+
assert bool(await repo.redis.exists(stream_topic)), "precondition: topic exists"
819817

820818
async def reader():
821819
try:
@@ -832,7 +830,9 @@ async def reader():
832830
# reclaiming the key while the task is still RUNNING.
833831
await asyncio.sleep(0.5)
834832
await repo.cleanup_stream(stream_topic)
835-
assert not await repo.stream_exists(stream_topic), "precondition: key reclaimed"
833+
assert not bool(await repo.redis.exists(stream_topic)), (
834+
"precondition: key reclaimed"
835+
)
836836

837837
# Give the fallback several cycles to (wrongly) close the stream.
838838
await asyncio.sleep(4)

0 commit comments

Comments
 (0)