Skip to content

Commit 152c8d1

Browse files
committed
fix: reset to main branch behavior to verify bug test failure
1 parent e058c31 commit 152c8d1

File tree

3 files changed

+108
-66
lines changed

3 files changed

+108
-66
lines changed

packages/agents-core/src/run.ts

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -732,36 +732,24 @@ export class Runner extends RunHooks<any, AgentOutputType<unknown>> {
732732

733733
state._originalInput = turnResult.originalInput;
734734
state._generatedItems = turnResult.generatedItems;
735-
// Don't reset counter here - it's already been adjusted by resolveInterruptedTurn's rewind logic
736-
// Counter will be reset when _currentTurn is incremented (starting a new turn)
735+
if (turnResult.nextStep.type === 'next_step_run_again') {
736+
state._currentTurnPersistedItemCount = 0;
737+
}
737738
state._currentStep = turnResult.nextStep;
738739

739740
if (turnResult.nextStep.type === 'next_step_interruption') {
740741
// we are still in an interruption, so we need to avoid an infinite loop
741742
return new RunResult<TContext, TAgent>(state);
742743
}
743744

744-
// If continuing from interruption with next_step_run_again, continue the loop
745-
// but DON'T increment turn or reset counter - we're continuing the same turn
746-
if (turnResult.nextStep.type === 'next_step_run_again') {
747-
continue;
748-
}
749-
750745
continue;
751746
}
752747

753748
if (state._currentStep.type === 'next_step_run_again') {
754749
const artifacts = await prepareAgentArtifacts(state);
755750

756-
// Only increment turn and reset counter when starting a NEW turn
757-
// If counter is non-zero, it means we're continuing from an interruption (counter was rewound)
758-
// In that case, don't reset the counter - it's already been adjusted by the rewind logic
759751
state._currentTurn++;
760-
if (state._currentTurnPersistedItemCount === 0) {
761-
// Only reset if counter is already 0 (starting a new turn)
762-
// If counter is non-zero, we're continuing from interruption and it was already adjusted
763-
state._currentTurnPersistedItemCount = 0;
764-
}
752+
state._currentTurnPersistedItemCount = 0;
765753

766754
if (state._currentTurn > state._maxTurns) {
767755
state._currentAgentSpan?.setError({
@@ -879,8 +867,9 @@ export class Runner extends RunHooks<any, AgentOutputType<unknown>> {
879867

880868
state._originalInput = turnResult.originalInput;
881869
state._generatedItems = turnResult.generatedItems;
882-
// Don't reset counter here - it's already been adjusted by resolveInterruptedTurn's rewind logic
883-
// Counter will be reset when _currentTurn is incremented (starting a new turn)
870+
if (turnResult.nextStep.type === 'next_step_run_again') {
871+
state._currentTurnPersistedItemCount = 0;
872+
}
884873
state._currentStep = turnResult.nextStep;
885874

886875
if (parallelGuardrailPromise) {
@@ -1032,8 +1021,9 @@ export class Runner extends RunHooks<any, AgentOutputType<unknown>> {
10321021

10331022
result.state._originalInput = turnResult.originalInput;
10341023
result.state._generatedItems = turnResult.generatedItems;
1035-
// Don't reset counter here - it's already been adjusted by resolveInterruptedTurn's rewind logic
1036-
// Counter will be reset when _currentTurn is incremented (starting a new turn)
1024+
if (turnResult.nextStep.type === 'next_step_run_again') {
1025+
result.state._currentTurnPersistedItemCount = 0;
1026+
}
10371027
result.state._currentStep = turnResult.nextStep;
10381028
if (turnResult.nextStep.type === 'next_step_interruption') {
10391029
// we are still in an interruption, so we need to avoid an infinite loop
@@ -1045,15 +1035,8 @@ export class Runner extends RunHooks<any, AgentOutputType<unknown>> {
10451035
if (result.state._currentStep.type === 'next_step_run_again') {
10461036
const artifacts = await prepareAgentArtifacts(result.state);
10471037

1048-
// Only increment turn and reset counter when starting a NEW turn
1049-
// If counter is non-zero, it means we're continuing from an interruption (counter was rewound)
1050-
// In that case, don't reset the counter - it's already been adjusted by the rewind logic
10511038
result.state._currentTurn++;
1052-
if (result.state._currentTurnPersistedItemCount === 0) {
1053-
// Only reset if counter is already 0 (starting a new turn)
1054-
// If counter is non-zero, we're continuing from interruption and it was already adjusted
1055-
result.state._currentTurnPersistedItemCount = 0;
1056-
}
1039+
result.state._currentTurnPersistedItemCount = 0;
10571040

10581041
if (result.state._currentTurn > result.state._maxTurns) {
10591042
result.state._currentAgentSpan?.setError({
@@ -1225,15 +1208,10 @@ export class Runner extends RunHooks<any, AgentOutputType<unknown>> {
12251208

12261209
result.state._originalInput = turnResult.originalInput;
12271210
result.state._generatedItems = turnResult.generatedItems;
1228-
// Don't reset counter here - it's already been adjusted by resolveInterruptedTurn's rewind logic
1229-
// Counter will be reset when _currentTurn is incremented (starting a new turn)
1230-
result.state._currentStep = turnResult.nextStep;
1231-
1232-
// If continuing from interruption with next_step_run_again, don't increment turn or reset counter
1233-
// We're continuing the same turn, not starting a new one
12341211
if (turnResult.nextStep.type === 'next_step_run_again') {
1235-
continue;
1212+
result.state._currentTurnPersistedItemCount = 0;
12361213
}
1214+
result.state._currentStep = turnResult.nextStep;
12371215
}
12381216

12391217
if (result.state._currentStep.type === 'next_step_final_output') {

packages/agents-core/src/runImplementation.ts

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -687,37 +687,6 @@ export async function resolveInterruptedTurn<TContext>(
687687
return false;
688688
});
689689

690-
// Filter out handoffs that were already executed before the interruption.
691-
// Handoffs that were already executed will have their call items in originalPreStepItems.
692-
// We check by callId to avoid re-executing the same handoff call.
693-
const executedHandoffCallIds = new Set<string>();
694-
for (const item of originalPreStepItems) {
695-
if (item instanceof RunHandoffCallItem && item.rawItem.callId) {
696-
executedHandoffCallIds.add(item.rawItem.callId);
697-
}
698-
}
699-
const pendingHandoffs = processedResponse.handoffs.filter((handoff) => {
700-
const callId = handoff.toolCall?.callId;
701-
// Only filter by callId - if callId matches, this handoff was already executed
702-
return !callId || !executedHandoffCallIds.has(callId);
703-
});
704-
705-
// If there are pending handoffs that haven't been executed yet, execute them now.
706-
// Otherwise, if there were handoffs that were already executed, we need to make sure
707-
// they don't get re-executed when we continue.
708-
if (pendingHandoffs.length > 0) {
709-
return await executeHandoffCalls(
710-
agent,
711-
originalInput,
712-
preStepItems,
713-
newItems,
714-
newResponse,
715-
pendingHandoffs,
716-
runner,
717-
state._context,
718-
);
719-
}
720-
721690
const completedStep = await maybeCompleteTurnFromToolResults({
722691
agent,
723692
runner,

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,6 +1235,101 @@ describe('Runner.run', () => {
12351235
expect(savedHostedCall.providerData).toEqual(hostedCall.providerData);
12361236
expect(savedHostedCall.id).toBe('approval-1');
12371237
});
1238+
1239+
it('prevents duplicate function_call items when resuming from interruption after tool approval', async () => {
1240+
// Regression test for issue #701
1241+
//
1242+
// Bug: When resuming a turn after approving a tool call, duplicate function_call items
1243+
// were saved to the session. The bug occurred because _currentTurnPersistedItemCount
1244+
// was incorrectly reset to 0 after resolveInterruptedTurn returned next_step_run_again,
1245+
// causing saveToSession to save all items from the beginning of the turn, including
1246+
// the already-persisted function_call item.
1247+
//
1248+
// Expected behavior: Only 1 function_call item should be saved to the session.
1249+
// Buggy behavior: 2 function_call items (duplicate) were saved.
1250+
//
1251+
// Test scenario:
1252+
// 1. Initial run with tool requiring approval creates an interruption
1253+
// 2. Tool call is approved
1254+
// 3. Run is resumed with the approved state
1255+
// 4. Session should contain exactly 1 function_call item (not 2)
1256+
1257+
const getWeatherTool = tool({
1258+
name: 'get_weather',
1259+
description: 'Get weather for a city',
1260+
parameters: z.object({ city: z.string() }),
1261+
needsApproval: async () => true, // Require approval for all calls
1262+
execute: async ({ city }) => `Sunny, 72°F in ${city}`,
1263+
});
1264+
1265+
const model = new FakeModel([
1266+
// First response: tool call that requires approval
1267+
{
1268+
output: [
1269+
{
1270+
type: 'function_call',
1271+
id: 'fc_1',
1272+
callId: 'call_weather_1',
1273+
name: 'get_weather',
1274+
status: 'completed',
1275+
arguments: JSON.stringify({ city: 'Oakland' }),
1276+
providerData: {},
1277+
} as protocol.FunctionCallItem,
1278+
],
1279+
usage: new Usage(),
1280+
},
1281+
// Second response: after approval, final answer
1282+
{
1283+
output: [fakeModelMessage('The weather is sunny in Oakland.')],
1284+
usage: new Usage(),
1285+
},
1286+
]);
1287+
1288+
const agent = new Agent({
1289+
name: 'Assistant',
1290+
instructions: 'Use get_weather tool to answer weather questions.',
1291+
model,
1292+
tools: [getWeatherTool],
1293+
toolUseBehavior: 'run_llm_again', // Must use 'run_llm_again' so resolveInterruptedTurn returns next_step_run_again
1294+
});
1295+
1296+
const session = new MemorySession();
1297+
1298+
// Use sessionInputCallback to match the scenario from issue #701
1299+
const sessionInputCallback = async (
1300+
historyItems: AgentInputItem[],
1301+
newItems: AgentInputItem[],
1302+
) => {
1303+
return [...historyItems, ...newItems];
1304+
};
1305+
1306+
// Step 1: Initial run creates an interruption for tool approval
1307+
let result = await run(
1308+
agent,
1309+
[{ role: 'user', content: "What's the weather in Oakland?" }],
1310+
{ session, sessionInputCallback },
1311+
);
1312+
1313+
// Step 2: Approve the tool call
1314+
for (const interruption of result.interruptions || []) {
1315+
result.state.approve(interruption);
1316+
}
1317+
1318+
// Step 3: Resume the run with the approved state
1319+
// Note: No sessionInputCallback on resume - this is part of the bug scenario
1320+
result = await run(agent, result.state, { session });
1321+
1322+
// Step 4: Verify only one function_call item exists in the session
1323+
const allItems = await session.getItems();
1324+
const functionCalls = allItems.filter(
1325+
(item): item is protocol.FunctionCallItem =>
1326+
item.type === 'function_call' && item.callId === 'call_weather_1',
1327+
);
1328+
1329+
// The bug would cause 2 function_call items to be saved (duplicate)
1330+
// The fix ensures only 1 function_call item is saved
1331+
expect(functionCalls).toHaveLength(1);
1332+
});
12381333
});
12391334
});
12401335

0 commit comments

Comments
 (0)