Skip to content

Commit 70ba54e

Browse files
Wzy19930507Fancy-hjyp
Wzy19930507
and
Fancy-hjyp
authored
fix(core): handle pending tool calls when maxIters reached (#1005) (#1261)
## AgentScope-Java Version io.agentscope:agentscope-parent:pom:1.0.12-SNAPSHOT ## Description Closes #1005 This PR fixes a session-state consistency bug in ReActAgent when maxIters is reached with pending tool calls. What was wrong: - If an iteration ended at maxIters while there were unresolved ToolUseBlocks, the agent could leave memory with tool_use entries but no matching tool_result. - On the next user turn, validation throw failed. ## Checklist Please check the following items before code is ready to be reviewed. - [x] Code has been formatted with `mvn spotless:apply` - [x] All tests are passing (`mvn test`) - [ ] Javadoc comments are complete and follow project conventions - [ ] Related documentation has been updated (e.g. links, examples, etc.) - [x] Code is ready for review --------- Co-authored-by: Fancy-hjyp <2594297576@qq.com>
1 parent 0f6a1db commit 70ba54e

2 files changed

Lines changed: 339 additions & 0 deletions

File tree

‎agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,28 @@ private Mono<PostActingEvent> notifyPostActingHook(
828828
protected Mono<Msg> summarizing() {
829829
log.debug("Maximum iterations reached. Generating summary...");
830830

831+
// Handle pending tool calls that were not completed before max iterations
832+
if (hasPendingToolUse()) {
833+
List<ToolUseBlock> pendingTools = extractPendingToolCalls();
834+
log.warn(
835+
"Max iterations reached with {} pending tool calls. Adding error results.",
836+
pendingTools.size());
837+
838+
for (ToolUseBlock toolUse : pendingTools) {
839+
ToolResultBlock errorResult =
840+
buildErrorToolResult(
841+
toolUse.getId(),
842+
"Tool execution cancelled because maximum iterations limit ("
843+
+ maxIters
844+
+ ") was reached");
845+
846+
Msg errorResultMsg =
847+
ToolResultMessageBuilder.buildToolResultMsg(
848+
errorResult, toolUse, getName());
849+
memory.addMessage(errorResultMsg);
850+
}
851+
}
852+
831853
List<Msg> messageList = prepareSummaryMessages();
832854
GenerateOptions generateOptions = buildGenerateOptions();
833855

‎agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentSummarizingTest.java‎

Lines changed: 317 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@
2828
import io.agentscope.core.message.Msg;
2929
import io.agentscope.core.message.MsgRole;
3030
import io.agentscope.core.message.TextBlock;
31+
import io.agentscope.core.message.ToolResultBlock;
3132
import io.agentscope.core.message.ToolUseBlock;
3233
import io.agentscope.core.model.ChatResponse;
3334
import io.agentscope.core.model.ChatUsage;
35+
import java.lang.reflect.Method;
3436
import java.time.Duration;
3537
import java.util.List;
3638
import java.util.Map;
@@ -397,4 +399,319 @@ void testSummaryAddedToMemory() {
397399
assertEquals(
398400
MsgRole.ASSISTANT, lastMessage.getRole(), "Summary message should be ASSISTANT");
399401
}
402+
403+
@Test
404+
@DisplayName("Should handle second call after maxIters with pending tool calls - Issue #1005")
405+
void testSecondCallAfterMaxItersWithPendingToolCalls() {
406+
// This test reproduces the bug reported in Issue #1005:
407+
// 1. User has multi-round conversation with tool call
408+
// 2. Tool doesn't respond (or times out), leaving pending tool calls
409+
// 3. maxIters is reached, session auto-ends
410+
// 4. User sends new message -> Should NOT throw IllegalStateException
411+
412+
InMemoryMemory memory = new InMemoryMemory();
413+
final String toolId = "call_638e428da2cf48ceb8b05762";
414+
415+
// Mock model that returns a tool call on first call, then summary
416+
final int[] callCount = {0};
417+
MockModel mockModel =
418+
new MockModel(
419+
messages -> {
420+
int callNum = callCount[0]++;
421+
if (callNum == 0) {
422+
// First call: return tool use block (simulating tool call)
423+
return List.of(
424+
ChatResponse.builder()
425+
.id("msg_0")
426+
.content(
427+
List.of(
428+
ToolUseBlock.builder()
429+
.name("search_tool")
430+
.id(toolId)
431+
.input(
432+
Map.of(
433+
"query",
434+
"test"))
435+
.build()))
436+
.usage(new ChatUsage(10, 20, 30))
437+
.build());
438+
} else {
439+
// Second call: summarizing (because maxIters=1 reached)
440+
return List.of(
441+
ChatResponse.builder()
442+
.id("msg_summary")
443+
.content(
444+
List.of(
445+
TextBlock.builder()
446+
.text(
447+
"I reached the"
448+
+ " maximum"
449+
+ " iteration"
450+
+ " limit."
451+
+ " Please try"
452+
+ " again.")
453+
.build()))
454+
.usage(new ChatUsage(10, 20, 30))
455+
.build());
456+
}
457+
});
458+
459+
MockToolkit mockToolkit = new MockToolkit();
460+
461+
// Create agent with maxIters=1 to quickly trigger summarizing
462+
ReActAgent agent =
463+
ReActAgent.builder()
464+
.name("TestAgent")
465+
.sysPrompt("You are a helpful assistant.")
466+
.model(mockModel)
467+
.toolkit(mockToolkit)
468+
.memory(memory)
469+
.maxIters(1)
470+
.build();
471+
472+
// First user message - triggers tool call and maxIters summarizing
473+
Msg firstUserMsg = TestUtils.createUserMessage("User", "Please search for something");
474+
Msg firstResponse =
475+
agent.call(firstUserMsg)
476+
.block(Duration.ofMillis(TestConstants.DEFAULT_TEST_TIMEOUT_MS));
477+
478+
// Verify first response
479+
assertNotNull(firstResponse, "First response should not be null");
480+
assertEquals(MsgRole.ASSISTANT, firstResponse.getRole());
481+
482+
// CRITICAL: Verify that the pending tool call has been resolved in memory
483+
// Before the fix, memory would have pending tool calls without results
484+
// After the fix, summarizing() should add error results for pending tools
485+
List<Msg> memoryMessages = memory.getMessages();
486+
487+
// Find if there's a tool result message for the pending tool
488+
boolean hasToolResultForPendingTool =
489+
memoryMessages.stream()
490+
.flatMap(m -> m.getContentBlocks(ToolResultBlock.class).stream())
491+
.anyMatch(tr -> tr.getId() != null && tr.getId().equals(toolId));
492+
493+
assertTrue(
494+
hasToolResultForPendingTool,
495+
"Memory should contain error result for pending tool call after summarizing");
496+
497+
Msg pendingToolResultMsg =
498+
memoryMessages.stream()
499+
.filter(
500+
m ->
501+
m.getContentBlocks(ToolResultBlock.class).stream()
502+
.anyMatch(
503+
tr ->
504+
tr.getId() != null
505+
&& tr.getId()
506+
.equals(toolId)))
507+
.findFirst()
508+
.orElse(null);
509+
assertNotNull(pendingToolResultMsg, "Pending tool call should have a result message");
510+
assertEquals(
511+
MsgRole.TOOL,
512+
pendingToolResultMsg.getRole(),
513+
"Pending tool error result should be stored as TOOL message");
514+
515+
// Verify the tool result indicates cancellation due to max iterations
516+
ToolResultBlock toolResult =
517+
memoryMessages.stream()
518+
.flatMap(m -> m.getContentBlocks(ToolResultBlock.class).stream())
519+
.filter(tr -> tr.getId() != null && tr.getId().equals(toolId))
520+
.findFirst()
521+
.orElse(null);
522+
523+
// Tool result should be present (either from toolkit or from summarizing fix)
524+
assertNotNull(toolResult);
525+
526+
// SECOND CALL - This is the critical test for Issue #1005
527+
// Before the fix, this would throw:
528+
// IllegalStateException: Cannot add messages without tool results when pending tool calls
529+
// exist
530+
531+
// Reset model for second user interaction
532+
final int[] secondCallCount = {0};
533+
MockModel secondMockModel =
534+
new MockModel(
535+
messages -> {
536+
int callNum = secondCallCount[0]++;
537+
if (callNum == 0) {
538+
return List.of(
539+
ChatResponse.builder()
540+
.id("msg_second_0")
541+
.content(
542+
List.of(
543+
TextBlock.builder()
544+
.text(
545+
"Hello! How can I"
546+
+ " help you"
547+
+ " today?")
548+
.build()))
549+
.usage(new ChatUsage(5, 10, 15))
550+
.build());
551+
}
552+
return List.of();
553+
});
554+
555+
ReActAgent secondAgent =
556+
ReActAgent.builder()
557+
.name("TestAgent")
558+
.sysPrompt("You are a helpful assistant.")
559+
.model(secondMockModel)
560+
.toolkit(mockToolkit)
561+
.memory(memory) // Same memory
562+
.maxIters(2)
563+
.build();
564+
565+
// Second user message - this would throw IllegalStateException before the fix
566+
Msg secondUserMsg = TestUtils.createUserMessage("User", "Hello again");
567+
568+
// This should NOT throw: "Cannot add messages without tool results when pending tool calls
569+
// exist"
570+
Msg secondResponse =
571+
secondAgent
572+
.call(secondUserMsg)
573+
.block(Duration.ofMillis(TestConstants.DEFAULT_TEST_TIMEOUT_MS));
574+
575+
// Verify second response succeeded
576+
assertNotNull(secondResponse, "Second response should not be null");
577+
assertEquals(MsgRole.ASSISTANT, secondResponse.getRole());
578+
assertTrue(
579+
secondResponse.getFirstContentBlock() instanceof TextBlock,
580+
"Second response should contain TextBlock");
581+
582+
TextBlock secondText = (TextBlock) secondResponse.getFirstContentBlock();
583+
assertEquals("Hello! How can I help you today?", secondText.getText());
584+
}
585+
586+
@Test
587+
@DisplayName("Should add exactly one TOOL result for each pending tool during summarizing")
588+
void testSummarizingAddsOneToolResultPerPendingTool() {
589+
InMemoryMemory memory = new InMemoryMemory();
590+
final String toolId1 = "call_pending_1";
591+
final String toolId2 = "call_pending_2";
592+
593+
Msg pendingAssistantMsg =
594+
Msg.builder()
595+
.name("TestAgent")
596+
.role(MsgRole.ASSISTANT)
597+
.content(
598+
List.of(
599+
ToolUseBlock.builder()
600+
.name("search_tool")
601+
.id(toolId1)
602+
.input(Map.of("query", "weather"))
603+
.build(),
604+
ToolUseBlock.builder()
605+
.name("search_tool")
606+
.id(toolId2)
607+
.input(Map.of("query", "news"))
608+
.build()))
609+
.build();
610+
memory.addMessage(pendingAssistantMsg);
611+
612+
MockModel mockModel =
613+
new MockModel(
614+
messages ->
615+
List.of(
616+
ChatResponse.builder()
617+
.id("msg_summary")
618+
.content(
619+
List.of(
620+
TextBlock.builder()
621+
.text(
622+
"Iteration limit"
623+
+ " reached.")
624+
.build()))
625+
.usage(new ChatUsage(10, 20, 30))
626+
.build()));
627+
628+
MockToolkit mockToolkit = new MockToolkit();
629+
ReActAgent agent =
630+
ReActAgent.builder()
631+
.name("TestAgent")
632+
.sysPrompt("You are a helpful assistant.")
633+
.model(mockModel)
634+
.toolkit(mockToolkit)
635+
.memory(memory)
636+
.maxIters(1)
637+
.build();
638+
639+
Msg summaryResponse = invokeSummarizing(agent);
640+
assertNotNull(summaryResponse, "Summary response should not be null");
641+
642+
List<Msg> memoryMessages = memory.getMessages();
643+
644+
long toolId1ToolRoleCount =
645+
memoryMessages.stream()
646+
.filter(m -> m.getRole() == MsgRole.TOOL)
647+
.filter(
648+
m ->
649+
m.getContentBlocks(ToolResultBlock.class).stream()
650+
.anyMatch(
651+
tr ->
652+
tr.getId() != null
653+
&& tr.getId()
654+
.equals(toolId1)))
655+
.count();
656+
long toolId2ToolRoleCount =
657+
memoryMessages.stream()
658+
.filter(m -> m.getRole() == MsgRole.TOOL)
659+
.filter(
660+
m ->
661+
m.getContentBlocks(ToolResultBlock.class).stream()
662+
.anyMatch(
663+
tr ->
664+
tr.getId() != null
665+
&& tr.getId()
666+
.equals(toolId2)))
667+
.count();
668+
669+
assertEquals(
670+
1L, toolId1ToolRoleCount, "toolId1 should have exactly one TOOL result message");
671+
assertEquals(
672+
1L, toolId2ToolRoleCount, "toolId2 should have exactly one TOOL result message");
673+
674+
long toolId1NonToolRoleCount =
675+
memoryMessages.stream()
676+
.filter(m -> m.getRole() != MsgRole.TOOL)
677+
.filter(
678+
m ->
679+
m.getContentBlocks(ToolResultBlock.class).stream()
680+
.anyMatch(
681+
tr ->
682+
tr.getId() != null
683+
&& tr.getId()
684+
.equals(toolId1)))
685+
.count();
686+
long toolId2NonToolRoleCount =
687+
memoryMessages.stream()
688+
.filter(m -> m.getRole() != MsgRole.TOOL)
689+
.filter(
690+
m ->
691+
m.getContentBlocks(ToolResultBlock.class).stream()
692+
.anyMatch(
693+
tr ->
694+
tr.getId() != null
695+
&& tr.getId()
696+
.equals(toolId2)))
697+
.count();
698+
699+
assertEquals(
700+
0L, toolId1NonToolRoleCount, "toolId1 should not have non-TOOL result messages");
701+
assertEquals(
702+
0L, toolId2NonToolRoleCount, "toolId2 should not have non-TOOL result messages");
703+
}
704+
705+
private static Msg invokeSummarizing(ReActAgent agent) {
706+
try {
707+
Method method = ReActAgent.class.getDeclaredMethod("summarizing");
708+
method.setAccessible(true);
709+
@SuppressWarnings("unchecked")
710+
reactor.core.publisher.Mono<Msg> mono =
711+
(reactor.core.publisher.Mono<Msg>) method.invoke(agent);
712+
return mono.block(Duration.ofMillis(TestConstants.DEFAULT_TEST_TIMEOUT_MS));
713+
} catch (Exception e) {
714+
throw new RuntimeException("Failed to invoke summarizing()", e);
715+
}
716+
}
400717
}

0 commit comments

Comments
 (0)