Skip to content

Commit 46e0c1e

Browse files
authored
fix(container): emit SSE done event when container exits (#17)
The SSE stream endpoint now yields an `event: done` frame after docker logs end, and marks the session COMPLETED in the DB via mark_completed(). Previously the frontend relied on the SSE connection closing (which no longer happens between turns with the per-turn entrypoint loop).
1 parent e2854c9 commit 46e0c1e

2 files changed

Lines changed: 114 additions & 5 deletions

File tree

apps/api/src/helprs/modules/container/router.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
"""Container session API routes."""
22

3+
import json
34
from uuid import UUID
45

56
from fastapi import APIRouter, Request
67
from fastapi.responses import StreamingResponse
78

9+
from helprs.core.database import get_db_context
810
from helprs.core.dependencies import DbSession, GetSettings
911
from helprs.core.exceptions import NotFoundError
1012
from helprs.core.middleware import limiter
@@ -24,6 +26,7 @@
2426
create_session,
2527
get_session_events,
2628
get_session_or_404,
29+
mark_completed,
2730
send_message,
2831
start_container,
2932
stop_container,
@@ -151,6 +154,19 @@ async def _event_stream():
151154
try:
152155
async for event in stream_and_persist(docker, cs.container_id, session_id=session_id, offset=offset):
153156
yield event
157+
158+
# Stream ended naturally — container exited.
159+
# Mark session completed in DB and send done event to frontend.
160+
msg = "Session completed."
161+
try:
162+
async with get_db_context() as db_ctx:
163+
result = await mark_completed(db_ctx, session_id, docker)
164+
if result.status == ContainerStatus.FAILED:
165+
msg = "Session failed."
166+
except Exception:
167+
pass # Best effort; cleanup task handles stragglers
168+
169+
yield f"event: done\ndata: {json.dumps({'message': msg})}\n\n"
154170
finally:
155171
await docker.close()
156172

apps/api/tests/modules/container/test_router.py

Lines changed: 98 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine
1616

1717
from helprs.core.config import get_settings
18-
from helprs.core.database import Base
18+
from helprs.core.database import Base, clear_session_factory, set_session_factory
1919
from helprs.core.security import create_access_token, fernet_encrypt
2020
from helprs.main import create_app
2121
from helprs.modules.identity.models import GitHubUser
@@ -36,9 +36,11 @@ async def app_with_db():
3636
application = create_app()
3737
session_factory = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False)
3838
application.state.session_factory = session_factory
39+
set_session_factory(session_factory)
3940

4041
yield application
4142

43+
clear_session_factory()
4244
async with engine.begin() as conn:
4345
await conn.run_sync(Base.metadata.drop_all)
4446
await engine.dispose()
@@ -101,8 +103,10 @@ async def seeded_app(app_with_db):
101103
class FakeDockerClientForRouter:
102104
"""Minimal fake that is injected via monkeypatch."""
103105

104-
def __init__(self):
106+
def __init__(self, log_lines: list[str] | None = None, exit_code: int = 0):
105107
self.container_id = "fake-router-container-id"
108+
self._log_lines = log_lines or ['{"type":"system","subtype":"init"}\n', '{"type":"assistant","message":{}}\n']
109+
self._exit_code = exit_code
106110

107111
async def create_container(self, image, environment, volumes, labels):
108112
return self.container_id
@@ -117,11 +121,14 @@ async def remove_container(self, container_id, force=False):
117121
pass
118122

119123
async def container_logs(self, container_id, follow=False) -> AsyncIterator[str]:
120-
yield "log line 1"
121-
yield "log line 2"
124+
for line in self._log_lines:
125+
yield line
126+
127+
async def write_to_container(self, container_id, data):
128+
pass
122129

123130
async def wait_container(self, container_id):
124-
return 0
131+
return self._exit_code
125132

126133
async def close(self):
127134
pass
@@ -241,3 +248,89 @@ async def test_stop_session_not_found(self, seeded_app):
241248
)
242249

243250
assert resp.status_code == 404
251+
252+
253+
class TestStreamDoneEvent:
254+
async def test_stream_emits_done_event_when_container_exits(self, seeded_app):
255+
"""When docker logs end (container exit), the SSE stream must emit an event: done."""
256+
app = seeded_app["app"]
257+
token = seeded_app["access_token"]
258+
installation_id = seeded_app["installation_id"]
259+
260+
# Create a RUNNING session in the DB via the service layer
261+
session_factory = app.state.session_factory
262+
async with session_factory() as session:
263+
from helprs.modules.container.models import ContainerSession, ContainerStatus
264+
265+
cs = ContainerSession(
266+
installation_id=installation_id,
267+
pr_number=1,
268+
repo_full_name="org/repo",
269+
skill_name="challenge-me",
270+
status=ContainerStatus.RUNNING,
271+
container_id="fake-router-container-id",
272+
)
273+
session.add(cs)
274+
await session.commit()
275+
session_id = cs.id
276+
277+
fake_docker = FakeDockerClientForRouter()
278+
279+
with patch(
280+
"helprs.modules.container.router._get_docker_client",
281+
return_value=fake_docker,
282+
):
283+
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client:
284+
resp = await client.get(
285+
f"/api/v1/containers/sessions/{session_id}/stream",
286+
headers={"Authorization": f"Bearer {token}"},
287+
)
288+
289+
assert resp.status_code == 200
290+
body = resp.text
291+
# The last SSE frame must be an event: done
292+
assert "event: done" in body
293+
assert '"message"' in body
294+
295+
async def test_stream_marks_session_completed_in_db(self, seeded_app):
296+
"""After the stream ends, the session status should be COMPLETED in the DB."""
297+
app = seeded_app["app"]
298+
token = seeded_app["access_token"]
299+
installation_id = seeded_app["installation_id"]
300+
301+
session_factory = app.state.session_factory
302+
async with session_factory() as session:
303+
from helprs.modules.container.models import ContainerSession, ContainerStatus
304+
305+
cs = ContainerSession(
306+
installation_id=installation_id,
307+
pr_number=2,
308+
repo_full_name="org/repo",
309+
skill_name="challenge-me",
310+
status=ContainerStatus.RUNNING,
311+
container_id="fake-router-container-id",
312+
)
313+
session.add(cs)
314+
await session.commit()
315+
session_id = cs.id
316+
317+
fake_docker = FakeDockerClientForRouter()
318+
319+
with patch(
320+
"helprs.modules.container.router._get_docker_client",
321+
return_value=fake_docker,
322+
):
323+
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client:
324+
await client.get(
325+
f"/api/v1/containers/sessions/{session_id}/stream",
326+
headers={"Authorization": f"Bearer {token}"},
327+
)
328+
329+
# Verify session is COMPLETED in DB
330+
async with session_factory() as session:
331+
from sqlalchemy import select
332+
333+
result = await session.execute(select(ContainerSession).where(ContainerSession.id == session_id))
334+
updated = result.scalar_one()
335+
assert updated.status == ContainerStatus.COMPLETED
336+
assert updated.completed_at is not None

0 commit comments

Comments
 (0)