Skip to content

Commit 7892cdc

Browse files
committed
fix local codex review
[P1] Avoid duplicating session history — packages/agents-core/src/runImplementation.ts:1861-1872 When we compute sessionItems for a custom sessionInputCallback, we rely on reference equality (historySet.has(item)) to decide which entries came from prior history. That assumes the callback returns the exact same object references for every preserved history item. A very common sanitization pattern is to clone history items (e.g. history.map(item => structuredClone(item))) before returning them. In that case none of the clones are found in historySet or newInputSet, so every history entry is treated as “appended” and we persist the entire history again. After one or two turns the session now contains duplicate copies of every message and will grow explosively on subsequent runs. Please adjust the change detection (e.g. track by index/length or require the callback to return new items explicitly) so that cloning history entries does not cause the memory to balloon.
1 parent ce85e41 commit 7892cdc

File tree

2 files changed

+59
-11
lines changed

2 files changed

+59
-11
lines changed

packages/agents-core/src/runImplementation.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1858,22 +1858,35 @@ export async function prepareInputItemsWithSession(
18581858
);
18591859
}
18601860

1861-
const historySet = new Set(history);
1862-
const newInputSet = new Set(newInputItems);
1861+
const historyCounts = new Map<string, number>();
1862+
for (const item of history) {
1863+
const key = toSmartString(item);
1864+
historyCounts.set(key, (historyCounts.get(key) ?? 0) + 1);
1865+
}
1866+
1867+
const newInputCounts = new Map<string, number>();
1868+
for (const item of newInputItems) {
1869+
const key = toSmartString(item);
1870+
newInputCounts.set(key, (newInputCounts.get(key) ?? 0) + 1);
1871+
}
1872+
18631873
const appended: AgentInputItem[] = [];
18641874
for (const item of combined) {
1865-
if (historySet.has(item) || !newInputSet.has(item)) {
1866-
if (!historySet.has(item) && !newInputSet.has(item)) {
1867-
appended.push(item);
1868-
}
1875+
const key = toSmartString(item);
1876+
const historyRemaining = historyCounts.get(key) ?? 0;
1877+
if (historyRemaining > 0) {
1878+
historyCounts.set(key, historyRemaining - 1);
18691879
continue;
18701880
}
1871-
appended.push(item);
1872-
}
18731881

1874-
if (appended.length === 0 && combined.length > history.length) {
1875-
// When callbacks replace every new item with fresh objects, fall back to the tail slice.
1876-
appended.push(...combined.slice(history.length));
1882+
const newRemaining = newInputCounts.get(key) ?? 0;
1883+
if (newRemaining > 0) {
1884+
newInputCounts.set(key, newRemaining - 1);
1885+
appended.push(item);
1886+
continue;
1887+
}
1888+
1889+
appended.push(item);
18771890
}
18781891

18791892
return {

packages/agents-core/test/run.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,41 @@ describe('Runner.run', () => {
813813
});
814814
});
815815

816+
it('does not duplicate history when callback clones entries', async () => {
817+
const model = new RecordingModel([
818+
{
819+
...TEST_MODEL_RESPONSE_BASIC,
820+
output: [fakeModelMessage('clone response')],
821+
},
822+
]);
823+
const history = [user('Existing history item')];
824+
const session = new MemorySession(history);
825+
const agent = new Agent({ name: 'CloneSession', model });
826+
const runner = new Runner();
827+
828+
await runner.run(agent, [user('Fresh input')], {
829+
session,
830+
sessionInputCallback: (incomingHistory, newItems) => {
831+
const clonedHistory = incomingHistory.map((item) =>
832+
structuredClone(item),
833+
);
834+
const clonedNewItems = newItems.map((item) =>
835+
structuredClone(item),
836+
);
837+
return clonedHistory.concat(clonedNewItems);
838+
},
839+
});
840+
841+
expect(session.added).toHaveLength(1);
842+
const [persistedItems] = session.added;
843+
const persistedUsers = persistedItems.filter(
844+
(item): item is protocol.UserMessageItem =>
845+
item.type === 'message' && 'role' in item && item.role === 'user',
846+
);
847+
expect(persistedUsers).toHaveLength(1);
848+
expect(getFirstTextContent(persistedUsers[0])).toBe('Fresh input');
849+
});
850+
816851
it('throws when session input callback returns invalid data', async () => {
817852
const model = new RecordingModel([
818853
{

0 commit comments

Comments
 (0)