Skip to content

Commit 23ea5bd

Browse files
committed
fix local codex review comment
- [P1] Preserve new session items when callback mutates history — packages/agents-core/src/runImplementation.ts:1651-1688 If a sessionInputCallback mutates the history array (e.g. the natural pattern history.push(...newItems.map(item => ({ ...item }))); return history; to annotate new rows) the logic here misclassifies those freshly added items as existing history. Because we build historyCounts/historyRefs after the callback, the cloned entries land in historyRefs and the loop then skips them, leaving appended empty. Downstream we persist sessionItems: appended.length > 0 ? appended : [], so the turn’s new input never gets written to the session. That silently drops user input for any in-place merge strategy, which is a severe regression for the new memory feature.
1 parent 4d7cdbe commit 23ea5bd

File tree

2 files changed

+50
-5
lines changed

2 files changed

+50
-5
lines changed

packages/agents-core/src/runImplementation.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,6 +1656,11 @@ export async function prepareInputItemsWithSession(
16561656
};
16571657
}
16581658

1659+
// Capture snapshots before invoking the callback so we can reason about the original state even
1660+
// if the callback mutates the history array in-place.
1661+
const historySnapshot = history.slice();
1662+
const newInputSnapshot = newInputItems.slice();
1663+
16591664
// Delegate history reconciliation to the user-supplied callback. It must return a concrete list
16601665
// to keep downstream model requests well-typed.
16611666
const combined = await sessionInputCallback(history, newInputItems);
@@ -1665,10 +1670,10 @@ export async function prepareInputItemsWithSession(
16651670
);
16661671
}
16671672

1668-
const historyCounts = buildItemFrequencyMap(history);
1669-
const newInputCounts = buildItemFrequencyMap(newInputItems);
1670-
const historyRefs = buildItemReferenceMap(history);
1671-
const newInputRefs = buildItemReferenceMap(newInputItems);
1673+
const historyCounts = buildItemFrequencyMap(historySnapshot);
1674+
const newInputCounts = buildItemFrequencyMap(newInputSnapshot);
1675+
const historyRefs = buildItemReferenceMap(historySnapshot);
1676+
const newInputRefs = buildItemReferenceMap(newInputSnapshot);
16721677

16731678
const appended: AgentInputItem[] = [];
16741679
for (const item of combined) {
@@ -1707,7 +1712,7 @@ export async function prepareInputItemsWithSession(
17071712
? combined
17081713
: appended.length > 0
17091714
? appended
1710-
: newInputItems,
1715+
: newInputSnapshot,
17111716
sessionItems: appended.length > 0 ? appended : [],
17121717
};
17131718
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,46 @@ describe('prepareInputItemsWithSession', () => {
347347
expect(sessionItems[0]).toBe(newItem);
348348
});
349349

350+
it('persists appended copies when callbacks mutate history in place', async () => {
351+
const historyItem: AgentInputItem = {
352+
type: 'message',
353+
role: 'user',
354+
content: 'past',
355+
id: 'history-1',
356+
};
357+
const newItem: AgentInputItem = {
358+
type: 'message',
359+
role: 'user',
360+
content: 'fresh',
361+
id: 'new-1',
362+
};
363+
const session = new StubSession([historyItem]);
364+
365+
let appendedItems: AgentInputItem[] = [];
366+
const result = await prepareInputItemsWithSession(
367+
[newItem],
368+
session,
369+
(history, newItems) => {
370+
appendedItems = newItems.map((item) => ({
371+
...item,
372+
providerData: { annotated: true },
373+
}));
374+
history.push(...appendedItems);
375+
return history;
376+
},
377+
);
378+
379+
expect(appendedItems).toHaveLength(1);
380+
expect(result.preparedInput).toEqual([historyItem, ...appendedItems]);
381+
const sessionItems = result.sessionItems;
382+
if (!sessionItems) {
383+
throw new Error('Expected sessionItems to be defined.');
384+
}
385+
expect(sessionItems).toEqual(appendedItems);
386+
expect(sessionItems[0]).toBe(appendedItems[0]);
387+
expect(sessionItems[0]).not.toBe(newItem);
388+
});
389+
350390
it('omits session history from prepared input when includeHistoryInPreparedInput is false', async () => {
351391
const historyItem: AgentInputItem = {
352392
type: 'message',

0 commit comments

Comments
 (0)