Skip to content

Commit 98d56c0

Browse files
committed
fix local codex review comment
- [P1] Prevent duplicating run items on approval resume — packages/agents-core/src/runImplementation.ts:460-462 When we resume a turn after tool approval, originalPreStepItems already contains the assistant message/tool-call items that were generated before the interruption. Initializing newItems with processedResponse.newItems reuses those same instances, so SingleStepResult.generatedItems ends up concatenating [...preStepItems, ...newItems] where the message/tool-call appear twice. This causes duplicate entries to be persisted in session history and re-emitted to callers every time an approval is resolved. The previous implementation started from the tool results only (const newItems = functionResults.map(...)) to avoid this duplication. We should revert to building newItems from the newly produced tool results instead of cloning the prior processedResponse.newItems array.
1 parent 6c57d2a commit 98d56c0

File tree

2 files changed

+68
-3
lines changed

2 files changed

+68
-3
lines changed

packages/agents-core/src/runImplementation.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,19 @@ export async function resolveTurnAfterModelResponse<TContext>(
457457
state: RunState<TContext, Agent<TContext, any>>,
458458
): Promise<SingleStepResult> {
459459
const preStepItems = originalPreStepItems;
460-
let newItems = processedResponse.newItems;
460+
const seenItems = new Set<RunItem>(originalPreStepItems);
461+
const newItems: RunItem[] = [];
462+
const appendIfNew = (item: RunItem) => {
463+
if (seenItems.has(item)) {
464+
return;
465+
}
466+
newItems.push(item);
467+
seenItems.add(item);
468+
};
469+
470+
for (const item of processedResponse.newItems) {
471+
appendIfNew(item);
472+
}
461473

462474
const [functionResults, computerResults] = await Promise.all([
463475
executeFunctionToolCalls(
@@ -474,8 +486,12 @@ export async function resolveTurnAfterModelResponse<TContext>(
474486
),
475487
]);
476488

477-
newItems = newItems.concat(functionResults.map((r) => r.runItem));
478-
newItems = newItems.concat(computerResults);
489+
for (const result of functionResults) {
490+
appendIfNew(result.runItem);
491+
}
492+
for (const item of computerResults) {
493+
appendIfNew(item);
494+
}
479495

480496
// run hosted MCP approval requests
481497
if (processedResponse.mcpApprovalRequests.length > 0) {

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1744,6 +1744,55 @@ describe('resolveTurnAfterModelResponse', () => {
17441744
expect(result.nextStep.type).toBe('next_step_run_again');
17451745
});
17461746

1747+
it('does not duplicate previously persisted model items when resuming after approvals', async () => {
1748+
const toolCall = {
1749+
...TEST_MODEL_FUNCTION_CALL,
1750+
id: 'call-resume',
1751+
callId: 'call-resume',
1752+
};
1753+
const message = fakeModelMessage('Tool approval pending');
1754+
message.id = 'message-resume';
1755+
const response: ModelResponse = {
1756+
output: [toolCall, message],
1757+
usage: new Usage(),
1758+
} as any;
1759+
1760+
const processedResponse = processModelResponse(
1761+
response,
1762+
TEST_AGENT,
1763+
[TEST_TOOL],
1764+
[],
1765+
);
1766+
1767+
const priorItems = [...processedResponse.newItems];
1768+
state._generatedItems = priorItems;
1769+
1770+
const result = await withTrace('test', () =>
1771+
resolveTurnAfterModelResponse(
1772+
TEST_AGENT,
1773+
'test input',
1774+
priorItems,
1775+
response,
1776+
processedResponse,
1777+
runner,
1778+
state,
1779+
),
1780+
);
1781+
1782+
const persistedToolCalls = result.generatedItems.filter((item) => {
1783+
return item instanceof ToolCallItem && item.rawItem.id === 'call-resume';
1784+
});
1785+
expect(persistedToolCalls).toHaveLength(1);
1786+
1787+
const persistedMessages = result.generatedItems.filter((item) => {
1788+
return (
1789+
item instanceof MessageOutputItem &&
1790+
item.rawItem.id === 'message-resume'
1791+
);
1792+
});
1793+
expect(persistedMessages).toHaveLength(1);
1794+
});
1795+
17471796
it('does not finalize when hosted MCP approval happens in the same turn; runs again', async () => {
17481797
const approvalAgent = new Agent({ name: 'MCPAgent', outputType: 'text' });
17491798
const mcpTool = hostedMcpTool({

0 commit comments

Comments
 (0)