diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java index dc61ad9178..b41fdce1e2 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java @@ -383,7 +383,7 @@ public static class SkillRegistration { private Toolkit toolkit; private AgentSkill skill; private Object toolObject; - private AgentTool agentTool; + private final List agentTools = new ArrayList<>(); private McpClientWrapper mcpClientWrapper; private SubAgentProvider subAgentProvider; private SubAgentConfig subAgentConfig; @@ -424,13 +424,15 @@ public SkillRegistration tool(Object toolObject) { } /** - * Set the AgentTool instance to register. + * Add an AgentTool instance to register. May be called multiple times to bind several + * tools to the same skill — every tool is bound into the skill's gated tool group + * (previously each call overwrote the previous one, so only the last tool was bound). * * @param agentTool The AgentTool instance * @return This builder for chaining */ public SkillRegistration agentTool(AgentTool agentTool) { - this.agentTool = agentTool; + this.agentTools.add(agentTool); return this; } @@ -511,7 +513,7 @@ public SkillRegistration subAgent(SubAgentProvider provider) { */ public SkillRegistration subAgent(SubAgentProvider provider, SubAgentConfig config) { if (this.toolObject != null - || this.agentTool != null + || !this.agentTools.isEmpty() || this.mcpClientWrapper != null) { throw new IllegalStateException( "Cannot set multiple registration types. Use only one of: tool()," @@ -593,7 +595,7 @@ public void apply() { skillBox.registerSkill(skill); if (toolObject != null - || agentTool != null + || !agentTools.isEmpty() || mcpClientWrapper != null || subAgentProvider != null) { if (toolkit == null && (toolkit = skillBox.toolkit) == null) { @@ -604,17 +606,48 @@ public void apply() { if (toolkit.getToolGroup(skillToolGroup) == null) { toolkit.createToolGroup(skillToolGroup, skillToolGroup, false); } - toolkit.registration() - .group(skillToolGroup) - .presetParameters(presetParameters) - .extendedModel(extendedModel) - .enableTools(enableTools) - .disableTools(disableTools) - .agentTool(agentTool) - .tool(toolObject) - .mcpClient(mcpClientWrapper) - .subAgent(subAgentProvider, subAgentConfig) - .apply(); + // Toolkit.ToolRegistration 每次只能注册一个工具（exactly-one 校验）， + // 多 tool 的 skill 必须逐个独立注册，否则只有最后一个生效 + for (AgentTool tool : agentTools) { + toolkit.registration() + .group(skillToolGroup) + .presetParameters(presetParameters) + .extendedModel(extendedModel) + .enableTools(enableTools) + .disableTools(disableTools) + .agentTool(tool) + .apply(); + } + if (toolObject != null) { + toolkit.registration() + .group(skillToolGroup) + .presetParameters(presetParameters) + .extendedModel(extendedModel) + .enableTools(enableTools) + .disableTools(disableTools) + .tool(toolObject) + .apply(); + } + if (mcpClientWrapper != null) { + toolkit.registration() + .group(skillToolGroup) + .presetParameters(presetParameters) + .extendedModel(extendedModel) + .enableTools(enableTools) + .disableTools(disableTools) + .mcpClient(mcpClientWrapper) + .apply(); + } + if (subAgentProvider != null) { + toolkit.registration() + .group(skillToolGroup) + .presetParameters(presetParameters) + .extendedModel(extendedModel) + .enableTools(enableTools) + .disableTools(disableTools) + .subAgent(subAgentProvider, subAgentConfig) + .apply(); + } } } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxTest.java index a2cee901c7..5efc59dcff 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxTest.java @@ -187,6 +187,35 @@ void testSuccessfullyRegisterWhenOnlyAgentToolProvided() { assertNotNull(toolkit.getTool("agent_tool_only"), "Agent tool should be registered"); } + @Test + @DisplayName("Should bind ALL agent tools when multiple are registered on one skill") + void testMultipleAgentToolsAllBoundToSkillGroup() { + AgentTool first = createTestTool("multi_tool_first"); + AgentTool second = createTestTool("multi_tool_second"); + AgentSkill skill = + new AgentSkill( + "Multi Tool Skill", "Skill with two agent tools", "# Multi", null); + + // 连续两次 agentTool()：此前第二次会覆盖第一次，导致只有 second 被绑进门控组 + skillBox.registration().skill(skill).agentTool(first).agentTool(second).apply(); + + String groupName = skill.getSkillId() + "_skill_tools"; + assertNotNull(toolkit.getToolGroup(groupName), "skill tool group should exist"); + assertTrue( + toolkit.getToolGroup(groupName).getTools().contains("multi_tool_first"), + "first tool must also be bound into the skill group (regression: was" + + " overwritten)"); + assertTrue( + toolkit.getToolGroup(groupName).getTools().contains("multi_tool_second"), + "second tool must be bound into the skill group"); + assertFalse( + toolkit.getToolGroup(groupName).isActive(), + "skill tool group must start inactive (gated until skill is loaded)"); + + assertNotNull(toolkit.getTool("multi_tool_first")); + assertNotNull(toolkit.getTool("multi_tool_second")); + } + @Test @DisplayName("Should successfully register when only mcp client is provided") void testSuccessfullyRegisterWhenOnlyMcpClientProvided() { diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/compaction/ConversationCompactor.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/compaction/ConversationCompactor.java index 1bb6380abe..0b759f82b4 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/compaction/ConversationCompactor.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/compaction/ConversationCompactor.java @@ -115,9 +115,11 @@ public Mono>> compactIfNeeded( return Mono.just(Optional.empty()); } - // Filter previous summary messages from the prefix before offloading to avoid - // re-storing already-archived summaries. - List prefix = filterSummaryMessages(new ArrayList<>(messages.subList(0, cutoff))); + // 摘要输入必须保留上轮摘要（链式摘要的意图链不能断，#2359）； + // 过滤版仅用于 flush/offload 去重（filterSummaryMessages 的 javadoc 原意）， + // 此前过滤版同时喂了摘要输入 → 连续压缩丢原始意图。 + List prefix = new ArrayList<>(messages.subList(0, cutoff)); + List prefixForFlush = filterSummaryMessages(prefix); List tail = new ArrayList<>(messages.subList(cutoff, messages.size())); log.info( @@ -131,7 +133,7 @@ public Mono>> compactIfNeeded( Mono flushStep = config.isFlushBeforeCompact() ? flushManager - .flushMemories(rc, prefix) + .flushMemories(rc, prefixForFlush) .doOnSuccess(v -> log.debug("Memory flush before compaction done")) .onErrorResume( e -> { diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/memory/compaction/ConversationCompactorChainedSummaryTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/memory/compaction/ConversationCompactorChainedSummaryTest.java new file mode 100644 index 0000000000..99ecf0f85d --- /dev/null +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/memory/compaction/ConversationCompactorChainedSummaryTest.java @@ -0,0 +1,164 @@ +package io.agentscope.harness.agent.memory.compaction; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import io.agentscope.core.message.Msg; +import io.agentscope.core.message.MsgRole; +import io.agentscope.core.message.TextBlock; +import io.agentscope.core.model.ChatModelBase; +import io.agentscope.core.model.ChatResponse; +import io.agentscope.core.model.GenerateOptions; +import io.agentscope.core.model.ToolSchema; +import io.agentscope.harness.agent.memory.MemoryFlushManager; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Flux; + +/** + * Regression tests for #2359: consecutive compactions must keep the previous summary in + * the summarization input (chained intent), instead of filtering it out — which used to + * decay the original user intent round over round (and degenerate to + * "No previous conversation history." when the prefix was all summaries). + */ +class ConversationCompactorChainedSummaryTest { + + private static final String PRIOR_INTENT = "原始意图：诊断生产 OOM 并给出根因"; + + /** Captures the text the model is asked to summarize, returns a fixed summary. */ + private static final class CapturingModel extends ChatModelBase { + final AtomicReference seenPrompt = new AtomicReference<>(""); + + @Override + public String getModelName() { + return "capturing-summary-model"; + } + + @Override + protected Flux doStream( + List messages, List tools, GenerateOptions options) { + StringBuilder sb = new StringBuilder(); + for (Msg m : messages) { + String t = m.getTextContent(); + if (t != null) sb.append(t); + } + seenPrompt.set(sb.toString()); + return Flux.just( + ChatResponse.builder() + .content( + List.of( + TextBlock.builder().text("压缩后的新摘要").build())) + .build()); + } + } + + private static Msg userMsg(String text) { + return Msg.builder() + .role(MsgRole.USER) + .content(TextBlock.builder().text(text).build()) + .build(); + } + + private static Msg assistantMsg(String text) { + return Msg.builder() + .role(MsgRole.ASSISTANT) + .content(TextBlock.builder().text(text).build()) + .build(); + } + + private static Msg priorSummaryMsg() { + return Msg.builder() + .role(MsgRole.USER) + .name(ConversationCompactor.SUMMARY_MSG_NAME) + .content( + TextBlock.builder() + .text( + "Here is a summary of the conversation to date:\n\n" + + PRIOR_INTENT) + .build()) + .build(); + } + + @Test + @DisplayName("#2359: previous summary is included in the next round's summarization input") + void chainedCompaction_keepsPriorSummaryInSummarizationInput() { + CapturingModel model = new CapturingModel(); + ConversationCompactor compactor = + new ConversationCompactor(model, mock(MemoryFlushManager.class)); + + List messages = new ArrayList<>(); + messages.add(priorSummaryMsg()); // 上一轮压缩的摘要（携带原始意图） + for (int i = 0; i < 6; i++) { + messages.add(userMsg("新的问题 " + i + "：继续排查")); + messages.add(assistantMsg("新的回答 " + i)); + } + + CompactionConfig config = + CompactionConfig.builder() + .triggerMessages(4) + .keepMessages(2) + .flushBeforeCompact(false) + .offloadBeforeCompact(false) + .build(); + + Optional> result = + compactor.compactIfNeeded(null, messages, config, "agent", "session").block(); + + assertTrue(result.isPresent(), "compaction should trigger"); + + String prompt = model.seenPrompt.get(); + assertTrue( + prompt.contains(PRIOR_INTENT), + "#2359 regression: prior summary must be part of the summarization input, got: " + + prompt); + assertFalse( + prompt.contains("No previous conversation history"), + "summary must not degenerate when a prior summary exists"); + } + + @Test + @DisplayName("#2359: prefix made only of a prior summary still produces a real summary") + void prefixOfOnlyPriorSummary_doesNotDegenerate() { + CapturingModel model = new CapturingModel(); + ConversationCompactor compactor = + new ConversationCompactor(model, mock(MemoryFlushManager.class)); + + List messages = new ArrayList<>(); + messages.add(priorSummaryMsg()); + messages.add(userMsg("补充一个问题")); + messages.add(assistantMsg("补充回答")); + messages.add(userMsg("再补充一个")); + messages.add(assistantMsg("再补充回答")); + + CompactionConfig config = + CompactionConfig.builder() + .triggerMessages(3) + .keepMessages(2) + .flushBeforeCompact(false) + .offloadBeforeCompact(false) + .build(); + + Optional> result = + compactor.compactIfNeeded(null, messages, config, "agent", "session").block(); + + assertTrue(result.isPresent()); + String prompt = model.seenPrompt.get(); + assertTrue( + prompt.contains(PRIOR_INTENT), + "prior summary must remain the summarization baseline, got: " + prompt); + assertFalse( + result.get().stream() + .anyMatch( + m -> { + String t = m.getTextContent(); + return t != null + && t.contains("No previous conversation history"); + }), + "must not emit the degenerate 'No previous conversation history' summary"); + } +}