diff --git a/packages/agents-core/src/run.ts b/packages/agents-core/src/run.ts index 474f94c6..078614b4 100644 --- a/packages/agents-core/src/run.ts +++ b/packages/agents-core/src/run.ts @@ -732,24 +732,35 @@ export class Runner extends RunHooks> { state._originalInput = turnResult.originalInput; state._generatedItems = turnResult.generatedItems; - if (turnResult.nextStep.type === 'next_step_run_again') { - state._currentTurnPersistedItemCount = 0; - } - state._currentStep = turnResult.nextStep; + // Don't reset counter here - resolveInterruptedTurn already adjusted it via rewind logic + // The counter will be reset when _currentTurn is incremented (starting a new turn) if (turnResult.nextStep.type === 'next_step_interruption') { // we are still in an interruption, so we need to avoid an infinite loop + state._currentStep = turnResult.nextStep; return new RunResult(state); } - continue; + // If continuing from interruption with next_step_run_again, set step to undefined + // so the loop treats it as a new step without incrementing the turn. + // The counter has already been adjusted by resolveInterruptedTurn's rewind logic. + if (turnResult.nextStep.type === 'next_step_run_again') { + state._currentStep = undefined; + continue; + } + + state._currentStep = turnResult.nextStep; } if (state._currentStep.type === 'next_step_run_again') { const artifacts = await prepareAgentArtifacts(state); - state._currentTurn++; - state._currentTurnPersistedItemCount = 0; + // Only increment turn and reset counter if we're starting a new turn, + // not if we're continuing from an interruption (which would have _lastTurnResponse set) + if (!state._lastTurnResponse) { + state._currentTurn++; + state._currentTurnPersistedItemCount = 0; + } if (state._currentTurn > state._maxTurns) { state._currentAgentSpan?.setError({ @@ -770,7 +781,8 @@ export class Runner extends RunHooks> { let parallelGuardrailPromise: | Promise | undefined; - if (state._currentTurn === 1) { + // Only run input guardrails on the first turn of a new run, not when continuing from an interruption + if (state._currentTurn === 1 && !state._lastTurnResponse) { const guardrails = this.#splitInputGuardrails(state); if (guardrails.blocking.length > 0) { await this.#runInputGuardrails(state, guardrails.blocking); @@ -1021,22 +1033,35 @@ export class Runner extends RunHooks> { result.state._originalInput = turnResult.originalInput; result.state._generatedItems = turnResult.generatedItems; - if (turnResult.nextStep.type === 'next_step_run_again') { - result.state._currentTurnPersistedItemCount = 0; - } - result.state._currentStep = turnResult.nextStep; + // Don't reset counter here - resolveInterruptedTurn already adjusted it via rewind logic + // The counter will be reset when _currentTurn is incremented (starting a new turn) + if (turnResult.nextStep.type === 'next_step_interruption') { // we are still in an interruption, so we need to avoid an infinite loop + result.state._currentStep = turnResult.nextStep; return; } - continue; + + // If continuing from interruption with next_step_run_again, set step to undefined + // so the loop treats it as a new step without incrementing the turn. + // The counter has already been adjusted by resolveInterruptedTurn's rewind logic. + if (turnResult.nextStep.type === 'next_step_run_again') { + result.state._currentStep = undefined; + continue; + } + + result.state._currentStep = turnResult.nextStep; } if (result.state._currentStep.type === 'next_step_run_again') { const artifacts = await prepareAgentArtifacts(result.state); - result.state._currentTurn++; - result.state._currentTurnPersistedItemCount = 0; + // Only increment turn and reset counter if we're starting a new turn, + // not if we're continuing from an interruption (which would have _lastTurnResponse set) + if (!result.state._lastTurnResponse) { + result.state._currentTurn++; + result.state._currentTurnPersistedItemCount = 0; + } if (result.state._currentTurn > result.state._maxTurns) { result.state._currentAgentSpan?.setError({ @@ -1057,7 +1082,11 @@ export class Runner extends RunHooks> { let parallelGuardrailPromise: | Promise | undefined; - if (result.state._currentTurn === 1) { + // Only run input guardrails on the first turn of a new run, not when continuing from an interruption + if ( + result.state._currentTurn === 1 && + !result.state._lastTurnResponse + ) { const guardrails = this.#splitInputGuardrails(result.state); if (guardrails.blocking.length > 0) { await this.#runInputGuardrails(result.state, guardrails.blocking); diff --git a/packages/agents-core/test/run.test.ts b/packages/agents-core/test/run.test.ts index e1befe9b..b151a51b 100644 --- a/packages/agents-core/test/run.test.ts +++ b/packages/agents-core/test/run.test.ts @@ -1235,6 +1235,101 @@ describe('Runner.run', () => { expect(savedHostedCall.providerData).toEqual(hostedCall.providerData); expect(savedHostedCall.id).toBe('approval-1'); }); + + it('prevents duplicate function_call items when resuming from interruption after tool approval', async () => { + // Regression test for issue #701 + // + // Bug: When resuming a turn after approving a tool call, duplicate function_call items + // were saved to the session. The bug occurred because _currentTurnPersistedItemCount + // was incorrectly reset to 0 after resolveInterruptedTurn returned next_step_run_again, + // causing saveToSession to save all items from the beginning of the turn, including + // the already-persisted function_call item. + // + // Expected behavior: Only 1 function_call item should be saved to the session. + // Buggy behavior: 2 function_call items (duplicate) were saved. + // + // Test scenario: + // 1. Initial run with tool requiring approval creates an interruption + // 2. Tool call is approved + // 3. Run is resumed with the approved state + // 4. Session should contain exactly 1 function_call item (not 2) + + const getWeatherTool = tool({ + name: 'get_weather', + description: 'Get weather for a city', + parameters: z.object({ city: z.string() }), + needsApproval: async () => true, // Require approval for all calls + execute: async ({ city }) => `Sunny, 72°F in ${city}`, + }); + + const model = new FakeModel([ + // First response: tool call that requires approval + { + output: [ + { + type: 'function_call', + id: 'fc_1', + callId: 'call_weather_1', + name: 'get_weather', + status: 'completed', + arguments: JSON.stringify({ city: 'Oakland' }), + providerData: {}, + } as protocol.FunctionCallItem, + ], + usage: new Usage(), + }, + // Second response: after approval, final answer + { + output: [fakeModelMessage('The weather is sunny in Oakland.')], + usage: new Usage(), + }, + ]); + + const agent = new Agent({ + name: 'Assistant', + instructions: 'Use get_weather tool to answer weather questions.', + model, + tools: [getWeatherTool], + toolUseBehavior: 'run_llm_again', // Must use 'run_llm_again' so resolveInterruptedTurn returns next_step_run_again + }); + + const session = new MemorySession(); + + // Use sessionInputCallback to match the scenario from issue #701 + const sessionInputCallback = async ( + historyItems: AgentInputItem[], + newItems: AgentInputItem[], + ) => { + return [...historyItems, ...newItems]; + }; + + // Step 1: Initial run creates an interruption for tool approval + let result = await run( + agent, + [{ role: 'user', content: "What's the weather in Oakland?" }], + { session, sessionInputCallback }, + ); + + // Step 2: Approve the tool call + for (const interruption of result.interruptions || []) { + result.state.approve(interruption); + } + + // Step 3: Resume the run with the approved state + // Note: No sessionInputCallback on resume - this is part of the bug scenario + result = await run(agent, result.state, { session }); + + // Step 4: Verify only one function_call item exists in the session + const allItems = await session.getItems(); + const functionCalls = allItems.filter( + (item): item is protocol.FunctionCallItem => + item.type === 'function_call' && item.callId === 'call_weather_1', + ); + + // The bug would cause 2 function_call items to be saved (duplicate) + // The fix ensures only 1 function_call item is saved + expect(functionCalls).toHaveLength(1); + }); }); });