Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ public static class SkillRegistration {
private Toolkit toolkit;
private AgentSkill skill;
private Object toolObject;
private AgentTool agentTool;
private final List<AgentTool> agentTools = new ArrayList<>();
private McpClientWrapper mcpClientWrapper;
Comment on lines 383 to 387
private SubAgentProvider<?> subAgentProvider;
private SubAgentConfig subAgentConfig;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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(),"
Expand Down Expand Up @@ -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) {
Expand All @@ -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 必须逐个独立注册,否则只有最后一个生效
Comment on lines +609 to +610
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();
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment on lines +199 to +200

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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,11 @@ public Mono<Optional<List<Msg>>> compactIfNeeded(
return Mono.just(Optional.empty());
}

// Filter previous summary messages from the prefix before offloading to avoid
// re-storing already-archived summaries.
List<Msg> prefix = filterSummaryMessages(new ArrayList<>(messages.subList(0, cutoff)));
// 摘要输入必须保留上轮摘要(链式摘要的意图链不能断,#2359);
// 过滤版仅用于 flush/offload 去重(filterSummaryMessages 的 javadoc 原意),
// 此前过滤版同时喂了摘要输入 → 连续压缩丢原始意图。
List<Msg> prefix = new ArrayList<>(messages.subList(0, cutoff));
List<Msg> prefixForFlush = filterSummaryMessages(prefix);
Comment on lines +118 to +122
List<Msg> tail = new ArrayList<>(messages.subList(cutoff, messages.size()));

log.info(
Expand All @@ -131,7 +133,7 @@ public Mono<Optional<List<Msg>>> compactIfNeeded(
Mono<Void> flushStep =
config.isFlushBeforeCompact()
? flushManager
.flushMemories(rc, prefix)
.flushMemories(rc, prefixForFlush)
.doOnSuccess(v -> log.debug("Memory flush before compaction done"))
.onErrorResume(
e -> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String> seenPrompt = new AtomicReference<>("");

@Override
public String getModelName() {
return "capturing-summary-model";
}

@Override
protected Flux<ChatResponse> doStream(
List<Msg> messages, List<ToolSchema> 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.<io.agentscope.core.message.ContentBlock>of(
TextBlock.builder().text("压缩后的新摘要").build()))
.build());
Comment on lines +51 to +56
}
}

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<Msg> 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<List<Msg>> 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<Msg> 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<List<Msg>> 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");
}
}
Loading