Skip to content

Commit 7ccce6a

Browse files
committed
fix(memory): admit only complete stored tool exchanges
1 parent f912fde commit 7ccce6a

4 files changed

Lines changed: 170 additions & 15 deletions

File tree

‎apps/sim/executor/handlers/agent/memory.durability.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,42 @@ describe('optional Agent memory durability failures', () => {
208208
}
209209
)
210210

211+
it.each(['missing', 'mismatched', 'intervening', 'duplicate', 'orphan'])(
212+
'omits a %s legacy result group before provider conversion',
213+
async (failure) => {
214+
const call = {
215+
role: 'assistant',
216+
content: null,
217+
function_call: { name: 'lookup', arguments: '{}' },
218+
}
219+
const result = { role: 'function', name: 'lookup', content: 'Saved result' }
220+
const messages =
221+
failure === 'missing'
222+
? [call]
223+
: failure === 'mismatched'
224+
? [call, { ...result, name: 'different' }]
225+
: failure === 'intervening'
226+
? [call, { role: 'user', content: 'interruption' }, result]
227+
: failure === 'duplicate'
228+
? [call, result, result]
229+
: [result]
230+
mocks.items.mockResolvedValue({
231+
items: [
232+
{
233+
kind: 'exchange',
234+
appendKey: 'step:1',
235+
turnId: 'previous-turn',
236+
data: { version: 1, messages },
237+
provenance: { status: 'exact', entries: [] },
238+
},
239+
],
240+
})
241+
await expect(
242+
new Memory().fetchMemoryMessages(ctx, inputs, undefined, { richHistory: true })
243+
).resolves.toEqual(prefix)
244+
}
245+
)
246+
211247
it('drops optional scoped appends when storage fails without retrying an unscoped write', async () => {
212248
mocks.append.mockRejectedValue(storageFailure())
213249
await expect(

‎apps/sim/executor/handlers/agent/memory.ts‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
readPlainMemoryTail,
3030
seedMemoryMessages,
3131
} from '@/lib/memory/conversation-store'
32+
import { parseConversationHistoryGroup } from '@/lib/memory/history-group'
3233
import {
3334
markConversationExchangeGroup,
3435
selectConversationContextWindow,
@@ -744,21 +745,8 @@ export class Memory {
744745
encryptedNative =
745746
typeof item.data.encryptedNative === 'string' ? item.data.encryptedNative : undefined
746747
}
747-
const valid = values.every(
748-
(value) =>
749-
isPlainRecord(value) &&
750-
['system', 'user', 'assistant', 'tool', 'function'].includes(String(value.role)) &&
751-
(typeof value.content === 'string' ||
752-
(value.role === 'assistant' &&
753-
value.content === null &&
754-
(Array.isArray(value.tool_calls) ||
755-
(isPlainRecord(value.function_call) &&
756-
typeof value.function_call.name === 'string' &&
757-
value.function_call.name.length > 0 &&
758-
typeof value.function_call.arguments === 'string'))))
759-
)
760-
if (!valid || values.length === 0) continue
761-
const group = values as Message[]
748+
const group = parseConversationHistoryGroup(values)
749+
if (!group) continue
762750
const groupBytes =
763751
Buffer.byteLength(JSON.stringify(item.data), 'utf8') +
764752
Buffer.byteLength(JSON.stringify(item.provenance), 'utf8')
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/** @vitest-environment node */
2+
import { describe, expect, it } from 'vitest'
3+
import { parseConversationHistoryGroup } from '@/lib/memory/history-group'
4+
5+
const legacyCall = {
6+
role: 'assistant',
7+
content: null,
8+
function_call: { name: 'lookup', arguments: '{}' },
9+
}
10+
const legacyResult = { role: 'function', name: 'lookup', content: 'result' }
11+
const toolCall = (id: string) => ({
12+
id,
13+
type: 'function',
14+
function: { name: 'lookup', arguments: '{}' },
15+
})
16+
const parallelCall = {
17+
role: 'assistant',
18+
content: null,
19+
tool_calls: [toolCall('first'), toolCall('second')],
20+
}
21+
const result = (id: string) => ({ role: 'tool', tool_call_id: id, content: 'result' })
22+
const text = { role: 'user', content: 'conversation text or bounded execution record' }
23+
24+
describe('stored conversation history groups', () => {
25+
it.each([
26+
[text],
27+
[legacyCall, legacyResult],
28+
[{ ...legacyCall, content: 'Calling lookup' }, legacyResult],
29+
[legacyCall, legacyResult, legacyCall, legacyResult],
30+
[parallelCall, result('second'), result('first')],
31+
[parallelCall, result('first'), { ...result('second'), content: '{"error":"tool failed"}' }],
32+
])('retains complete history unchanged: %j', (...group) => {
33+
expect(parseConversationHistoryGroup(group)).toBe(group)
34+
})
35+
36+
it.each([
37+
[],
38+
[{ role: { toString: 1 }, content: 'invalid role' }],
39+
[legacyCall],
40+
[{ ...legacyCall, content: 'Calling lookup' }],
41+
[legacyResult],
42+
[legacyCall, { ...legacyResult, name: 'different' }],
43+
[legacyCall, legacyResult, legacyResult],
44+
[legacyCall, text, legacyResult],
45+
[legacyCall, result('lookup')],
46+
[parallelCall],
47+
[parallelCall, result('first')],
48+
[parallelCall, result('first'), result('first')],
49+
[parallelCall, result('first'), result('different')],
50+
[parallelCall, result('first'), text, result('second')],
51+
[result('first'), parallelCall, result('first'), result('second')],
52+
[{ ...parallelCall, tool_calls: [toolCall('first'), toolCall('first')] }, result('first')],
53+
[{ ...parallelCall, tool_calls: [{ ...toolCall('first'), id: '' }] }, result('')],
54+
[{ ...parallelCall, function_call: legacyCall.function_call }, legacyResult],
55+
[{ ...legacyCall, role: 'user' }, legacyResult],
56+
[{ ...parallelCall, role: 'user' }, result('first'), result('second')],
57+
[{ role: 'assistant', content: null }],
58+
[{ role: 'assistant', content: null, tool_calls: [] }],
59+
[{ role: 'assistant', content: 'text', tool_calls: 'invalid' }],
60+
[{ ...legacyCall, function_call: { name: 'lookup', arguments: {} } }, legacyResult],
61+
])('omits incomplete or malformed groups: %j', (...group) => {
62+
expect(parseConversationHistoryGroup(group)).toBeUndefined()
63+
})
64+
})
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { isPlainRecord } from '@sim/utils/object'
2+
import type { Message } from '@/providers/types'
3+
4+
/** Stored history is admitted as a whole group, with adjacent, fully resolved call batches. */
5+
export function parseConversationHistoryGroup(
6+
values: readonly unknown[]
7+
): readonly Message[] | undefined {
8+
if (values.length === 0) return undefined
9+
const pending = new Set<string>()
10+
for (const value of values) {
11+
if (
12+
!isPlainRecord(value) ||
13+
typeof value.role !== 'string' ||
14+
!['system', 'user', 'assistant', 'tool', 'function'].includes(value.role)
15+
)
16+
return undefined
17+
18+
const calls: string[] = []
19+
if (value.function_call != null) {
20+
const call = value.function_call
21+
if (
22+
!isPlainRecord(call) ||
23+
typeof call.name !== 'string' ||
24+
!call.name ||
25+
typeof call.arguments !== 'string'
26+
)
27+
return undefined
28+
calls.push(`function:${call.name}`)
29+
}
30+
if (value.tool_calls != null) {
31+
if (!Array.isArray(value.tool_calls) || (calls.length > 0 && value.tool_calls.length > 0))
32+
return undefined
33+
for (const call of value.tool_calls) {
34+
if (
35+
!isPlainRecord(call) ||
36+
typeof call.id !== 'string' ||
37+
!call.id ||
38+
call.type !== 'function' ||
39+
!isPlainRecord(call.function) ||
40+
typeof call.function.name !== 'string' ||
41+
!call.function.name ||
42+
typeof call.function.arguments !== 'string'
43+
)
44+
return undefined
45+
calls.push(`tool:${call.id}`)
46+
}
47+
}
48+
if (
49+
(calls.length > 0 && value.role !== 'assistant') ||
50+
(typeof value.content !== 'string' &&
51+
!(value.role === 'assistant' && value.content === null && calls.length > 0))
52+
)
53+
return undefined
54+
55+
if (value.role === 'function' || value.role === 'tool') {
56+
const id = value.role === 'function' ? value.name : value.tool_call_id
57+
if (typeof id !== 'string' || !pending.delete(`${value.role}:${id}`)) return undefined
58+
} else {
59+
if (pending.size > 0) return undefined
60+
for (const call of calls) {
61+
if (pending.has(call)) return undefined
62+
pending.add(call)
63+
}
64+
}
65+
}
66+
return pending.size === 0 ? (values as readonly Message[]) : undefined
67+
}

0 commit comments

Comments
 (0)