Skip to content

Commit e453d8f

Browse files
author
Hex
committed
fix: chat architecture bugs — double response, unbounded caches, missing newline
- Fix critical double response in history endpoint (res.send + res.json crashed) - Cap currentWork items at 200 to prevent unbounded growth during long runs - Cap historyCache at 50 entries with LRU eviction - Add periodic cleanup for historyResponseCache (30s expiry) - Add tests for SSE client tracking, work item capping, live state cleanup - Fix syntax error (missing newline) that crashed server on hot-reload
1 parent 2fcb1fc commit e453d8f

File tree

4 files changed

+82
-1
lines changed

4 files changed

+82
-1
lines changed

packages/server/src/agent-backend/openclaw.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,9 @@ export function createOpenClawBackend(config: OpenClawConfig): AgentBackend & {
116116
const pending = new Map<string, { resolve: (v: unknown) => void; reject: (e: Error) => void }>()
117117

118118
// ── History cache: keyed by sessionKey, invalidated by file mtime+size ──
119+
// Capped to 50 entries to prevent unbounded growth
119120
const historyCache = new Map<string, { turns: ParsedTurn[]; hasMore: boolean; mtime: number; size: number }>()
121+
const HISTORY_CACHE_MAX = 50
120122
let msgId = 0
121123

122124
function getKeyPath(): string {
@@ -810,6 +812,11 @@ export function createOpenClawBackend(config: OpenClawConfig): AgentBackend & {
810812
mtime: stat.mtimeMs,
811813
size: stat.size
812814
})
815+
// Evict oldest entries if cache is too large
816+
if (historyCache.size > HISTORY_CACHE_MAX) {
817+
const firstKey = historyCache.keys().next().value
818+
if (firstKey) historyCache.delete(firstKey)
819+
}
813820

814821
return { turns, hasMore }
815822
}

packages/server/src/chat/chat.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,4 +345,66 @@ describe('§2.4 Chat Module (Server)', () => {
345345
expect(chatModule.getSessionKeyForThread('unknown-thread')).toBeTruthy()
346346
expect(chatModule.getSessionKeyForThread('unknown-thread')).toMatch(/^agent:main:thread:unknown-thread$/)
347347
})
348+
349+
it('MUST track and untrack SSE clients correctly', () => {
350+
chatModule.trackSSEClient('thread-1')
351+
chatModule.trackSSEClient('thread-1')
352+
chatModule.trackSSEClient('thread-2')
353+
chatModule.untrackSSEClient('thread-1')
354+
// Still one client on thread-1
355+
chatModule.untrackSSEClient('thread-1')
356+
// Zero clients on thread-1
357+
chatModule.untrackSSEClient('thread-1')
358+
// Should not go negative — just stays at 0
359+
chatModule.untrackSSEClient('thread-2')
360+
})
361+
362+
it('MUST cap accumulated work items at 200 to prevent unbounded growth', () => {
363+
// Set up a thread mapping
364+
const sessionKey = chatModule.resolveSessionKey('thread-cap')
365+
366+
// Simulate 250 work events
367+
const statusHandlers = backend._handlers.get('chat.work') ?? []
368+
for (let i = 0; i < 250; i++) {
369+
for (const handler of statusHandlers) {
370+
handler({ sessionKey, work: { type: 'tool_call', name: `tool-${i}`, timestamp: Date.now() } })
371+
}
372+
}
373+
374+
const live = chatModule.getLiveState('thread-cap')
375+
expect(live.work).toBeTruthy()
376+
expect(live.work!.length).toBeLessThanOrEqual(200)
377+
})
378+
379+
it('MUST clear live state on chat.turn event', () => {
380+
const sessionKey = chatModule.resolveSessionKey('thread-clear')
381+
382+
// Simulate status + work + stream events
383+
const statusHandlers = backend._handlers.get('chat.status') ?? []
384+
for (const h of statusHandlers) h({ sessionKey, status: 'working' })
385+
386+
const workHandlers = backend._handlers.get('chat.work') ?? []
387+
for (const h of workHandlers) h({ sessionKey, work: { type: 'tool_call', name: 'test', timestamp: Date.now() } })
388+
389+
const streamHandlers = backend._handlers.get('chat.stream') ?? []
390+
for (const h of streamHandlers) h({ sessionKey, text: 'hello' })
391+
392+
let live = chatModule.getLiveState('thread-clear')
393+
expect(live.status).toBe('working')
394+
expect(live.work?.length).toBeGreaterThan(0)
395+
expect(live.streamText).toBeTruthy()
396+
397+
// Now emit turn event — should clear all live state
398+
const turnHandlers = backend._handlers.get('chat.turn') ?? []
399+
for (const h of turnHandlers)
400+
h({
401+
sessionKey,
402+
turn: { role: 'assistant', content: 'done', workItems: [], thinkingBlocks: [], timestamp: Date.now() }
403+
})
404+
405+
live = chatModule.getLiveState('thread-clear')
406+
expect(live.status).toBeUndefined()
407+
expect(live.work).toBeUndefined()
408+
expect(live.streamText).toBeUndefined()
409+
})
348410
})

packages/server/src/chat/chat.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,8 @@ export function createChatModule(
211211
} else if (eventName === 'chat.work') {
212212
const items = currentWork.get(threadKey) ?? []
213213
items.push(data.work)
214+
// Cap accumulated work items to prevent unbounded growth during long agent runs
215+
if (items.length > 200) items.splice(0, items.length - 200)
214216
currentWork.set(threadKey, items)
215217
// Clear accumulated stream text when a tool call arrives —
216218
// the text before the tool call was captured as a thinking item

packages/server/src/chat/routes.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,17 @@ export function createChatRoutes(chatModule: ChatModule, backend: AgentBackend,
9494
const historyResponseCache = new Map<string, { data: any; timestamp: number }>()
9595
const HISTORY_CACHE_TTL = 5000 // 5s — fresh enough for human perception, avoids constant re-parse
9696

97+
// Periodic cleanup of stale cache entries (prevent unbounded growth)
98+
setInterval(() => {
99+
const now = Date.now()
100+
for (const [key, entry] of historyResponseCache) {
101+
if (now - entry.timestamp > HISTORY_CACHE_TTL * 6) {
102+
// 30s expiry
103+
historyResponseCache.delete(key)
104+
}
105+
}
106+
}, 30_000)
107+
97108
router.get('/api/threads/:threadKey/history', async (req, res) => {
98109
const threadKey = req.params.threadKey
99110
const sessionKey = chatModule.resolveSessionKey(threadKey)
@@ -114,7 +125,6 @@ export function createChatRoutes(chatModule: ChatModule, backend: AgentBackend,
114125
const json = JSON.stringify(data)
115126
res.setHeader('Content-Type', 'application/json')
116127
res.send(json)
117-
res.json({ turns: result.turns, hasMore: result.hasMore })
118128
} catch {
119129
res.json({ turns: [], hasMore: false })
120130
}

0 commit comments

Comments
 (0)