Skip to content

Commit 1d897fc

Browse files
authored
Merge pull request #7 from mariuspruvot/fix/e2e-bugs-batch-2
fix: e2e SSE streaming bugs (batch 2)
2 parents c068811 + 6150185 commit 1d897fc

9 files changed

Lines changed: 222 additions & 97 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ infra/
4747
- **Container orchestration**: `container` module manages ephemeral Docker lifecycle, credential injection, result relay
4848
- **Skills as agents**: each skill is a self-contained folder with workflow definitions, mounted into containers
4949
- **SSE passthrough**: backend relays container output to frontend (no AI response generation in backend)
50+
- **Stream-json protocol**: containers emit NDJSON with 5 event types: `system` (init/retry), `assistant` (one event per content block — thinking/text/tool_use), `user` (tool_result), `result` (session end + metadata), `rate_limit_event`. The `result.result` field duplicates the last assistant text — only display assistant events, use result for status only. No `--include-partial-messages` flag, so no `stream_event` deltas.
5051
- **API prefix**: all routes under `/api/v1`
5152
- **Admin panel**: SQLAdmin at `/admin`, configured in `admin/views.py`
5253

@@ -87,6 +88,7 @@ Required `.env` at repo root (see docker-compose.yml):
8788
## Gotchas
8889

8990
- Always run `make lint` before pushing — ruff + eslint must pass
91+
- **Debug SSE pipeline**: `claude -p "prompt" --output-format stream-json --verbose 2>/dev/null` captures raw stream-json to validate event parsing
9092
- DB migrations: `make migrate` inside Docker, or `cd apps/api && uv run alembic upgrade head` locally
9193
- Test conftest **must** set env vars before importing from `helprs.*`
9294
- **Agent-readiness**: This repo must be fully understandable by a fresh Claude Code instance with no prior context. Keep docs and CLAUDE.md accurate.

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,29 @@ async def get_container_session(
114114

115115

116116
@router.get("/sessions/{session_id}/stream")
117-
@limiter.limit("10/minute")
117+
@limiter.limit("60/minute")
118118
async def stream_container_output(
119119
session_id: UUID,
120120
request: Request,
121121
db: DbSession,
122+
offset: int = 0,
122123
):
123-
"""SSE endpoint streaming container stdout/stderr."""
124+
"""SSE endpoint streaming container stdout/stderr.
125+
126+
Accepts an ``offset`` query parameter: the number of events to skip.
127+
Clients should pass the last received event ``id`` so that reconnects
128+
resume from where they left off instead of replaying the full log.
129+
130+
Also reads the ``Last-Event-ID`` header (sent automatically by
131+
EventSource on native auto-reconnect) as a fallback when the query
132+
parameter is absent or zero.
133+
"""
134+
# EventSource auto-reconnect sends Last-Event-ID header, not query params.
135+
if offset == 0:
136+
last_event_id = request.headers.get("last-event-id", "")
137+
if last_event_id.isdigit():
138+
offset = int(last_event_id)
139+
124140
cs = await get_session_or_404(db, session_id)
125141

126142
if cs.status != ContainerStatus.RUNNING or not cs.container_id:
@@ -130,7 +146,7 @@ async def stream_container_output(
130146

131147
async def _event_stream():
132148
try:
133-
async for event in stream_output(docker, cs.container_id):
149+
async for event in stream_output(docker, cs.container_id, offset=offset):
134150
yield event
135151
finally:
136152
await docker.close()

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

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from __future__ import annotations
1212

1313
import asyncio
14+
import base64
1415
import contextlib
1516
import json
1617
import os
@@ -137,21 +138,24 @@ async def remove_container(self, container_id: str, force: bool = False) -> None
137138

138139
async def container_logs(self, container_id: str, follow: bool = False) -> AsyncIterator[str]:
139140
container = await self._docker.containers.get(container_id)
140-
async for line in container.log(stdout=True, stderr=True, follow=follow):
141+
# Only read stdout — Claude Code writes stream-json to stdout.
142+
# stderr contains verbose/diagnostic output that duplicates events.
143+
async for line in container.log(stdout=True, stderr=False, follow=follow):
141144
yield line
142145

143146
async def write_to_container(self, container_id: str, data: str) -> None:
144147
"""Write a message to the container's FIFO via docker exec.
145148
146149
The entrypoint reads from /tmp/claude-input FIFO, so we write there.
150+
Uses base64 encoding to safely pass arbitrary JSON through the shell
151+
without risking interpretation of special characters ($, `, !, etc.).
147152
"""
148153
container = await self._docker.containers.get(container_id)
154+
encoded = base64.b64encode(data.encode()).decode()
149155
exec_obj = await container.exec(
150-
cmd=["bash", "-c", f"echo {json.dumps(data)} > /tmp/claude-input"],
151-
stdout=False,
152-
stderr=False,
156+
cmd=["sh", "-c", f"echo {encoded} | base64 -d > /tmp/claude-input"],
153157
)
154-
await exec_obj.start()
158+
await exec_obj.start(detach=True)
155159

156160
async def wait_container(self, container_id: str) -> int:
157161
container = await self._docker.containers.get(container_id)
@@ -293,10 +297,49 @@ async def start_container(
293297
async def stream_output(
294298
docker: DockerClient,
295299
container_id: str,
300+
offset: int = 0,
296301
) -> AsyncIterator[str]:
297-
"""Async generator yielding container log lines as SSE events."""
298-
async for line in docker.container_logs(container_id, follow=True):
299-
yield f"data: {line}\n\n"
302+
"""Async generator yielding container log lines as SSE events.
303+
304+
Docker may split long stdout lines (e.g. stream-json tool_result
305+
events with full file contents) across multiple log frames. We
306+
buffer chunks and only emit complete newline-delimited lines so
307+
the frontend always receives valid, parseable JSON per SSE event.
308+
309+
Each event includes an incrementing ``id:`` field so that clients
310+
can resume from the last received event via the ``offset`` query
311+
parameter (number of events to skip).
312+
313+
When the container is quiet (Claude is thinking), the Docker log
314+
stream produces no data. We send SSE comments (``:``) every
315+
``KEEPALIVE_INTERVAL`` seconds to prevent idle-timeout disconnects.
316+
"""
317+
keepalive_interval = 15.0
318+
event_id = 0
319+
buffer = ""
320+
321+
log_iter = docker.container_logs(container_id, follow=True).__aiter__()
322+
while True:
323+
try:
324+
chunk = await asyncio.wait_for(log_iter.__anext__(), timeout=keepalive_interval)
325+
buffer += chunk
326+
while "\n" in buffer:
327+
line, buffer = buffer.split("\n", 1)
328+
line = line.strip()
329+
if not line:
330+
continue
331+
event_id += 1
332+
if event_id <= offset:
333+
continue
334+
yield f"id: {event_id}\ndata: {line}\n\n"
335+
except TimeoutError:
336+
# No data from container — send SSE keepalive comment to
337+
# prevent the HTTP connection from being closed by proxies
338+
# or the browser. SSE comments are silently ignored by
339+
# EventSource clients.
340+
yield ": keepalive\n\n"
341+
except StopAsyncIteration:
342+
break
300343

301344

302345
async def send_message(

apps/web/src/features/session/ContainerSession.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ describe('ContainerSession', () => {
171171
expect(mockEventSources.length).toBeGreaterThan(0)
172172
})
173173

174-
expect(mockedBuildStreamUrl).toHaveBeenCalledWith('test-session-id', 'test-token')
174+
expect(mockedBuildStreamUrl).toHaveBeenCalledWith('test-session-id', 'test-token', 0)
175175
})
176176

177177
test('calls onBack when back button is clicked', async () => {

apps/web/src/features/session/ContainerSession.tsx

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,15 @@ export default function ContainerSession({
5050
const [sending, setSending] = useState(false)
5151

5252
const lineIdRef = useRef(0)
53+
const sseOffsetRef = useRef(0)
54+
const reconnectCountRef = useRef(0)
5355
const eventSourceRef = useRef<EventSource | null>(null)
5456
const mountedRef = useRef(true)
5557
const inputRef = useRef<HTMLInputElement>(null)
5658

59+
const MAX_RECONNECTS = 5
60+
const RECONNECT_BASE_DELAY_MS = 2000
61+
5762
const appendLine = useCallback((text: string, kind?: TerminalLine['kind']) => {
5863
if (!mountedRef.current) return
5964
lineIdRef.current += 1
@@ -71,6 +76,8 @@ export default function ContainerSession({
7176
mountedRef.current = true
7277
const abortController = new AbortController()
7378
lineIdRef.current = 0
79+
sseOffsetRef.current = 0
80+
reconnectCountRef.current = 0
7481
setLines([])
7582

7683
async function init() {
@@ -109,6 +116,19 @@ export default function ContainerSession({
109116
}
110117

111118
async function pollSessionStatus(sessionId: string) {
119+
reconnectCountRef.current += 1
120+
if (reconnectCountRef.current > MAX_RECONNECTS) {
121+
setStatus('failed')
122+
setError('Lost connection to session')
123+
appendLine('[error] Lost connection after multiple retries.')
124+
return
125+
}
126+
127+
// Exponential backoff: 2s, 4s, 8s, 16s, 32s
128+
const delay = RECONNECT_BASE_DELAY_MS * Math.pow(2, reconnectCountRef.current - 1)
129+
await new Promise((r) => setTimeout(r, delay))
130+
if (!mountedRef.current) return
131+
112132
try {
113133
const fresh = await getContainerSession(sessionId)
114134
if (!mountedRef.current) return
@@ -119,7 +139,6 @@ export default function ContainerSession({
119139
setError('Container failed')
120140
appendLine('[error] Container failed.')
121141
} else if (fresh.status === 'running') {
122-
appendLine('[reconnecting] Stream dropped, container still running...')
123142
connectStream(sessionId)
124143
}
125144
} catch {
@@ -137,18 +156,24 @@ export default function ContainerSession({
137156
return
138157
}
139158

140-
const url = buildStreamUrl(sessionId, accessToken)
159+
const url = buildStreamUrl(sessionId, accessToken, sseOffsetRef.current)
141160
const source = new EventSource(url, { withCredentials: true })
142161
eventSourceRef.current = source
143162

144163
source.addEventListener('open', () => {
145164
if (!mountedRef.current) return
146165
setStatus('running')
166+
// Connection succeeded — reset reconnect counter
167+
reconnectCountRef.current = 0
147168
})
148169

149170
// Default message event — parse Claude Code stream-json
150171
source.onmessage = (event: MessageEvent) => {
151172
if (!mountedRef.current) return
173+
// Track the last event id so reconnects can skip already-received events
174+
if (event.lastEventId) {
175+
sseOffsetRef.current = parseInt(event.lastEventId, 10) || sseOffsetRef.current
176+
}
152177
const parsed = parseStreamEvent(event.data as string)
153178
if (parsed) {
154179
appendLine(parsed.text, parsed.kind)

apps/web/src/features/session/TerminalOutput.tsx

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,17 +89,11 @@ export default function TerminalOutput({ lines, isRunning }: TerminalOutputProps
8989
key={line.id}
9090
className="whitespace-pre-wrap break-words"
9191
style={
92-
line.kind === 'tool'
93-
? { color: '#6e6e73', fontSize: '12px' }
94-
: line.kind === 'system'
95-
? { color: '#6e6e73', fontSize: '12px', fontStyle: 'italic' }
96-
: line.kind === 'error'
97-
? { color: '#ff6961' }
98-
: line.kind === 'user'
99-
? { color: '#E2A039' }
100-
: line.kind === 'result'
101-
? { color: '#E0E0E0', marginTop: '16px', paddingTop: '16px', borderTop: '1px solid rgba(255,255,255,0.08)' }
102-
: undefined
92+
line.kind === 'error'
93+
? { color: '#ff6961' }
94+
: line.kind === 'status'
95+
? { color: '#9a9898', fontStyle: 'italic' }
96+
: undefined
10397
}
10498
>
10599
{line.text}

apps/web/src/features/session/containerApi.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,12 @@ export async function sendMessage(sessionId: string, content: string): Promise<v
8383
/**
8484
* Build the SSE stream URL for a container session.
8585
* Auth token is appended as a query parameter since EventSource cannot send headers.
86+
* Optional `offset` skips already-received events on reconnect.
8687
*/
87-
export function buildStreamUrl(sessionId: string, accessToken: string): string {
88-
return `${API_BASE}/api/v1/containers/sessions/${sessionId}/stream?access_token=${encodeURIComponent(accessToken)}`
88+
export function buildStreamUrl(sessionId: string, accessToken: string, offset?: number): string {
89+
let url = `${API_BASE}/api/v1/containers/sessions/${sessionId}/stream?access_token=${encodeURIComponent(accessToken)}`
90+
if (offset && offset > 0) {
91+
url += `&offset=${offset}`
92+
}
93+
return url
8994
}

0 commit comments

Comments
 (0)