From 900681a8f9e5ba476fe541e46c7b838e8fd3a1c0 Mon Sep 17 00:00:00 2001 From: xun Date: Fri, 27 Feb 2026 19:00:18 +0800 Subject: [PATCH 1/5] skill.md support model --- .../core/memory/InMemoryMemory.java | 19 + .../io/agentscope/core/memory/Memory.java | 13 + .../io/agentscope/core/skill/AgentSkill.java | 124 +++- .../skill/MapBasedSkillModelProvider.java | 188 ++++++ .../io/agentscope/core/skill/SkillBox.java | 358 ++++++++++- .../core/skill/SkillModelProvider.java | 50 ++ .../skill/SkillSubagentPromptBuilder.java | 149 +++++ .../core/skill/SkillToolFactory.java | 219 ++++++- .../agentscope/core/skill/util/SkillUtil.java | 4 +- .../tool/subagent/ContextSharingMode.java | 64 ++ .../core/tool/subagent/SubAgentConfig.java | 69 ++- .../core/tool/subagent/SubAgentContext.java | 116 ++++ .../core/tool/subagent/SubAgentProvider.java | 57 +- .../core/tool/subagent/SubAgentTool.java | 285 ++++++++- .../agentscope/core/skill/AgentSkillTest.java | 44 ++ .../skill/MapBasedSkillModelProviderTest.java | 263 +++++++++ .../core/skill/SkillBoxRegistrationTest.java | 189 ++++++ .../agentscope/core/skill/SkillBoxTest.java | 39 ++ .../core/skill/SkillModelProviderTest.java | 66 +++ .../skill/SkillSubagentPromptBuilderTest.java | 258 ++++++++ .../core/skill/SkillToolFactoryTest.java | 220 +++++++ .../agentscope/core/skill/SkillUtilTest.java | 34 ++ .../io/agentscope/core/tool/ToolkitTest.java | 10 +- .../core/tool/subagent/SubAgentToolTest.java | 555 +++++++++++++++++- agentscope-dependencies-bom/pom.xml | 2 +- .../agentscope-bom/pom.xml | 2 +- .../memory/autocontext/AutoContextMemory.java | 41 ++ pom.xml | 2 +- 28 files changed, 3374 insertions(+), 66 deletions(-) create mode 100644 agentscope-core/src/main/java/io/agentscope/core/skill/MapBasedSkillModelProvider.java create mode 100644 agentscope-core/src/main/java/io/agentscope/core/skill/SkillModelProvider.java create mode 100644 agentscope-core/src/main/java/io/agentscope/core/skill/SkillSubagentPromptBuilder.java create mode 100644 agentscope-core/src/main/java/io/agentscope/core/tool/subagent/ContextSharingMode.java create mode 100644 agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentContext.java create mode 100644 agentscope-core/src/test/java/io/agentscope/core/skill/MapBasedSkillModelProviderTest.java create mode 100644 agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java create mode 100644 agentscope-core/src/test/java/io/agentscope/core/skill/SkillModelProviderTest.java create mode 100644 agentscope-core/src/test/java/io/agentscope/core/skill/SkillSubagentPromptBuilderTest.java create mode 100644 agentscope-core/src/test/java/io/agentscope/core/skill/SkillToolFactoryTest.java diff --git a/agentscope-core/src/main/java/io/agentscope/core/memory/InMemoryMemory.java b/agentscope-core/src/main/java/io/agentscope/core/memory/InMemoryMemory.java index 869828408..982ca4773 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/memory/InMemoryMemory.java +++ b/agentscope-core/src/main/java/io/agentscope/core/memory/InMemoryMemory.java @@ -124,4 +124,23 @@ public void deleteMessage(int index) { public void clear() { messages.clear(); } + + /** + * Creates a fork (copy) of this memory. + * + *

The fork contains a copy of all messages at the time of invocation. Changes to the fork + * do not affect the original memory, and vice versa. + * + * @return A new InMemoryMemory instance containing copies of all messages + */ + @Override + public Memory fork() { + InMemoryMemory forked = new InMemoryMemory(); + for (Msg msg : messages) { + if (msg != null) { + forked.messages.add(msg); + } + } + return forked; + } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/memory/Memory.java b/agentscope-core/src/main/java/io/agentscope/core/memory/Memory.java index 4b223fedf..62974c92f 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/memory/Memory.java +++ b/agentscope-core/src/main/java/io/agentscope/core/memory/Memory.java @@ -59,4 +59,17 @@ public interface Memory extends StateModule { * is typically irreversible unless state has been persisted. */ void clear(); + + /** + * Creates a fork (copy) of this memory. + * + *

The fork contains a copy of all messages at the time of invocation. Changes to the fork + * do not affect the original memory, and vice versa. + * + *

This is useful when you want to provide context to a sub-agent without allowing it to + * modify the parent's memory. + * + * @return A new Memory instance containing copies of all messages + */ + Memory fork(); } diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkill.java b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkill.java index ee3027b34..02a82e584 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkill.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkill.java @@ -67,6 +67,8 @@ public class AgentSkill { private final String skillContent; private final Map resources; private final String source; + private final String model; + private final String context; /** * Creates an AgentSkill with explicit parameters. @@ -104,6 +106,59 @@ public AgentSkill( String skillContent, Map resources, String source) { + this(name, description, skillContent, resources, source, null); + } + + /** + * Creates an AgentSkill with explicit parameters, custom source, and model. + * + *

Use this constructor when you want to create a skill directly without parsing + * markdown. The source parameter indicates where the skill originated from. + * The model parameter specifies which AI model should be used when this skill is active. + * + * @param name Skill name (must not be null or empty) + * @param description Skill description (must not be null or empty) + * @param skillContent The skill implementation or instructions (must not be null or empty) + * @param resources Supporting resources referenced by the skill (can be null) + * @param source Source identifier for the skill (null defaults to "custom") + * @param model Model reference for the skill (can be null) + * @throws IllegalArgumentException if name, description, or skillContent is null or empty + */ + public AgentSkill( + String name, + String description, + String skillContent, + Map resources, + String source, + String model) { + this(name, description, skillContent, resources, source, model, null); + } + + /** + * Creates an AgentSkill with explicit parameters, custom source, model, and context. + * + *

Use this constructor when you want to create a skill directly without parsing markdown. + * The source parameter indicates where the skill originated from. The model parameter + * specifies which AI model should be used when this skill is active. The context parameter + * specifies how memory should be shared with the sub-agent. + * + * @param name Skill name (must not be null or empty) + * @param description Skill description (must not be null or empty) + * @param skillContent The skill implementation or instructions (must not be null or empty) + * @param resources Supporting resources referenced by the skill (can be null) + * @param source Source identifier for the skill (null defaults to "custom") + * @param model Model reference for the skill (can be null) + * @param context Context sharing mode: "shared" (default), "fork", or "new" (can be null) + * @throws IllegalArgumentException if name, description, or skillContent is null or empty + */ + public AgentSkill( + String name, + String description, + String skillContent, + Map resources, + String source, + String model, + String context) { if (name == null || name.isEmpty() || description == null || description.isEmpty()) { throw new IllegalArgumentException( "The skill must have `name` and `description` fields."); @@ -117,6 +172,8 @@ public AgentSkill( this.skillContent = skillContent; this.resources = resources != null ? new HashMap<>(resources) : new HashMap<>(); this.source = source != null ? source : "custom"; + this.model = model; + this.context = context; } /** @@ -157,6 +214,36 @@ public String getSource() { return source; } + /** + * Gets the model reference for this skill. + * + *

The model specifies which AI model should be used when this skill is active. For example: + * "haiku", "sonnet", "openai:gpt-4o". + * + * @return The model reference, or null if no specific model is required + */ + public String getModel() { + return model; + } + + /** + * Gets the context sharing mode for this skill. + * + *

The context specifies how memory should be shared between the parent agent and the + * sub-agent: + * + *

+ * + * @return The context mode string, or null if default (shared) + */ + public String getContext() { + return context; + } + /** * Gets the skill resources. * @@ -261,6 +348,8 @@ public static class Builder { private String skillContent; private Map resources; private String source; + private String model; + private String context; /** * Creates an empty builder. @@ -280,6 +369,8 @@ private Builder(AgentSkill baseSkill) { this.skillContent = baseSkill.skillContent; this.resources = new HashMap<>(baseSkill.resources); this.source = baseSkill.source; + this.model = baseSkill.model; + this.context = baseSkill.context; } /** @@ -370,6 +461,36 @@ public Builder source(String source) { return this; } + /** + * Sets the model reference. + * + * @param model The model reference (e.g., "haiku", "openai:gpt-4o") + * @return This builder + */ + public Builder model(String model) { + this.model = model; + return this; + } + + /** + * Sets the context sharing mode. + * + *

The context specifies how memory should be shared: + * + *

+ * + * @param context The context mode string + * @return This builder + */ + public Builder context(String context) { + this.context = context; + return this; + } + /** * Builds the AgentSkill instance. * @@ -377,7 +498,8 @@ public Builder source(String source) { * @throws IllegalArgumentException if required fields are missing */ public AgentSkill build() { - return new AgentSkill(name, description, skillContent, resources, source); + return new AgentSkill( + name, description, skillContent, resources, source, model, context); } } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/MapBasedSkillModelProvider.java b/agentscope-core/src/main/java/io/agentscope/core/skill/MapBasedSkillModelProvider.java new file mode 100644 index 000000000..427f0f9e3 --- /dev/null +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/MapBasedSkillModelProvider.java @@ -0,0 +1,188 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.skill; + +import io.agentscope.core.model.Model; +import java.util.HashMap; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A Map-based implementation of {@link SkillModelProvider} that resolves model references using + * direct name lookup and alias mapping. + * + *

This provider supports multiple model reference formats: + * + *

+ * + *

Example usage: + * + *

{@code
+ * MapBasedSkillModelProvider provider = MapBasedSkillModelProvider.builder()
+ *     .register("qwen-turbo", qwenModel)
+ *     .register("dashscope:qwen-plus", qwenPlusModel)
+ *     .alias("qwen-turbo", "qwen-turbo")
+ *     .alias("qwen-plus", "dashscope:qwen-plus")
+ *     .defaultModel(defaultModel)
+ *     .build();
+ *
+ * Model model = provider.getModel("qwen-turbo");  // Returns qwenModel
+ * }
+ */ +public class MapBasedSkillModelProvider implements SkillModelProvider { + + private static final Logger log = LoggerFactory.getLogger(MapBasedSkillModelProvider.class); + + private final Map modelsByName; + private final Map aliasMapping; + private final Model defaultModel; + + private MapBasedSkillModelProvider(Builder builder) { + this.modelsByName = new HashMap<>(builder.modelsByName); + this.aliasMapping = new HashMap<>(builder.aliasMapping); + this.defaultModel = builder.defaultModel; + } + + /** + * Creates a new builder for constructing MapBasedSkillModelProvider instances. + * + * @return a new Builder instance + */ + public static Builder builder() { + return new Builder(); + } + + @Override + public Model getModel(String modelRef) { + // Handle null or blank input + if (modelRef == null || modelRef.isBlank()) { + log.debug("Model reference is null or blank, returning default model"); + return defaultModel; + } + + // 1. Try direct lookup first + Model directMatch = modelsByName.get(modelRef); + if (directMatch != null) { + log.debug("Found model by direct name: {}", modelRef); + return directMatch; + } + + // 2. Try alias lookup + String aliasedName = aliasMapping.get(modelRef); + if (aliasedName != null) { + Model aliasedModel = modelsByName.get(aliasedName); + if (aliasedModel != null) { + log.debug("Resolved alias '{}' to model: {}", modelRef, aliasedName); + return aliasedModel; + } + } + + // 3. Return default model if available + if (defaultModel != null) { + log.debug("Model '{}' not found, returning default model", modelRef); + return defaultModel; + } + + // 4. No model found + log.debug("Model '{}' not found and no default model configured", modelRef); + return null; + } + + /** Builder for constructing MapBasedSkillModelProvider instances. */ + public static class Builder { + private final Map modelsByName = new HashMap<>(); + private final Map aliasMapping = new HashMap<>(); + private Model defaultModel; + + private Builder() {} + + /** + * Sets the default model to use when a requested model is not found. + * + * @param defaultModel the default model + * @return this builder + */ + public Builder defaultModel(Model defaultModel) { + this.defaultModel = defaultModel; + return this; + } + + /** + * Registers a model with the given name. + * + * @param name the name to register the model under + * @param model the model instance + * @return this builder + * @throws IllegalArgumentException if name is null or blank + */ + public Builder register(String name, Model model) { + if (name == null || name.isBlank()) { + throw new IllegalArgumentException("Model name cannot be null or blank"); + } + if (model == null) { + throw new IllegalArgumentException("Model cannot be null"); + } + this.modelsByName.put(name, model); + return this; + } + + /** + * Registers all models from the given map. + * + * @param models map of model names to model instances + * @return this builder + */ + public Builder registerAll(Map models) { + if (models != null) { + this.modelsByName.putAll(models); + } + return this; + } + + /** + * Creates an alias from one name to another. + * + * @param alias the alias name + * @param targetName the target model name that the alias points to + * @return this builder + * @throws IllegalArgumentException if alias or targetName is null or blank + */ + public Builder alias(String alias, String targetName) { + if (alias == null || alias.isBlank()) { + throw new IllegalArgumentException("Alias cannot be null or blank"); + } + if (targetName == null || targetName.isBlank()) { + throw new IllegalArgumentException("Target name cannot be null or blank"); + } + this.aliasMapping.put(alias, targetName); + return this; + } + + /** + * Builds the MapBasedSkillModelProvider instance. + * + * @return a new MapBasedSkillModelProvider instance + */ + public MapBasedSkillModelProvider build() { + return new MapBasedSkillModelProvider(this); + } + } +} 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 9eaabbb16..833200fb8 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 @@ -15,6 +15,10 @@ */ package io.agentscope.core.skill; +import io.agentscope.core.ReActAgent; +import io.agentscope.core.memory.InMemoryMemory; +import io.agentscope.core.memory.Memory; +import io.agentscope.core.model.Model; import io.agentscope.core.skill.util.SkillFileSystemHelper; import io.agentscope.core.state.StateModule; import io.agentscope.core.tool.AgentTool; @@ -25,7 +29,9 @@ import io.agentscope.core.tool.file.ReadFileTool; import io.agentscope.core.tool.file.WriteFileTool; import io.agentscope.core.tool.mcp.McpClientWrapper; +import io.agentscope.core.tool.subagent.ContextSharingMode; import io.agentscope.core.tool.subagent.SubAgentConfig; +import io.agentscope.core.tool.subagent.SubAgentContext; import io.agentscope.core.tool.subagent.SubAgentProvider; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -53,6 +59,7 @@ public class SkillBox implements StateModule { private Path uploadDir; private SkillFileFilter fileFilter; private boolean autoUploadSkill = true; + private SkillModelProvider modelProvider; /** * Creates a SkillBox without a toolkit. @@ -63,11 +70,11 @@ public class SkillBox implements StateModule { */ @Deprecated public SkillBox() { - this(null, null, null); + this(null, null, null, null); } public SkillBox(Toolkit toolkit) { - this(toolkit, null, null); + this(toolkit, null, null, null); } /** @@ -77,7 +84,7 @@ public SkillBox(Toolkit toolkit) { * @param template Custom skill template (null or blank uses default) */ public SkillBox(String instruction, String template) { - this(null, instruction, template); + this(null, instruction, template, null); } /** @@ -88,10 +95,28 @@ public SkillBox(String instruction, String template) { * @param template Custom skill template (null or blank uses default) */ public SkillBox(Toolkit toolkit, String instruction, String template) { + this(toolkit, instruction, template, null); + } + + /** + * Creates a SkillBox with a toolkit, custom skill prompt instruction, template, + * and a model provider. + * + * @param toolkit The toolkit to bind + * @param instruction Custom instruction header (null or blank uses default) + * @param template Custom skill template (null or blank uses default) + * @param modelProvider The model provider for resolving model references + */ + public SkillBox( + Toolkit toolkit, + String instruction, + String template, + SkillModelProvider modelProvider) { this.skillPromptProvider = new AgentSkillPromptProvider(skillRegistry, instruction, template); - this.skillToolFactory = new SkillToolFactory(skillRegistry, toolkit); + this.skillToolFactory = new SkillToolFactory(skillRegistry, toolkit, this); this.toolkit = toolkit; + this.modelProvider = modelProvider; } /** @@ -151,6 +176,24 @@ public void bindToolkit(Toolkit toolkit) { this.skillToolFactory.bindToolkit(toolkit); } + /** + * Gets the model provider for resolving model references. + * + * @return The model provider, or null if not set + */ + public SkillModelProvider getModelProvider() { + return modelProvider; + } + + /** + * Sets the model provider for resolving model references. + * + * @param modelProvider The model provider to set + */ + public void setModelProvider(SkillModelProvider modelProvider) { + this.modelProvider = modelProvider; + } + /** * Synchronize tool group states based on skill activation status with a specific toolkit. * @@ -238,6 +281,125 @@ public void registerSkill(AgentSkill skill) { skillRegistry.registerSkill(skillId, skill, registered); logger.info("Registered skill '{}'", skillId); + + // Create sub-agent tool if skill has model configured + createSubAgentForSkill(skill, skillId); + } + + /** + * Creates a SubAgentTool for a skill if it has a model configured. + * + *

This method is called after skill registration to automatically create + * sub-agent tools for skills with model configuration. + * + * @param skill The skill to check for model configuration + * @param skillId The skill ID + */ + private void createSubAgentForSkill(AgentSkill skill, String skillId) { + String modelRef = skill.getModel(); + if (modelRef == null || modelRef.isBlank()) { + logger.debug( + "Skill '{}' has no model configured, skipping sub-agent creation", skillId); + return; + } + + if (toolkit == null) { + logger.warn( + "No toolkit configured for skill '{}', cannot create sub-agent with model '{}'", + skillId, + modelRef); + return; + } + + if (modelProvider == null) { + logger.warn( + "No SkillModelProvider configured for skill '{}', " + + "cannot create sub-agent with model '{}'", + skillId, + modelRef); + return; + } + + Model model = modelProvider.getModel(modelRef); + if (model == null) { + logger.warn( + "Model '{}' not found for skill '{}', skipping sub-agent creation", + modelRef, + skillId); + return; + } + + // Create tool group if needed + String skillToolGroup = skillId + "_skill_tools"; + if (toolkit.getToolGroup(skillToolGroup) == null) { + toolkit.createToolGroup(skillToolGroup, skillToolGroup, false); + } + + // Build system prompt using SkillSubagentPromptBuilder + final Model resolvedModel = model; + final Toolkit toolkitCopy = toolkit.copy(); + final String systemPrompt = + SkillSubagentPromptBuilder.builder() + .skill(skill) + .modelName(resolvedModel.getModelName()) + .build(); + + // Parse context sharing mode from skill + final ContextSharingMode contextMode = parseContextSharingMode(skill.getContext()); + + // Create SubAgentProvider - context-aware for memory sharing + SubAgentProvider provider = + new SubAgentProvider<>() { + @Override + public ReActAgent provideWithContext(SubAgentContext context) { + ReActAgent.Builder agentBuilder = + ReActAgent.builder() + .name(skill.getName() + "_agent") + .description(skill.getDescription()) + .model(resolvedModel) + .toolkit(toolkitCopy); + + // Check if context provides a memory to use (SHARED or FORK mode) + Memory memoryToUse = context.getMemoryToUse(); + if (memoryToUse != null) { + // Use the provided memory (shared or forked from parent) + agentBuilder.memory(memoryToUse); + } else { + // No memory provided - use independent memory with our system + // prompt + agentBuilder.sysPrompt(systemPrompt).memory(new InMemoryMemory()); + } + + return agentBuilder.build(); + } + }; + + // Register sub-agent tool + String toolName = "call_" + skill.getName(); + toolkit.registration() + .group(skillToolGroup) + .subAgent( + provider, + SubAgentConfig.builder() + .toolName(toolName) + .description( + "Execute " + + skill.getName() + + " skill task using model '" + + model.getModelName() + + "'") + .contextSharingMode(contextMode) + .build()) + .apply(); + + // Activate the tool group + toolkit.updateToolGroups(List.of(skillToolGroup), true); + + logger.info( + "Created sub-agent tool '{}' for skill '{}' with model '{}'", + toolName, + skillId, + model.getModelName()); } /** @@ -323,6 +485,7 @@ public static class SkillRegistration { private ExtendedModel extendedModel; private List enableTools; private List disableTools; + private String runtimeModel; public SkillRegistration(SkillBox skillBox) { this.skillBox = skillBox; @@ -513,6 +676,22 @@ public SkillRegistration extendedModel(ExtendedModel extendedModel) { return this; } + /** + * Set the runtime model to override the skill's model. + * + *

When set, this takes priority over the skill's model field. This allows dynamic model + * assignment at registration time. + * + *

Priority: runtimeModel > skill.getModel() + * + * @param runtimeModel The model reference (e.g., "haiku", "sonnet", "openai:gpt-4o") + * @return This builder for chaining + */ + public SkillRegistration runtimeModel(String runtimeModel) { + this.runtimeModel = runtimeModel; + return this; + } + /** * Apply the registration with all configured options. * @@ -548,6 +727,177 @@ public void apply() { .subAgent(subAgentProvider, subAgentConfig) .apply(); } + + // Create SubAgentTool if skill has model + createSubAgentIfHasModel(); + } + + /** + * Resolve the effective model reference. + * + *

Priority: runtimeModel > skill.getModel() + * + * @return The effective model reference, or null if neither is set + */ + private String resolveEffectiveModel() { + if (runtimeModel != null && !runtimeModel.isBlank()) { + return runtimeModel; + } + return skill.getModel(); + } + + /** + * Create a SubAgentTool if the skill has a model configured. + * + *

This method automatically creates a sub-agent tool that can execute the skill using the + * configured model. The tool name follows the pattern "call_{skillName}". + * + *

If no model is configured, the model provider is missing, or the model is not found, + * this method does nothing (graceful degradation). + */ + private void createSubAgentIfHasModel() { + String effectiveModel = resolveEffectiveModel(); + logger.debug( + "createSubAgentIfHasModel called for skill '{}', effectiveModel='{}', " + + "toolkit={}, modelProvider={}", + skill.getName(), + effectiveModel, + skillBox.toolkit != null ? "present" : "null", + skillBox.modelProvider != null ? "present" : "null"); + + if (effectiveModel == null || effectiveModel.isBlank()) { + logger.debug( + "Skill '{}' has no model configured, skipping sub-agent creation", + skill.getName()); + return; // No model specified + } + + if (skillBox.toolkit == null) { + logger.warn( + "No toolkit configured for skill '{}', cannot create sub-agent with model" + + " '{}'", + skill.getName(), + effectiveModel); + return; + } + + if (skillBox.modelProvider == null) { + logger.warn( + "No SkillModelProvider configured for skill '{}', " + + "cannot create sub-agent with model '{}'", + skill.getName(), + effectiveModel); + return; + } + + Model model = skillBox.modelProvider.getModel(effectiveModel); + if (model == null) { + logger.warn( + "Model '{}' not found for skill '{}', skipping sub-agent creation", + effectiveModel, + skill.getName()); + return; + } + + // Use the same toolkit reference as skillBox + Toolkit effectiveToolkit = toolkit != null ? toolkit : skillBox.toolkit; + + // Create SubAgentProvider with structured system prompt + final Model resolvedModel = model; + final Toolkit toolkitCopy = effectiveToolkit.copy(); + final String systemPrompt = + SkillSubagentPromptBuilder.builder() + .skill(skill) + .modelName(resolvedModel.getModelName()) + .build(); + + // Parse context sharing mode from skill + final ContextSharingMode contextMode = parseContextSharingMode(skill.getContext()); + + // Create SubAgentProvider - context-aware for memory sharing + SubAgentProvider provider = + new SubAgentProvider<>() { + @Override + public ReActAgent provideWithContext(SubAgentContext context) { + ReActAgent.Builder agentBuilder = + ReActAgent.builder() + .name(skill.getName() + "_agent") + .description(skill.getDescription()) + .model(resolvedModel) + .toolkit(toolkitCopy); + + // Check if context provides a memory to use (SHARED or FORK mode) + Memory memoryToUse = context.getMemoryToUse(); + if (memoryToUse != null) { + // Use the provided memory (shared or forked from parent) + agentBuilder.memory(memoryToUse); + } else { + // No memory provided - use independent memory with our system + // prompt + agentBuilder.sysPrompt(systemPrompt).memory(new InMemoryMemory()); + } + + return agentBuilder.build(); + } + }; + + // Create tool group if needed + String skillToolGroup = skill.getSkillId() + "_skill_tools"; + if (effectiveToolkit.getToolGroup(skillToolGroup) == null) { + effectiveToolkit.createToolGroup(skillToolGroup, skillToolGroup, false); + } + + // Register sub-agent tool + effectiveToolkit + .registration() + .group(skillToolGroup) + .subAgent( + provider, + SubAgentConfig.builder() + .toolName("call_" + skill.getName()) + .description( + "Execute " + + skill.getName() + + " skill task using configured model") + .contextSharingMode(contextMode) + .build()) + .apply(); + + logger.info( + "Created sub-agent tool 'call_{}' for skill '{}' with model '{}'", + skill.getName(), + skill.getName(), + model.getModelName()); + } + } + + /** + * Parses the context sharing mode from skill's context field. + * + *

Supported values: + * + *

    + *
  • null, empty, "shared" - SHARED (default) + *
  • "fork" - FORK + *
  • "new" - NEW + *
+ * + * @param context The context string from skill + * @return The corresponding ContextSharingMode + */ + private static ContextSharingMode parseContextSharingMode(String context) { + if (context == null || context.isEmpty() || "shared".equalsIgnoreCase(context)) { + return ContextSharingMode.SHARED; + } else if ("fork".equalsIgnoreCase(context)) { + return ContextSharingMode.FORK; + } else if ("new".equalsIgnoreCase(context)) { + return ContextSharingMode.NEW; + } else { + logger.warn( + "Unknown context mode '{}', defaulting to SHARED. " + + "Supported values: shared, fork, new", + context); + return ContextSharingMode.SHARED; } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillModelProvider.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillModelProvider.java new file mode 100644 index 000000000..9c4d555dc --- /dev/null +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillModelProvider.java @@ -0,0 +1,50 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.skill; + +import io.agentscope.core.model.Model; + +/** + * Provider for creating Model instances based on model references. + * + *

Supports multiple model reference formats: + * + *

    + *
  • Short alias: "qwen-turbo", "qwen-plus" - mapped to full model names + *
  • Full name: "qwen-turbo" - used directly + *
  • Provider format: "dashscope:qwen-plus" - uses specified provider + *
+ */ +public interface SkillModelProvider { + + /** + * Get a Model instance for the given model reference. + * + * @param modelRef The model reference (e.g., "qwen-turbo", "dashscope:qwen-plus") + * @return Model instance, or null if not available + */ + Model getModel(String modelRef); + + /** + * Check if a model reference is available. + * + * @param modelRef The model reference + * @return true if the model can be provided + */ + default boolean isAvailable(String modelRef) { + return getModel(modelRef) != null; + } +} diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillSubagentPromptBuilder.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillSubagentPromptBuilder.java new file mode 100644 index 000000000..0e6dec4ca --- /dev/null +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillSubagentPromptBuilder.java @@ -0,0 +1,149 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.skill; + +/** + * Builder for constructing system prompts for skill-based sub-agents. + * + *

This class creates comprehensive system prompts that include: + * + *

    + *
  • Role definition - Who the sub-agent is + *
  • Skill description - What the skill does + *
  • Detailed instructions - How to execute the skill + *
  • Behavioral guidelines - Constraints and best practices + *
  • Tool usage guidance - How to use available tools + *
+ * + *

This ensures sub-agents have clear boundaries and understand their role, preventing + * behavior drift and ensuring focused, accurate responses. + * + *

Example usage: + * + *

{@code
+ * String systemPrompt = SkillSubagentPromptBuilder.builder()
+ *     .skill(skill)
+ *     .modelName("qwen-turbo")
+ *     .build();
+ * }
+ */ +public class SkillSubagentPromptBuilder { + + private static final String SKILL_SUBAGENT_TEMPLATE = + """ + You are a specialized agent for the skill: {skillName}. + + ## Your Purpose + + {skillDescription} + + ## Your Instructions + + {skillContent} + + ## Guidelines + + - Focus ONLY on tasks related to this skill + - Use the available tools appropriately to complete the task + - If the task is outside this skill's scope, clearly state so + - Be concise and accurate in your responses + - Report your findings clearly without unnecessary elaboration + - Do not make assumptions about data or files that are not provided + - Always verify information before making claims + + ## Tool Usage + + - You have access to tools through the toolkit + - Choose the most appropriate tool for each subtask + - Chain tool calls when necessary for complex operations + - Handle tool errors gracefully and report issues clearly + + ## Important Constraints + + - Do not perform actions unrelated to the skill's purpose + - Do not modify files unless explicitly required by the skill + - Do not share sensitive information in your responses + - Always respect the skill's intended use case + + --- + *Executing with model: {modelName}* + """; + + private AgentSkill skill; + private String modelName; + + private SkillSubagentPromptBuilder() {} + + /** + * Creates a new builder instance. + * + * @return New builder + */ + public static SkillSubagentPromptBuilder builder() { + return new SkillSubagentPromptBuilder(); + } + + /** + * Sets the skill for which to build the system prompt. + * + * @param skill The skill definition + * @return This builder + */ + public SkillSubagentPromptBuilder skill(AgentSkill skill) { + this.skill = skill; + return this; + } + + /** + * Sets the model name being used for execution. + * + * @param modelName The model name + * @return This builder + */ + public SkillSubagentPromptBuilder modelName(String modelName) { + this.modelName = modelName; + return this; + } + + /** + * Builds the complete system prompt for the skill sub-agent. + * + * @return The formatted system prompt + * @throws IllegalStateException if skill is not set + */ + public String build() { + if (skill == null) { + throw new IllegalStateException("Skill must be set before building"); + } + + String name = skill.getName() != null ? skill.getName() : "unknown"; + String description = + skill.getDescription() != null + ? skill.getDescription() + : "No description provided."; + String content = + skill.getSkillContent() != null + ? skill.getSkillContent() + : "No instructions provided."; + String model = modelName != null ? modelName : "default"; + + return SKILL_SUBAGENT_TEMPLATE + .replace("{skillName}", name) + .replace("{skillDescription}", description) + .replace("{skillContent}", content) + .replace("{modelName}", model); + } +} diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillToolFactory.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillToolFactory.java index 35681f6ea..d4082f6a3 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillToolFactory.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillToolFactory.java @@ -15,13 +15,23 @@ */ package io.agentscope.core.skill; +import io.agentscope.core.ReActAgent; +import io.agentscope.core.memory.InMemoryMemory; +import io.agentscope.core.memory.Memory; import io.agentscope.core.message.ToolResultBlock; +import io.agentscope.core.model.Model; import io.agentscope.core.tool.AgentTool; import io.agentscope.core.tool.ToolCallParam; import io.agentscope.core.tool.Toolkit; +import io.agentscope.core.tool.subagent.ContextSharingMode; +import io.agentscope.core.tool.subagent.SubAgentConfig; +import io.agentscope.core.tool.subagent.SubAgentContext; +import io.agentscope.core.tool.subagent.SubAgentProvider; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import reactor.core.publisher.Mono; @@ -34,11 +44,16 @@ class SkillToolFactory { private static final Logger logger = LoggerFactory.getLogger(SkillToolFactory.class); private final SkillRegistry skillRegistry; + private final SkillBox skillBox; private Toolkit toolkit; - SkillToolFactory(SkillRegistry skillRegistry, Toolkit toolkit) { + /** Tracks which skills have already had their sub-agent tools created. */ + private final Set skillsWithSubAgentCreated = new HashSet<>(); + + SkillToolFactory(SkillRegistry skillRegistry, Toolkit toolkit, SkillBox skillBox) { this.skillRegistry = skillRegistry; this.toolkit = toolkit; + this.skillBox = skillBox; } /** @@ -186,11 +201,29 @@ private String buildSkillMarkdownResponse(String skillId, AgentSkill skill) { result.append("Successfully loaded skill: ").append(skillId).append("\n\n"); result.append("Name: ").append(skill.getName()).append("\n"); result.append("Description: ").append(skill.getDescription()).append("\n"); + + // Add model info if present + if (skill.getModel() != null && !skill.getModel().isBlank()) { + result.append("Model: ").append(skill.getModel()).append("\n"); + } + result.append("Source: ").append(skill.getSource()).append("\n\n"); result.append("Content:\n"); result.append("---\n"); result.append(skill.getSkillContent()); result.append("\n---\n"); + + // Add hint about sub-agent tool if skill has a configured model + if (skill.getModel() != null && !skill.getModel().isBlank()) { + String toolName = "call_" + skill.getName(); + result.append("\n**Note:** This skill is configured to use model '") + .append(skill.getModel()) + .append("'.\n"); + result.append("Use the '**") + .append(toolName) + .append("**' tool to execute tasks with this skill's configured model.\n"); + } + return result.toString(); } @@ -249,6 +282,9 @@ private String buildResourceNotFoundMessage( /** * Validate skill exists and activate it and its tool group. * + *

This method also creates a sub-agent tool if the skill has a model configured + * and the sub-agent tool hasn't been created yet. + * * @param skillId The unique identifier of the skill * @return The skill instance * @throws IllegalArgumentException if skill doesn't exist @@ -280,6 +316,187 @@ private AgentSkill validatedActiveSkill(String skillId) { toolkit.getToolGroup(toolsGroupName).getTools()); } + // Create sub-agent tool if skill has model and not already created + createSubAgentIfHasModel(skill, skillId); + return skill; } + + /** + * Create a SubAgentTool if the skill has a model configured and not already created. + * + *

This method is called when a skill is dynamically loaded via load_skill_through_path. + * It ensures that skills with models get their sub-agent tools created on-demand. + * + * @param skill The skill to check for model configuration + * @param skillId The skill ID for tracking creation status + */ + private void createSubAgentIfHasModel(AgentSkill skill, String skillId) { + // Check if skill has a model configured + String modelRef = skill.getModel(); + logger.debug( + "createSubAgentIfHasModel called for skill '{}', modelRef='{}', " + + "toolkit={}, skillBox={}, modelProvider={}", + skill.getName(), + modelRef, + toolkit != null ? "present" : "null", + skillBox != null ? "present" : "null", + skillBox != null && skillBox.getModelProvider() != null ? "present" : "null"); + + if (modelRef == null || modelRef.isBlank()) { + logger.debug( + "Skill '{}' has no model configured, skipping sub-agent creation", + skill.getName()); + return; // No model specified + } + + // Check if sub-agent tool already created for this skill + if (skillsWithSubAgentCreated.contains(skillId)) { + logger.debug("Sub-agent tool already exists for skill '{}'", skillId); + return; + } + + // Check prerequisites + if (toolkit == null) { + logger.warn( + "No toolkit available for skill '{}', cannot create sub-agent with model '{}'", + skill.getName(), + modelRef); + return; + } + + if (skillBox == null || skillBox.getModelProvider() == null) { + logger.warn( + "No SkillModelProvider configured for skill '{}', " + + "cannot create sub-agent with model '{}'", + skill.getName(), + modelRef); + return; + } + + // Resolve model + Model model = skillBox.getModelProvider().getModel(modelRef); + if (model == null) { + logger.warn( + "Model '{}' not found for skill '{}', skipping sub-agent creation", + modelRef, + skill.getName()); + return; + } + + // Create tool group if needed + String skillToolGroup = skillId + "_skill_tools"; + if (toolkit.getToolGroup(skillToolGroup) == null) { + toolkit.createToolGroup(skillToolGroup, skillToolGroup, false); + } + + // Parse context sharing mode from skill + ContextSharingMode contextMode = parseContextSharingMode(skill.getContext()); + + // Build system prompt using SkillSubagentPromptBuilder (only used for NEW mode) + final Model resolvedModel = model; + final Toolkit toolkitCopy = toolkit.copy(); + final String systemPrompt = + SkillSubagentPromptBuilder.builder() + .skill(skill) + .modelName(resolvedModel.getModelName()) + .build(); + + // Create SubAgentProvider - context-aware for memory sharing + // For SHARED and FORK modes, the context will contain the memory to use + // For NEW mode, the context will have null memory, so we use our own + SubAgentProvider provider = + new SubAgentProvider<>() { + @Override + public ReActAgent provideWithContext(SubAgentContext context) { + ReActAgent.Builder agentBuilder = + ReActAgent.builder() + .name(skill.getName() + "_agent") + .description(skill.getDescription()) + .model(resolvedModel) + .toolkit(toolkitCopy); + + // Check if context provides a memory to use (SHARED or FORK mode) + Memory memoryToUse = context.getMemoryToUse(); + if (memoryToUse != null) { + // Use the provided memory (shared or forked from parent) + agentBuilder.memory(memoryToUse); + logger.debug( + "Sub-agent '{}' using {} memory from context", + skill.getName(), + context.getContextSharingMode()); + } else { + // No memory provided - use independent memory with our system prompt + // This is the NEW mode case + agentBuilder.sysPrompt(systemPrompt).memory(new InMemoryMemory()); + logger.debug( + "Sub-agent '{}' using independent memory with skill system" + + " prompt", + skill.getName()); + } + + return agentBuilder.build(); + } + }; + + // Register sub-agent tool with context sharing mode from skill + String toolName = "call_" + skill.getName(); + toolkit.registration() + .group(skillToolGroup) + .subAgent( + provider, + SubAgentConfig.builder() + .toolName(toolName) + .description( + "Execute " + + skill.getName() + + " skill task using model '" + + model.getModelName() + + "'") + .contextSharingMode(contextMode) + .build()) + .apply(); + + // Mark as created + skillsWithSubAgentCreated.add(skillId); + + // Activate the tool group + toolkit.updateToolGroups(List.of(skillToolGroup), true); + + logger.info( + "Created sub-agent tool '{}' for skill '{}' with model '{}' (dynamic loading)", + toolName, + skill.getName(), + model.getModelName()); + } + + /** + * Parses the context sharing mode from skill's context field. + * + *

Supported values: + * + *

    + *
  • null, empty, "shared" - SHARED (default) + *
  • "fork" - FORK + *
  • "new" - NEW + *
+ * + * @param context The context string from skill + * @return The corresponding ContextSharingMode + */ + private ContextSharingMode parseContextSharingMode(String context) { + if (context == null || context.isEmpty() || "shared".equalsIgnoreCase(context)) { + return ContextSharingMode.SHARED; + } else if ("fork".equalsIgnoreCase(context)) { + return ContextSharingMode.FORK; + } else if ("new".equalsIgnoreCase(context)) { + return ContextSharingMode.NEW; + } else { + logger.warn( + "Unknown context mode '{}', defaulting to SHARED. " + + "Supported values: shared, fork, new", + context); + return ContextSharingMode.SHARED; + } + } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java index 46b29e0c8..8aa0688ad 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java @@ -91,6 +91,8 @@ public static AgentSkill createFrom( String name = metadata.get("name"); String description = metadata.get("description"); + String model = metadata.get("model"); + String context = metadata.get("context"); String skillContent = parsed.getContent(); if (name == null || name.isEmpty() || description == null || description.isEmpty()) { @@ -103,7 +105,7 @@ public static AgentSkill createFrom( "The SKILL.md must have content except for the YAML Front Matter."); } - return new AgentSkill(name, description, skillContent, resources, source); + return new AgentSkill(name, description, skillContent, resources, source, model, context); } /** diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/ContextSharingMode.java b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/ContextSharingMode.java new file mode 100644 index 000000000..32869a34a --- /dev/null +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/ContextSharingMode.java @@ -0,0 +1,64 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.tool.subagent; + +/** + * Mode for sharing memory context between parent agent and sub-agent. + * + *

This enum defines how memory is shared between the parent agent and sub-agent: + * + *

    + *
  • shared (default): SubAgent shares the same memory instance with parent. All messages + * are immediately visible to both agents. SubAgent uses parent's system prompt. + *
  • fork: SubAgent gets a copy (fork) of parent's memory at invocation time. SubAgent's + * execution doesn't affect parent's memory. SubAgent uses parent's system prompt. + *
  • new: SubAgent has completely independent memory with its own system prompt. No + * context from parent. + *
+ * + *

Aligned with skill specification: https://code.claude.com/docs/zh-CN/skills + */ +public enum ContextSharingMode { + /** + * Shared memory mode (default). + * + *

SubAgent uses the same memory instance as the parent agent. All messages are immediately + * visible to both. SubAgent uses parent's system prompt context. + * + *

This is the default mode when context is not specified in skill.md. + */ + SHARED, + + /** + * Fork memory mode. + * + *

SubAgent gets a copy (fork) of parent's memory at invocation time. Changes made by + * SubAgent don't affect parent's memory. SubAgent uses parent's system prompt context. + * + *

Use this when you need context awareness but want isolation from parent's memory. + */ + FORK, + + /** + * New independent memory mode. + * + *

SubAgent has completely independent memory with its own system prompt. No context from + * parent is available. + * + *

Use this for isolated tasks that don't need parent context. + */ + NEW +} diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentConfig.java b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentConfig.java index 66b3792fa..5fb20ceee 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentConfig.java +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentConfig.java @@ -27,12 +27,12 @@ * supports smart defaults that derive tool name and description from the agent itself. * *

Sub-agents operate in conversation mode, supporting multi-turn dialogue with session - * management. The tool exposes two parameters: + * management. The tool exposes the following parameters: * *

    *
  • {@code message} - Required. The message to send to the agent. - *
  • {@code conversation_id} - Optional. Omit to start a new conversation, provide to continue - * an existing one. + *
  • {@code session_id} - Optional. Omit to start a new conversation, provide to continue an + * existing one. *
* *

Default Behavior: @@ -42,12 +42,24 @@ *

  • Description: uses agent's description, or generates a default *
  • Session: uses {@link InMemorySession} for conversation state management *
  • Event forwarding: enabled by default + *
  • Context sharing: SHARED by default (sub-agent shares parent's memory) + * + * + *

    Context Sharing Modes: + * + *

      + *
    • {@link ContextSharingMode#SHARED} (default): Sub-agent shares the same memory instance with + * parent. All messages are immediately visible to both. + *
    • {@link ContextSharingMode#FORK}: Sub-agent gets a copy of parent's memory. Changes don't + * affect parent's memory. + *
    • {@link ContextSharingMode#NEW}: Sub-agent has completely independent memory with its own + * system prompt. *
    * *

    Example usage: * *

    {@code
    - * // Minimal configuration - uses all defaults
    + * // Minimal configuration - uses all defaults (SHARED context)
      * SubAgentConfig config = SubAgentConfig.defaults();
      *
      * // Custom configuration with persistent session
    @@ -56,6 +68,16 @@
      *     .description("Ask the expert a question")
      *     .session(new JsonSession(Path.of("sessions")))
      *     .build();
    + *
    + * // Use FORK mode for isolated context
    + * SubAgentConfig config = SubAgentConfig.builder()
    + *     .contextSharingMode(ContextSharingMode.FORK)
    + *     .build();
    + *
    + * // Use NEW mode for completely independent sub-agent
    + * SubAgentConfig config = SubAgentConfig.builder()
    + *     .contextSharingMode(ContextSharingMode.NEW)
    + *     .build();
      * }
    */ public class SubAgentConfig { @@ -65,6 +87,7 @@ public class SubAgentConfig { private final boolean forwardEvents; private final StreamOptions streamOptions; private final Session session; + private final ContextSharingMode contextSharingMode; private SubAgentConfig(Builder builder) { this.toolName = builder.toolName; @@ -72,6 +95,7 @@ private SubAgentConfig(Builder builder) { this.forwardEvents = builder.forwardEvents; this.streamOptions = builder.streamOptions; this.session = builder.session != null ? builder.session : new InMemorySession(); + this.contextSharingMode = builder.contextSharingMode; } /** @@ -148,6 +172,23 @@ public Session getSession() { return session; } + /** + * Gets the context sharing mode. + * + *

    Controls how memory is shared between the parent agent and sub-agent: + * + *

      + *
    • {@link ContextSharingMode#SHARED} (default): Share same memory instance + *
    • {@link ContextSharingMode#FORK}: Copy parent memory at invocation + *
    • {@link ContextSharingMode#NEW}: Independent memory with own system prompt + *
    + * + * @return The context sharing mode + */ + public ContextSharingMode getContextSharingMode() { + return contextSharingMode; + } + /** Builder for SubAgentConfig. */ public static class Builder { private String toolName; @@ -155,6 +196,7 @@ public static class Builder { private boolean forwardEvents = true; private StreamOptions streamOptions; private Session session; + private ContextSharingMode contextSharingMode = ContextSharingMode.SHARED; private Builder() {} @@ -229,6 +271,25 @@ public Builder session(Session session) { return this; } + /** + * Sets the context sharing mode. + * + *

    Controls how memory is shared between the parent agent and sub-agent: + * + *

      + *
    • {@link ContextSharingMode#SHARED} (default): Share same memory instance + *
    • {@link ContextSharingMode#FORK}: Copy parent memory at invocation + *
    • {@link ContextSharingMode#NEW}: Independent memory with own system prompt + *
    + * + * @param contextSharingMode The context sharing mode + * @return This builder + */ + public Builder contextSharingMode(ContextSharingMode contextSharingMode) { + this.contextSharingMode = contextSharingMode; + return this; + } + /** * Builds the configuration. * diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentContext.java b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentContext.java new file mode 100644 index 000000000..21a7b2612 --- /dev/null +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentContext.java @@ -0,0 +1,116 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.tool.subagent; + +import io.agentscope.core.agent.Agent; +import io.agentscope.core.memory.Memory; + +/** + * Context information provided to SubAgentProvider when creating agent instances. + * + *

    This context allows the provider to make informed decisions about agent + * configuration based on the parent agent's state, particularly for memory + * sharing modes. + * + *

    Usage example: + *

    {@code
    + * SubAgentProvider provider = context -> {
    + *     ReActAgent.Builder builder = ReActAgent.builder()
    + *         .name("MyAgent")
    + *         .model(model);
    + *
    + *     if (context.getMemoryToUse() != null) {
    + *         builder.memory(context.getMemoryToUse());
    + *     }
    + *
    + *     return builder.build();
    + * };
    + * }
    + */ +public class SubAgentContext { + + private final Agent parentAgent; + private final ContextSharingMode contextSharingMode; + private final Memory memoryToUse; + + /** + * Creates a new SubAgentContext. + * + * @param parentAgent The parent agent that is invoking the sub-agent + * @param contextSharingMode The context sharing mode to use + * @param memoryToUse The memory to use for the sub-agent (may be null for default) + */ + public SubAgentContext( + Agent parentAgent, ContextSharingMode contextSharingMode, Memory memoryToUse) { + this.parentAgent = parentAgent; + this.contextSharingMode = contextSharingMode; + this.memoryToUse = memoryToUse; + } + + /** + * Creates an empty context with no parent agent information. + * + *

    This is used for backward compatibility when no context is available. + * + * @return An empty SubAgentContext + */ + public static SubAgentContext empty() { + return new SubAgentContext(null, ContextSharingMode.NEW, null); + } + + /** + * Gets the parent agent that is invoking the sub-agent. + * + * @return The parent agent, or null if not available + */ + public Agent getParentAgent() { + return parentAgent; + } + + /** + * Gets the context sharing mode. + * + * @return The context sharing mode + */ + public ContextSharingMode getContextSharingMode() { + return contextSharingMode; + } + + /** + * Gets the memory to use for the sub-agent. + * + *

    This is pre-computed based on the context sharing mode: + *

      + *
    • SHARED: The parent agent's memory instance
    • + *
    • FORK: A forked copy of the parent agent's memory
    • + *
    • NEW: null (sub-agent should use its own memory)
    • + *
    + * + * @return The memory to use, or null if the sub-agent should use its own memory + */ + public Memory getMemoryToUse() { + return memoryToUse; + } + + /** + * Checks if this context has valid parent agent information. + * + * @return true if parent agent information is available + */ + public boolean hasParentAgent() { + return parentAgent != null; + } +} diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentProvider.java b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentProvider.java index 1b12df0ae..a1ad53336 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentProvider.java +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentProvider.java @@ -23,14 +23,31 @@ *

    Since ReActAgent is not thread-safe, this provider pattern ensures that each tool call gets a * fresh agent instance. This is similar to Spring's ObjectProvider pattern. * - *

    Example usage: + *

    The provider supports context-aware agent creation for memory sharing features: + *

      + *
    • The {@link #provideWithContext(SubAgentContext)} method receives context about the parent + * agent and the memory to use
    • + *
    • For SHARED mode, the context contains the parent's memory instance
    • + *
    • For FORK mode, the context contains a forked copy of parent's memory
    • + *
    • For NEW mode, the context contains null (use independent memory)
    • + *
    + * + *

    Example usage (context-aware for memory sharing): * *

    {@code
    - * SubAgentProvider provider = () -> ReActAgent.builder()
    - *     .name("ResearchAgent")
    - *     .model(model)
    - *     .sysPrompt("You are a research expert...")
    - *     .build();
    + * SubAgentProvider provider = context -> {
    + *     ReActAgent.Builder builder = ReActAgent.builder()
    + *         .name("ResearchAgent")
    + *         .model(model)
    + *         .sysPrompt("You are a research expert...");
    + *
    + *     // Use shared/forked memory if provided
    + *     if (context.getMemoryToUse() != null) {
    + *         builder.memory(context.getMemoryToUse());
    + *     }
    + *
    + *     return builder.build();
    + * };
      *
      * toolkit.registration()
      *     .subAgent(provider)
    @@ -43,12 +60,32 @@
     public interface SubAgentProvider {
     
         /**
    -     * Provides a new agent instance.
    +     * Provides a new agent instance with context information.
    +     *
    +     * 

    This method is called for each tool invocation to ensure thread safety. + * The context provides information about the parent agent and the memory to use. + * + *

    Implementations should: + *

      + *
    • Check {@link SubAgentContext#getMemoryToUse()} - if not null, use it as the agent's + * memory to enable SHARED or FORK context sharing modes
    • + *
    • If null, create the agent with its own independent memory
    • + *
    + * + * @param context The context containing parent agent information and memory to use + * @return A new agent instance + */ + T provideWithContext(SubAgentContext context); + + /** + * Provides a new agent instance without context information. * - *

    This method is called for each tool invocation to ensure thread safety. Implementations - * should create a new agent instance each time this method is called. + *

    This is a convenience method for backward compatibility. It calls + * {@link #provideWithContext(SubAgentContext)} with an empty context. * * @return A new agent instance */ - T provide(); + default T provide() { + return provideWithContext(SubAgentContext.empty()); + } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java index d0cd8d0f6..18cc59c78 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java @@ -15,9 +15,12 @@ */ package io.agentscope.core.tool.subagent; +import io.agentscope.core.ReActAgent; import io.agentscope.core.agent.Agent; import io.agentscope.core.agent.Event; import io.agentscope.core.agent.StreamOptions; +import io.agentscope.core.memory.Memory; +import io.agentscope.core.message.ImageBlock; import io.agentscope.core.message.Msg; import io.agentscope.core.message.MsgRole; import io.agentscope.core.message.TextBlock; @@ -27,7 +30,9 @@ import io.agentscope.core.tool.AgentTool; import io.agentscope.core.tool.ToolCallParam; import io.agentscope.core.tool.ToolEmitter; +import io.agentscope.core.tool.ToolExecutionContext; import io.agentscope.core.util.JsonUtils; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -45,13 +50,24 @@ *

    Thread safety is ensured by using {@link SubAgentProvider} to create a fresh agent instance * for each new session. * - *

    The tool exposes two parameters: + *

    The tool exposes the following parameters: * *

      *
    • {@code session_id} - Optional. Omit to start a new session, provide to continue an * existing one. *
    • {@code message} - Required. The message to send to the agent. *
    + * + *

    Context Sharing Modes: + * + *

      + *
    • SHARED (default): Sub-agent shares the same memory instance with parent. All + * messages are immediately visible to both. Sub-agent uses parent's system prompt context. + *
    • FORK: Sub-agent gets a copy of parent's memory at invocation time. Changes don't + * affect parent's memory. Sub-agent uses parent's system prompt context. + *
    • NEW: Sub-agent has completely independent memory with its own system prompt. No + * context from parent. + *
    */ public class SubAgentTool implements AgentTool { @@ -63,6 +79,17 @@ public class SubAgentTool implements AgentTool { /** Parameter name for message. */ private static final String PARAM_MESSAGE = "message"; + /** + * Context key for parent session ID. + * + *

    Applications can register the parent session ID in ToolExecutionContext to enable session + * inheritance for SHARED/FORK modes. The sub-agent will derive its session ID from the parent + * session. + * + *

    Example: context.register(CONTEXT_KEY_PARENT_SESSION_ID, "parent_session_123", String.class) + */ + public static final String CONTEXT_KEY_PARENT_SESSION_ID = "parentSessionId"; + private final String name; private final String description; private final SubAgentProvider agentProvider; @@ -114,6 +141,7 @@ public Mono callAsync(ToolCallParam param) { *

      *
    • Session ID generation for new conversations *
    • Agent state loading for continued sessions + *
    • Memory sharing based on context sharing mode *
    • Message execution (streaming or non-streaming based on config) *
    • Agent state persistence after execution *
    @@ -130,8 +158,26 @@ private Mono executeConversation(ToolCallParam param) { // Get or create session ID String sessionId = (String) input.get(PARAM_SESSION_ID); boolean isNewSession = sessionId == null; + + // Get context sharing mode early for session inheritance logic + ContextSharingMode contextMode = config.getContextSharingMode(); + if (isNewSession) { - sessionId = UUID.randomUUID().toString(); + // Try to inherit session ID from parent in SHARED/FORK modes + String parentSessionId = getParentSessionId(param.getContext()); + if (parentSessionId != null + && (contextMode == ContextSharingMode.SHARED + || contextMode == ContextSharingMode.FORK)) { + // Derive sub-agent session ID from parent session + sessionId = parentSessionId + "_" + name; + logger.debug( + "Inherited session ID from parent: {} -> {}", + parentSessionId, + sessionId); + } else { + // Generate new session ID + sessionId = UUID.randomUUID().toString(); + } } // Get message @@ -140,21 +186,48 @@ private Mono executeConversation(ToolCallParam param) { return Mono.just(ToolResultBlock.error("Message is required")); } - // Create agent for this session + // Prepare context for agent creation final String finalSessionId = sessionId; - Agent agent = agentProvider.provide(); - - // Load existing state if continuing session - if (!isNewSession && agent instanceof StateModule) { + Agent parentAgent = param.getAgent(); + + // Compute memory to use based on context sharing mode + Memory memoryToUse = computeMemoryToUse(parentAgent, contextMode); + + // Create SubAgentContext with parent agent and memory + SubAgentContext context = + new SubAgentContext(parentAgent, contextMode, memoryToUse); + + // Create agent with context (memory is set during construction) + Agent agent = agentProvider.provideWithContext(context); + + // Log sub-agent execution for debugging - Including model info if available + String modelInfo = "unknown"; + if (agent instanceof io.agentscope.core.ReActAgent) { + io.agentscope.core.model.Model agentModel = + ((io.agentscope.core.ReActAgent) agent).getModel(); + if (agentModel != null) { + modelInfo = agentModel.getModelName(); + } + } + logger.info( + "SubAgentTool executing: toolName={}, agentName={}, agentId={}," + + " model={}, contextMode={}", + name, + agent.getName(), + agent.getAgentId(), + modelInfo, + contextMode); + + // Load existing state if continuing session (only for NEW mode) + if (!isNewSession + && contextMode == ContextSharingMode.NEW + && agent instanceof StateModule) { loadAgentState(finalSessionId, (StateModule) agent); } - // Build user message - Msg userMsg = - Msg.builder() - .role(MsgRole.USER) - .content(TextBlock.builder().text(message).build()) - .build(); + // Build user message - in SHARED/FORK modes, try to preserve images from + // original user message + Msg userMsg = buildUserMessage(message, memoryToUse); logger.debug( "Session {} with agent '{}': {}", @@ -173,13 +246,13 @@ private Mono executeConversation(ToolCallParam param) { result = executeWithoutStreaming(agent, userMsg, finalSessionId); } - // Save state after execution - return result.doOnSuccess( - r -> { - if (agent instanceof StateModule) { - saveAgentState(finalSessionId, (StateModule) agent); - } - }); + // Save state after execution (only for NEW mode with independent session) + if (contextMode == ContextSharingMode.NEW && agent instanceof StateModule) { + return result.doOnSuccess( + r -> saveAgentState(finalSessionId, (StateModule) agent)); + } + + return result; } catch (Exception e) { logger.error("Error in session setup: {}", e.getMessage(), e); return Mono.just( @@ -188,6 +261,138 @@ private Mono executeConversation(ToolCallParam param) { }); } + /** + * Computes the memory to use based on context sharing mode. + * + *

    + * + *

      + *
    • SHARED: Returns parent's memory directly + *
    • FORK: Returns a fork of parent's memory + *
    • NEW: Returns null (sub-agent should use its own independent memory) + *
    + * + * @param parentAgent The parent agent (source of memory for SHARED/FORK modes) + * @param contextMode The context sharing mode + * @return The memory to use, or null for independent memory + */ + private Memory computeMemoryToUse(Agent parentAgent, ContextSharingMode contextMode) { + // Only ReActAgent supports memory sharing + if (!(parentAgent instanceof ReActAgent parentReactAgent)) { + logger.debug("Parent is not a ReActAgent, sub-agent will use independent memory"); + return null; + } + + Memory parentMemory = parentReactAgent.getMemory(); + if (parentMemory == null) { + logger.debug("Parent has no memory, sub-agent will use independent memory"); + return null; + } + + switch (contextMode) { + case SHARED: + // Fork parent's memory and remove pending tool calls + // Note: We cannot directly share parent's memory because it contains + // the pending tool call to this sub-agent, which would cause + // validation errors when the sub-agent tries to add new messages. + // We use a forked copy with pending tool calls removed. + Memory sharedMemory = parentMemory.fork(); + removePendingToolCalls(sharedMemory); + logger.debug( + "Sub-agent will use SHARED (forked with pending calls removed) memory from" + + " parent ({} messages)", + sharedMemory.getMessages().size()); + return sharedMemory; + + case FORK: + // Fork parent's memory and remove pending tool calls + Memory forkedMemory = parentMemory.fork(); + removePendingToolCalls(forkedMemory); + logger.debug( + "Sub-agent will use FORKed memory from parent ({} messages)", + forkedMemory.getMessages().size()); + return forkedMemory; + + case NEW: + // Use independent memory (return null) + logger.debug("Sub-agent will use NEW independent memory"); + return null; + + default: + logger.debug( + "Unknown context sharing mode: {}, using independent memory", contextMode); + return null; + } + } + + /** + * Removes pending tool calls from memory. + * + *

    This is necessary because when a sub-agent is invoked as a tool, the parent's memory + * contains the tool_use block that called the sub-agent. If we share this memory directly, + * the sub-agent will fail validation when trying to add new messages because of the pending + * tool call. + * + *

    This method removes: + *

      + *
    • ASSISTANT messages that contain only ToolUseBlocks (no text)
    • + *
    • Messages with pending tool calls that haven't been resolved
    • + *
    + * + * @param memory The memory to clean up + */ + private void removePendingToolCalls(Memory memory) { + List messages = memory.getMessages(); + // Iterate backwards and remove messages with pending tool calls + for (int i = messages.size() - 1; i >= 0; i--) { + Msg msg = messages.get(i); + // Check if this is an ASSISTANT message with only tool calls (no text content) + if (msg.getRole() == MsgRole.ASSISTANT && hasOnlyToolCalls(msg)) { + memory.deleteMessage(i); + logger.debug("Removed pending tool call message from shared memory"); + } else { + // Stop at the first message that's not a pending tool call + break; + } + } + } + + /** + * Checks if a message contains only tool calls (no text content). + * + * @param msg The message to check + * @return true if the message has only ToolUseBlocks + */ + private boolean hasOnlyToolCalls(Msg msg) { + List content = msg.getContent(); + if (content == null || content.isEmpty()) { + return false; + } + for (io.agentscope.core.message.ContentBlock block : content) { + if (block instanceof io.agentscope.core.message.TextBlock) { + return false; // Has text content, not just tool calls + } + } + // Check if there are any ToolUseBlocks + return content.stream().anyMatch(b -> b instanceof io.agentscope.core.message.ToolUseBlock); + } + + /** + * Gets the parent session ID from the tool execution context. + * + *

    The parent session ID should be registered in the context with the key {@link + * #CONTEXT_KEY_PARENT_SESSION_ID}. + * + * @param context The tool execution context, may be null + * @return The parent session ID, or null if not available + */ + private String getParentSessionId(ToolExecutionContext context) { + if (context == null) { + return null; + } + return context.get(CONTEXT_KEY_PARENT_SESSION_ID, String.class); + } + /** * Loads agent state from the session storage. * @@ -345,10 +550,42 @@ private ToolResultBlock buildResult(Msg response, String sessionId) { sessionId, textContent != null ? textContent : "(No response)")); } + /** + * Builds a user message with text content. + * + * @param message The text message + * @return The constructed message + */ + private Msg buildUserMessage(String message, Memory memory) { + // Try to extract images from the most recent user message in memory + List contentBlocks = new ArrayList<>(); + contentBlocks.add(TextBlock.builder().text(message).build()); + + if (memory != null) { + List messages = memory.getMessages(); + // Search from the most recent message backwards for images + for (int i = messages.size() - 1; i >= 0; i--) { + Msg msg = messages.get(i); + if (msg.getRole() == MsgRole.USER) { + List images = msg.getContentBlocks(ImageBlock.class); + if (!images.isEmpty()) { + contentBlocks.addAll(images); + logger.debug( + "Found {} image(s) in user message, forwarding to sub-agent", + images.size()); + break; // Only take images from the most recent user message + } + } + } + } + + return Msg.builder().role(MsgRole.USER).content(contentBlocks).build(); + } + /** * Builds the JSON schema for tool parameters. * - *

    Creates a schema with two properties: + *

    Creates a schema with properties: * *

      *
    • {@code session_id} - Optional string for continuing existing conversations @@ -363,12 +600,12 @@ private Map buildSchema() { Map properties = new HashMap<>(); - // Session ID (optional) + // Session ID (optional) - allow null since LLMs may explicitly pass null Map sessionIdProp = new HashMap<>(); - sessionIdProp.put("type", "string"); + sessionIdProp.put("type", List.of("string", "null")); sessionIdProp.put( "description", - "Session ID for multi-turn dialogue. Omit to start a NEW session." + "Session ID for multi-turn dialogue. Omit or pass null to start a NEW session." + " To CONTINUE an existing session and retain memory, you MUST extract" + " the session_id from the previous response and pass it here."); properties.put(PARAM_SESSION_ID, sessionIdProp); diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillTest.java index c6521f55f..193e003e7 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillTest.java @@ -283,4 +283,48 @@ void testGetResourcePathsNoResources() { assertNotNull(paths); assertTrue(paths.isEmpty()); } + + @Test + @DisplayName("Should create skill with model") + void shouldCreateSkillWithModel() { + AgentSkill skill = + AgentSkill.builder() + .name("test_skill") + .description("Test skill") + .skillContent("Content") + .model("qwen-turbo") + .build(); + + assertEquals("qwen-turbo", skill.getModel()); + } + + @Test + @DisplayName("Should allow null model") + void shouldAllowNullModel() { + AgentSkill skill = + AgentSkill.builder() + .name("test_skill") + .description("Test skill") + .skillContent("Content") + .build(); + + assertEquals(null, skill.getModel()); + } + + @Test + @DisplayName("Should modify model via toBuilder") + void shouldModifyModelViaToBuilder() { + AgentSkill original = + AgentSkill.builder() + .name("test_skill") + .description("Test skill") + .skillContent("Content") + .model("qwen-turbo") + .build(); + + AgentSkill modified = original.toBuilder().model("qwen-plus").build(); + + assertEquals("qwen-turbo", original.getModel()); + assertEquals("qwen-plus", modified.getModel()); + } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/MapBasedSkillModelProviderTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/MapBasedSkillModelProviderTest.java new file mode 100644 index 000000000..0e7b3274e --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/MapBasedSkillModelProviderTest.java @@ -0,0 +1,263 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.skill; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.agentscope.core.model.Model; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +class MapBasedSkillModelProviderTest { + + @Nested + @DisplayName("Direct Name Lookup") + class DirectNameLookupTests { + + @Test + @DisplayName("Should return model by direct name") + void shouldReturnModelByDirectName() { + Model qwenTurboModel = mock(Model.class); + when(qwenTurboModel.getModelName()).thenReturn("qwen-turbo"); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder() + .register("qwen-turbo", qwenTurboModel) + .build(); + + Model result = provider.getModel("qwen-turbo"); + assertEquals(qwenTurboModel, result); + } + + @Test + @DisplayName("Should return model with provider:model format") + void shouldResolveProviderFormat() { + Model qwenPlusModel = mock(Model.class); + when(qwenPlusModel.getModelName()).thenReturn("qwen-plus"); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder() + .register("dashscope:qwen-plus", qwenPlusModel) + .build(); + + Model result = provider.getModel("dashscope:qwen-plus"); + assertEquals(qwenPlusModel, result); + } + } + + @Nested + @DisplayName("Alias Resolution") + class AliasResolutionTests { + + @Test + @DisplayName("Should resolve alias to registered model") + void shouldResolveAlias() { + Model qwenTurboModel = mock(Model.class); + when(qwenTurboModel.getModelName()).thenReturn("qwen-turbo"); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder() + .register("qwen-turbo", qwenTurboModel) + .alias("qwen-turbo", "qwen-turbo") + .build(); + + Model result = provider.getModel("qwen-turbo"); + assertEquals(qwenTurboModel, result); + } + + @Test + @DisplayName("Should resolve multiple aliases to same model") + void shouldResolveMultipleAliases() { + Model qwenTurboModel = mock(Model.class); + when(qwenTurboModel.getModelName()).thenReturn("qwen-turbo"); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder() + .register("qwen-turbo", qwenTurboModel) + .alias("qwen-turbo", "qwen-turbo") + .alias("qwen-turbo-latest", "qwen-turbo") + .alias("fast-model", "qwen-turbo") + .build(); + + assertEquals(qwenTurboModel, provider.getModel("qwen-turbo")); + assertEquals(qwenTurboModel, provider.getModel("qwen-turbo-latest")); + assertEquals(qwenTurboModel, provider.getModel("fast-model")); + } + + @Test + @DisplayName("Should prefer direct match over alias") + void shouldPreferDirectMatchOverAlias() { + Model directModel = mock(Model.class); + when(directModel.getModelName()).thenReturn("direct"); + Model aliasTargetModel = mock(Model.class); + when(aliasTargetModel.getModelName()).thenReturn("alias-target"); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder() + .register("qwen-turbo", directModel) + .register("qwen-max", aliasTargetModel) + .alias("fast-model", "qwen-max") + .build(); + + // Direct registration should take precedence + Model result = provider.getModel("qwen-turbo"); + assertEquals(directModel, result); + } + } + + @Nested + @DisplayName("Default Model Fallback") + class DefaultModelFallbackTests { + + @Test + @DisplayName("Should return default model when not found") + void shouldReturnDefaultModelWhenNotFound() { + Model defaultModel = mock(Model.class); + when(defaultModel.getModelName()).thenReturn("default"); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder().defaultModel(defaultModel).build(); + + Model result = provider.getModel("unknown"); + assertEquals(defaultModel, result); + } + + @Test + @DisplayName("Should return null when no default and not found") + void shouldReturnNullWhenNoDefaultAndNotFound() { + MapBasedSkillModelProvider provider = MapBasedSkillModelProvider.builder().build(); + + Model result = provider.getModel("unknown"); + assertNull(result); + } + + @Test + @DisplayName("Should return default for null reference") + void shouldReturnDefaultForNullRef() { + Model defaultModel = mock(Model.class); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder().defaultModel(defaultModel).build(); + + assertEquals(defaultModel, provider.getModel(null)); + } + + @Test + @DisplayName("Should return default for empty reference") + void shouldReturnDefaultForEmptyRef() { + Model defaultModel = mock(Model.class); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder().defaultModel(defaultModel).build(); + + assertEquals(defaultModel, provider.getModel("")); + } + + @Test + @DisplayName("Should return default for blank reference") + void shouldReturnDefaultForBlankRef() { + Model defaultModel = mock(Model.class); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder().defaultModel(defaultModel).build(); + + assertEquals(defaultModel, provider.getModel(" ")); + } + } + + @Nested + @DisplayName("isAvailable Method") + class IsAvailableTests { + + @Test + @DisplayName("Should return true for registered model") + void shouldReturnTrueForRegisteredModel() { + Model model = mock(Model.class); + when(model.getModelName()).thenReturn("test-model"); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder().register("test", model).build(); + + assertTrue(provider.isAvailable("test")); + } + + @Test + @DisplayName("Should return true for aliased model") + void shouldReturnTrueForAliasedModel() { + Model model = mock(Model.class); + when(model.getModelName()).thenReturn("test-model"); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder() + .register("full-name", model) + .alias("short", "full-name") + .build(); + + assertTrue(provider.isAvailable("short")); + } + + @Test + @DisplayName("Should return false for unknown model without default") + void shouldReturnFalseForUnknownWithoutDefault() { + MapBasedSkillModelProvider provider = MapBasedSkillModelProvider.builder().build(); + + assertFalse(provider.isAvailable("unknown")); + } + + @Test + @DisplayName("Should return true for unknown model with default") + void shouldReturnTrueForUnknownWithDefault() { + Model defaultModel = mock(Model.class); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder().defaultModel(defaultModel).build(); + + assertTrue(provider.isAvailable("unknown")); + } + } + + @Nested + @DisplayName("registerAll Method") + class RegisterAllTests { + + @Test + @DisplayName("Should register multiple models at once") + void shouldRegisterMultipleModels() { + Model qwenTurboModel = mock(Model.class); + Model qwenPlusModel = mock(Model.class); + when(qwenTurboModel.getModelName()).thenReturn("qwen-turbo"); + when(qwenPlusModel.getModelName()).thenReturn("qwen-plus"); + + Map models = new HashMap<>(); + models.put("qwen-turbo", qwenTurboModel); + models.put("qwen-plus", qwenPlusModel); + + MapBasedSkillModelProvider provider = + MapBasedSkillModelProvider.builder().registerAll(models).build(); + + assertEquals(qwenTurboModel, provider.getModel("qwen-turbo")); + assertEquals(qwenPlusModel, provider.getModel("qwen-plus")); + } + } +} diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java new file mode 100644 index 000000000..cdeea1793 --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java @@ -0,0 +1,189 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.skill; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.agentscope.core.model.Model; +import io.agentscope.core.tool.Toolkit; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for SkillBox.SkillRegistration runtimeModel and sub-agent creation. + * + *

      These tests verify that skills with models automatically create SubAgentTools. + */ +@Tag("unit") +class SkillBoxRegistrationTest { + + private Toolkit toolkit; + private SkillBox skillBox; + private Model qwenTurboModel; + private Model qwenPlusModel; + + @BeforeEach + void setUp() { + toolkit = new Toolkit(); + qwenTurboModel = mock(Model.class); + when(qwenTurboModel.getModelName()).thenReturn("qwen-turbo"); + + qwenPlusModel = mock(Model.class); + when(qwenPlusModel.getModelName()).thenReturn("qwen-plus"); + + SkillModelProvider provider = + MapBasedSkillModelProvider.builder() + .register("qwen-turbo", qwenTurboModel) + .register("qwen-plus", qwenPlusModel) + .build(); + + skillBox = new SkillBox(toolkit, null, null, provider); + } + + @Nested + @DisplayName("Sub-Agent Tool Creation Tests") + class SubAgentToolCreationTests { + + @Test + @DisplayName("Should create sub-agent tool when skill has model") + void shouldCreateSubAgentToolWhenSkillHasModel() { + AgentSkill skill = + AgentSkill.builder() + .name("code_review") + .description("Code review skill") + .skillContent("Review code") + .model("qwen-turbo") + .build(); + + skillBox.registration().skill(skill).apply(); + + // Verify sub-agent tool was created + assertNotNull(toolkit.getTool("call_code_review")); + } + + @Test + @DisplayName("Should not create sub-agent tool when skill has no model") + void shouldNotCreateSubAgentToolWhenSkillHasNoModel() { + AgentSkill skill = + AgentSkill.builder() + .name("simple_skill") + .description("Simple skill") + .skillContent("Do something") + .build(); + + skillBox.registration().skill(skill).apply(); + + // No sub-agent tool should be created + assertNull(toolkit.getTool("call_simple_skill")); + } + + @Test + @DisplayName("Should use runtime model over skill model") + void shouldUseRuntimeModelOverSkillModel() { + AgentSkill skill = + AgentSkill.builder() + .name("code_review") + .description("Code review skill") + .skillContent("Review code") + .model("qwen-turbo") + .build(); + + skillBox.registration().skill(skill).runtimeModel("qwen-plus").apply(); + + // Tool should still be created + assertNotNull(toolkit.getTool("call_code_review")); + } + + @Test + @DisplayName("Should not create sub-agent when model not found") + void shouldNotCreateSubAgentWhenModelNotFound() { + AgentSkill skill = + AgentSkill.builder() + .name("unknown_model_skill") + .description("Skill with unknown model") + .skillContent("Content") + .model("unknown_model") + .build(); + + skillBox.registration().skill(skill).apply(); + + // Should not crash, just log warning + assertNull(toolkit.getTool("call_unknown_model_skill")); + } + + @Test + @DisplayName("Should not create sub-agent when no model provider") + void shouldNotCreateSubAgentWhenNoModelProvider() { + SkillBox boxWithoutProvider = new SkillBox(toolkit); + + AgentSkill skill = + AgentSkill.builder() + .name("code_review") + .description("Code review skill") + .skillContent("Review code") + .model("qwen-turbo") + .build(); + + boxWithoutProvider.registration().skill(skill).apply(); + + // Should not crash, just log warning + assertNull(toolkit.getTool("call_code_review")); + } + } + + @Nested + @DisplayName("Runtime Model Priority Tests") + class RuntimeModelPriorityTests { + + @Test + @DisplayName("Should create sub-agent with runtime model even when skill has no model") + void shouldCreateSubAgentWithRuntimeModelEvenWhenSkillHasNoModel() { + AgentSkill skill = + AgentSkill.builder() + .name("dynamic_skill") + .description("Dynamic skill") + .skillContent("Do something") + .build(); + + skillBox.registration().skill(skill).runtimeModel("qwen-turbo").apply(); + + // Tool should be created with runtime model + assertNotNull(toolkit.getTool("call_dynamic_skill")); + } + + @Test + @DisplayName("Should not create sub-agent when runtime model is blank") + void shouldNotCreateSubAgentWhenRuntimeModelIsBlank() { + AgentSkill skill = + AgentSkill.builder() + .name("blank_runtime_skill") + .description("Blank runtime skill") + .skillContent("Content") + .build(); + + skillBox.registration().skill(skill).runtimeModel(" ").apply(); + + // No tool should be created + assertNull(toolkit.getTool("call_blank_runtime_skill")); + } + } +} 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 e3f5a13db..a3480a2d8 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 @@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; @@ -28,6 +29,7 @@ import io.agentscope.core.message.TextBlock; import io.agentscope.core.message.ToolResultBlock; import io.agentscope.core.message.ToolUseBlock; +import io.agentscope.core.model.Model; import io.agentscope.core.tool.AgentTool; import io.agentscope.core.tool.Tool; import io.agentscope.core.tool.ToolCallParam; @@ -762,6 +764,43 @@ void testPreserveCustomValidator() { } } + @Nested + @DisplayName("ModelProvider Tests") + class ModelProviderTests { + @Test + @DisplayName("Should store model provider") + void shouldStoreModelProvider() { + Model defaultModel = mock(Model.class); + SkillModelProvider provider = + MapBasedSkillModelProvider.builder().defaultModel(defaultModel).build(); + + SkillBox skillBoxWithProvider = new SkillBox(toolkit, null, null, provider); + + assertSame(provider, skillBoxWithProvider.getModelProvider()); + } + + @Test + @DisplayName("Should allow setting model provider") + void shouldAllowSettingModelProvider() { + SkillBox skillBoxNoProvider = new SkillBox(toolkit); + Model defaultModel = mock(Model.class); + SkillModelProvider provider = + MapBasedSkillModelProvider.builder().defaultModel(defaultModel).build(); + + skillBoxNoProvider.setModelProvider(provider); + + assertSame(provider, skillBoxNoProvider.getModelProvider()); + } + + @Test + @DisplayName("Should return null model provider when not set") + void shouldReturnNullModelProviderWhenNotSet() { + SkillBox skillBoxNoProvider = new SkillBox(toolkit); + + assertNull(skillBoxNoProvider.getModelProvider()); + } + } + @Test @DisplayName("Should bind toolkit and propagate to SkillToolFactory") void testBindToolkitUpdatesSkillToolFactory() { diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillModelProviderTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillModelProviderTest.java new file mode 100644 index 000000000..8a3fd749e --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillModelProviderTest.java @@ -0,0 +1,66 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.skill; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.agentscope.core.model.Model; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +@DisplayName("SkillModelProvider Tests") +class SkillModelProviderTest { + + @Test + @DisplayName("Should return model from provider") + void shouldReturnModelFromProvider() { + Model qwenTurboModel = mock(Model.class); + when(qwenTurboModel.getModelName()).thenReturn("qwen-turbo"); + + SkillModelProvider provider = + modelRef -> "qwen-turbo".equals(modelRef) ? qwenTurboModel : null; + + Model result = provider.getModel("qwen-turbo"); + assertNotNull(result); + assertEquals("qwen-turbo", result.getModelName()); + } + + @Test + @DisplayName("Should return null when model not found") + void shouldReturnNullWhenModelNotFound() { + SkillModelProvider provider = modelRef -> null; + + Model result = provider.getModel("unknown"); + assertNull(result); + } + + @Test + @DisplayName("Should check availability") + void shouldCheckAvailability() { + Model qwenTurboModel = mock(Model.class); + SkillModelProvider provider = + modelRef -> "qwen-turbo".equals(modelRef) ? qwenTurboModel : null; + + assertTrue(provider.isAvailable("qwen-turbo")); + assertFalse(provider.isAvailable("unknown")); + } +} diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillSubagentPromptBuilderTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillSubagentPromptBuilderTest.java new file mode 100644 index 000000000..cf7716e81 --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillSubagentPromptBuilderTest.java @@ -0,0 +1,258 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.skill; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link SkillSubagentPromptBuilder}. + */ +@Tag("unit") +class SkillSubagentPromptBuilderTest { + + @Nested + @DisplayName("Builder Tests") + class BuilderTests { + + @Test + @DisplayName("Should build prompt with all skill information") + void shouldBuildPromptWithAllSkillInformation() { + AgentSkill skill = + AgentSkill.builder() + .name("code_review") + .description("Review code for quality and best practices") + .skillContent("# Code Review\nReview the code carefully...") + .model("qwen-turbo") + .build(); + + String prompt = + SkillSubagentPromptBuilder.builder() + .skill(skill) + .modelName("qwen-turbo") + .build(); + + assertNotNull(prompt); + assertTrue(prompt.contains("code_review")); + assertTrue(prompt.contains("Review code for quality and best practices")); + assertTrue(prompt.contains("# Code Review\nReview the code carefully...")); + assertTrue(prompt.contains("qwen-turbo")); + } + + @Test + @DisplayName("Should include role definition") + void shouldIncludeRoleDefinition() { + AgentSkill skill = + AgentSkill.builder() + .name("data_analysis") + .description("Analyze data") + .skillContent("Content") + .build(); + + String prompt = + SkillSubagentPromptBuilder.builder() + .skill(skill) + .modelName("qwen-plus") + .build(); + + assertTrue(prompt.contains("You are a specialized agent")); + assertTrue(prompt.contains("data_analysis")); + } + + @Test + @DisplayName("Should include guidelines section") + void shouldIncludeGuidelinesSection() { + AgentSkill skill = + AgentSkill.builder() + .name("test_skill") + .description("Test") + .skillContent("Content") + .build(); + + String prompt = + SkillSubagentPromptBuilder.builder().skill(skill).modelName("default").build(); + + assertTrue(prompt.contains("## Guidelines")); + assertTrue(prompt.contains("Focus ONLY on tasks related to this skill")); + } + + @Test + @DisplayName("Should include tool usage section") + void shouldIncludeToolUsageSection() { + AgentSkill skill = + AgentSkill.builder() + .name("test_skill") + .description("Test") + .skillContent("Content") + .build(); + + String prompt = + SkillSubagentPromptBuilder.builder().skill(skill).modelName("default").build(); + + assertTrue(prompt.contains("## Tool Usage")); + assertTrue(prompt.contains("toolkit")); + } + + @Test + @DisplayName("Should include constraints section") + void shouldIncludeConstraintsSection() { + AgentSkill skill = + AgentSkill.builder() + .name("test_skill") + .description("Test") + .skillContent("Content") + .build(); + + String prompt = + SkillSubagentPromptBuilder.builder().skill(skill).modelName("default").build(); + + assertTrue(prompt.contains("## Important Constraints")); + assertTrue(prompt.contains("Do not perform actions unrelated to the skill's purpose")); + } + + @Test + @DisplayName("Should throw exception when skill is not set") + void shouldThrowExceptionWhenSkillNotSet() { + assertThrows( + IllegalStateException.class, + () -> SkillSubagentPromptBuilder.builder().modelName("qwen-turbo").build()); + } + } + + @Nested + @DisplayName("Null Handling Tests") + class NullHandlingTests { + + @Test + @DisplayName("Should handle null skill name") + void shouldHandleNullSkillName() { + AgentSkill skill = + AgentSkill.builder() + .name("temp_skill") + .description("Test") + .skillContent("Content") + .build(); + + // Create skill and manually set name to null for testing + String prompt = + SkillSubagentPromptBuilder.builder() + .skill(skill) + .modelName("qwen-turbo") + .build(); + + assertTrue(prompt.contains("temp_skill")); + } + + @Test + @DisplayName("Should handle null model name") + void shouldHandleNullModelName() { + AgentSkill skill = + AgentSkill.builder() + .name("test_skill") + .description("Test description") + .skillContent("Test content") + .build(); + + String prompt = + SkillSubagentPromptBuilder.builder().skill(skill).modelName(null).build(); + + assertTrue(prompt.contains("default")); + } + + @Test + @DisplayName("Should handle empty model name") + void shouldHandleEmptyModelName() { + AgentSkill skill = + AgentSkill.builder() + .name("test_skill") + .description("Test description") + .skillContent("Test content") + .build(); + + String prompt = SkillSubagentPromptBuilder.builder().skill(skill).modelName("").build(); + + // Empty string should be preserved, but displayed as empty + assertNotNull(prompt); + } + } + + @Nested + @DisplayName("Template Structure Tests") + class TemplateStructureTests { + + @Test + @DisplayName("Should have all expected sections") + void shouldHaveAllExpectedSections() { + AgentSkill skill = + AgentSkill.builder() + .name("complete_skill") + .description("A complete skill for testing") + .skillContent("# Instructions\nDo something useful.") + .build(); + + String prompt = + SkillSubagentPromptBuilder.builder().skill(skill).modelName("qwen-max").build(); + + assertTrue(prompt.contains("## Your Purpose"), "Should have Purpose section"); + assertTrue(prompt.contains("## Your Instructions"), "Should have Instructions section"); + assertTrue(prompt.contains("## Guidelines"), "Should have Guidelines section"); + assertTrue(prompt.contains("## Tool Usage"), "Should have Tool Usage section"); + assertTrue( + prompt.contains("## Important Constraints"), "Should have Constraints section"); + } + + @Test + @DisplayName("Should properly format multiline skill content") + void shouldProperlyFormatMultilineSkillContent() { + String multilineContent = + """ + # Code Review Guidelines + + ## Key Areas to Review + 1. Code quality + 2. Performance + 3. Security + + ## Output Format + Return findings in JSON format. + """; + + AgentSkill skill = + AgentSkill.builder() + .name("code_review") + .description("Review code") + .skillContent(multilineContent) + .build(); + + String prompt = + SkillSubagentPromptBuilder.builder() + .skill(skill) + .modelName("qwen-turbo") + .build(); + + assertTrue(prompt.contains("# Code Review Guidelines")); + assertTrue(prompt.contains("## Key Areas to Review")); + assertTrue(prompt.contains("1. Code quality")); + assertTrue(prompt.contains("Return findings in JSON format")); + } + } +} diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillToolFactoryTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillToolFactoryTest.java new file mode 100644 index 000000000..73f18933d --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillToolFactoryTest.java @@ -0,0 +1,220 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.agentscope.core.skill; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.when; + +import io.agentscope.core.message.TextBlock; +import io.agentscope.core.message.ToolResultBlock; +import io.agentscope.core.tool.AgentTool; +import io.agentscope.core.tool.ToolCallParam; +import io.agentscope.core.tool.Toolkit; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class SkillToolFactoryTest { + + @Mock private SkillRegistry skillRegistry; + + @Mock private Toolkit toolkit; + + @Mock private SkillBox skillBox; + + private SkillToolFactory skillToolFactory; + + @BeforeEach + void setUp() { + skillToolFactory = new SkillToolFactory(skillRegistry, toolkit, skillBox); + } + + @Test + @DisplayName("Should include model info in response when skill has model") + void test_buildSkillMarkdownResponse_skillWithModel_includesModelInfo() { + // Arrange + AgentSkill skillWithModel = + new AgentSkill( + "code_review", + "Code review skill", + "Review code according to best practices", + null, + "custom", + "qwen-turbo"); + + String skillId = skillWithModel.getSkillId(); + RegisteredSkill registeredSkill = new RegisteredSkill(skillId); + + setupSkillRegistryMocks(skillId, skillWithModel, registeredSkill); + + // Act + AgentTool loadTool = skillToolFactory.createSkillAccessToolAgentTool(); + ToolResultBlock result = + loadTool.callAsync( + ToolCallParam.builder() + .input(Map.of("skillId", skillId, "path", "SKILL.md")) + .build()) + .block(); + + // Assert + assertNotNull(result); + String content = getTextContent(result); + assertTrue(content.contains("Model: qwen-turbo"), "Response should contain model info"); + assertTrue( + content.contains("**Note:**"), "Response should contain note about sub-agent tool"); + assertTrue( + content.contains("call_code_review"), + "Response should mention the sub-agent tool name"); + } + + @Test + @DisplayName("Should not include model info in response when skill has no model") + void test_buildSkillMarkdownResponse_skillWithoutModel_excludesModelInfo() { + // Arrange + AgentSkill skillWithoutModel = + new AgentSkill( + "simple_skill", "Simple skill", "Do something", null, "custom", null); + + String skillId = skillWithoutModel.getSkillId(); + RegisteredSkill registeredSkill = new RegisteredSkill(skillId); + + setupSkillRegistryMocks(skillId, skillWithoutModel, registeredSkill); + + // Act + AgentTool loadTool = skillToolFactory.createSkillAccessToolAgentTool(); + ToolResultBlock result = + loadTool.callAsync( + ToolCallParam.builder() + .input(Map.of("skillId", skillId, "path", "SKILL.md")) + .build()) + .block(); + + // Assert + assertNotNull(result); + String content = getTextContent(result); + assertFalse(content.contains("Model:"), "Response should NOT contain model info"); + assertFalse( + content.contains("**Note:**"), + "Response should NOT contain note about sub-agent tool"); + } + + @Test + @DisplayName("Should not include model info when model is empty string") + void test_buildSkillMarkdownResponse_emptyModel_excludesModelInfo() { + // Arrange + AgentSkill skillWithEmptyModel = + new AgentSkill( + "empty_model_skill", + "Skill with empty model", + "Do something", + null, + "custom", + ""); + + String skillId = skillWithEmptyModel.getSkillId(); + RegisteredSkill registeredSkill = new RegisteredSkill(skillId); + + setupSkillRegistryMocks(skillId, skillWithEmptyModel, registeredSkill); + + // Act + AgentTool loadTool = skillToolFactory.createSkillAccessToolAgentTool(); + ToolResultBlock result = + loadTool.callAsync( + ToolCallParam.builder() + .input(Map.of("skillId", skillId, "path", "SKILL.md")) + .build()) + .block(); + + // Assert + assertNotNull(result); + String content = getTextContent(result); + assertFalse( + content.contains("Model:"), + "Response should NOT contain model info for empty model"); + assertFalse( + content.contains("**Note:**"), "Response should NOT contain note for empty model"); + } + + @Test + @DisplayName("Should include correct tool name format in hint") + void test_buildSkillMarkdownResponse_modelHint_correctToolName() { + // Arrange + AgentSkill skill = + new AgentSkill( + "my_cool_skill", + "My cool skill", + "Cool instructions", + null, + "custom", + "qwen-plus"); + + String skillId = skill.getSkillId(); + RegisteredSkill registeredSkill = new RegisteredSkill(skillId); + + setupSkillRegistryMocks(skillId, skill, registeredSkill); + + // Act + AgentTool loadTool = skillToolFactory.createSkillAccessToolAgentTool(); + ToolResultBlock result = + loadTool.callAsync( + ToolCallParam.builder() + .input(Map.of("skillId", skillId, "path", "SKILL.md")) + .build()) + .block(); + + // Assert + assertNotNull(result); + String content = getTextContent(result); + assertTrue( + content.contains("call_my_cool_skill"), + "Response should contain correct tool name format"); + assertTrue( + content.contains("model 'qwen-plus'"), + "Response should mention the configured model"); + } + + /** Sets up common mocks for skill registry. */ + private void setupSkillRegistryMocks( + String skillId, AgentSkill skill, RegisteredSkill registeredSkill) { + when(skillRegistry.exists(skillId)).thenReturn(true); + when(skillRegistry.getSkill(skillId)).thenReturn(skill); + when(skillRegistry.getRegisteredSkill(skillId)).thenReturn(registeredSkill); + // Use lenient for stubbing that may not be invoked in all tests + lenient() + .when(skillRegistry.getAllRegisteredSkills()) + .thenReturn(Map.of(skillId, registeredSkill)); + lenient().when(toolkit.getToolGroup(any())).thenReturn(null); + } + + /** Extracts text content from ToolResultBlock output. */ + private String getTextContent(ToolResultBlock result) { + return result.getOutput().stream() + .filter(block -> block instanceof TextBlock) + .map(block -> ((TextBlock) block).getText()) + .findFirst() + .orElse(""); + } +} diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java index 7e2dd03e2..cf15ee7bb 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -222,6 +223,39 @@ void testCreateFromWithNumericMetadata() { assertEquals("123", skill.getName()); assertEquals("456", skill.getDescription()); } + + @Test + @DisplayName("Should create skill with model from markdown") + void shouldCreateSkillWithModelFromMarkdown() { + String skillMd = + "---\n" + + "name: test_skill\n" + + "description: Test skill\n" + + "model: haiku\n" + + "---\n" + + "# Content\n"; + + AgentSkill skill = SkillUtil.createFrom(skillMd, null); + + assertEquals("test_skill", skill.getName()); + assertEquals("haiku", skill.getModel()); + } + + @Test + @DisplayName("Should create skill without model when not specified") + void shouldCreateSkillWithoutModelWhenNotSpecified() { + String skillMd = + "---\n" + + "name: test_skill\n" + + "description: Test skill\n" + + "---\n" + + "# Content\n"; + + AgentSkill skill = SkillUtil.createFrom(skillMd, null); + + assertEquals("test_skill", skill.getName()); + assertNull(skill.getModel()); + } } @Nested diff --git a/agentscope-core/src/test/java/io/agentscope/core/tool/ToolkitTest.java b/agentscope-core/src/test/java/io/agentscope/core/tool/ToolkitTest.java index 6aad078f5..5a49d1076 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/tool/ToolkitTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/tool/ToolkitTest.java @@ -686,19 +686,19 @@ void testSetMultipleTypesTool() { .mcpClient(mcpClientWrapper) .agentTool(agentTool) .tool(testToolObject) - .subAgent(() -> mock(Agent.class)); + .subAgent(context -> mock(Agent.class)); toolkit.registration() .agentTool(agentTool) .mcpClient(mcpClientWrapper) .tool(testToolObject) - .subAgent(() -> mock(Agent.class)); + .subAgent(context -> mock(Agent.class)); toolkit.registration() .tool(testToolObject) .agentTool(agentTool) .mcpClient(mcpClientWrapper) - .subAgent(() -> mock(Agent.class)); + .subAgent(context -> mock(Agent.class)); toolkit.registration() - .subAgent(() -> mock(Agent.class)) + .subAgent(context -> mock(Agent.class)) .mcpClient(mcpClientWrapper) .agentTool(agentTool) .tool(testToolObject); @@ -764,7 +764,7 @@ void testSetValueThenResetToNull() { // Test 4: Set subAgent, then reset to null, should throw exception Toolkit.ToolRegistration registration4 = toolkit.registration(); - registration4.subAgent(() -> mock(Agent.class)).subAgent(null); + registration4.subAgent(context -> mock(Agent.class)).subAgent(null); IllegalStateException exception4 = assertThrows(IllegalStateException.class, () -> registration4.apply()); assertTrue( diff --git a/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/SubAgentToolTest.java b/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/SubAgentToolTest.java index 909badfd8..f71175937 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/SubAgentToolTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/SubAgentToolTest.java @@ -18,24 +18,33 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.agentscope.core.ReActAgent; import io.agentscope.core.agent.Agent; import io.agentscope.core.agent.Event; import io.agentscope.core.agent.EventType; import io.agentscope.core.agent.StreamOptions; +import io.agentscope.core.memory.InMemoryMemory; +import io.agentscope.core.memory.Memory; +import io.agentscope.core.message.ImageBlock; import io.agentscope.core.message.Msg; import io.agentscope.core.message.MsgRole; import io.agentscope.core.message.TextBlock; import io.agentscope.core.message.ToolResultBlock; import io.agentscope.core.message.ToolUseBlock; +import io.agentscope.core.message.URLSource; import io.agentscope.core.tool.ToolCallParam; import io.agentscope.core.tool.ToolEmitter; +import io.agentscope.core.tool.ToolExecutionContext; import io.agentscope.core.tool.Toolkit; import java.util.ArrayList; import java.util.HashMap; @@ -57,7 +66,7 @@ void testCreateWithDefaults() { // Create a mock agent Agent mockAgent = createMockAgent("TestAgent", "Test description"); - SubAgentTool tool = new SubAgentTool(() -> mockAgent, null); + SubAgentTool tool = new SubAgentTool(context -> mockAgent, null); assertEquals("call_testagent", tool.getName()); assertEquals("Test description", tool.getDescription()); @@ -69,7 +78,7 @@ void testCreateWithDefaults() { void testToolNameGeneration() { Agent mockAgent = createMockAgent("Research Agent", "Research tasks"); - SubAgentTool tool = new SubAgentTool(() -> mockAgent, null); + SubAgentTool tool = new SubAgentTool(context -> mockAgent, null); assertEquals("call_research_agent", tool.getName()); } @@ -85,7 +94,7 @@ void testCustomToolName() { .description("Custom description") .build(); - SubAgentTool tool = new SubAgentTool(() -> mockAgent, config); + SubAgentTool tool = new SubAgentTool(context -> mockAgent, config); assertEquals("custom_tool", tool.getName()); assertEquals("Custom description", tool.getDescription()); @@ -96,7 +105,7 @@ void testCustomToolName() { void testConversationSchema() { Agent mockAgent = createMockAgent("TestAgent", "Test"); - SubAgentTool tool = new SubAgentTool(() -> mockAgent, SubAgentConfig.defaults()); + SubAgentTool tool = new SubAgentTool(context -> mockAgent, SubAgentConfig.defaults()); Map schema = tool.getParameters(); assertEquals("object", schema.get("type")); @@ -118,7 +127,7 @@ void testConversationUsesSession() { AtomicInteger creationCount = new AtomicInteger(0); SubAgentProvider provider = - () -> { + context -> { creationCount.incrementAndGet(); Agent agent = mock(Agent.class); when(agent.getName()).thenReturn("TestAgent"); @@ -184,7 +193,8 @@ void testConversationExecution() { SubAgentTool tool = new SubAgentTool( - () -> mockAgent, SubAgentConfig.builder().forwardEvents(false).build()); + context -> mockAgent, + SubAgentConfig.builder().forwardEvents(false).build()); Map input = new HashMap<>(); input.put("message", "Hello"); @@ -201,13 +211,185 @@ void testConversationExecution() { assertTrue(text.contains("Hello there!")); } + @Test + @DisplayName("Should handle explicit null session_id and create new session") + void testNullSessionIdHandling() { + Agent mockAgent = mock(Agent.class); + when(mockAgent.getName()).thenReturn("TestAgent"); + when(mockAgent.getDescription()).thenReturn("Test agent"); + when(mockAgent.call(any(List.class))) + .thenReturn( + Mono.just( + Msg.builder() + .role(MsgRole.ASSISTANT) + .content( + TextBlock.builder() + .text("Response with null session") + .build()) + .build())); + + SubAgentTool tool = + new SubAgentTool( + context -> mockAgent, + SubAgentConfig.builder().forwardEvents(false).build()); + + // Explicitly pass null for session_id (simulating LLM behavior) + Map input = new HashMap<>(); + input.put("session_id", null); + input.put("message", "Hello"); + ToolUseBlock toolUse = + ToolUseBlock.builder().id("1").name("call_testagent").input(input).build(); + + ToolResultBlock result = + tool.callAsync(ToolCallParam.builder().toolUseBlock(toolUse).input(input).build()) + .block(); + + assertNotNull(result); + String text = extractText(result); + assertTrue(text.contains("session_id:")); + assertTrue(text.contains("Response with null session")); + // Should generate a new session_id when null is provided + String sessionId = extractSessionId(result); + assertNotNull(sessionId); + assertFalse(sessionId.isEmpty()); + } + + @Test + @DisplayName("Should inherit session ID from parent in SHARED mode") + void testSessionIdInheritanceInSharedMode() { + Agent mockAgent = mock(Agent.class); + when(mockAgent.getName()).thenReturn("TestAgent"); + when(mockAgent.getDescription()).thenReturn("Test agent"); + when(mockAgent.call(any(List.class))) + .thenReturn( + Mono.just( + Msg.builder() + .role(MsgRole.ASSISTANT) + .content(TextBlock.builder().text("Response").build()) + .build())); + + SubAgentTool tool = + new SubAgentTool( + context -> mockAgent, + SubAgentConfig.builder() + .forwardEvents(false) + .contextSharingMode(ContextSharingMode.SHARED) + .build()); + + // Create context with parent session ID + String parentSessionId = "parent_session_123"; + ToolExecutionContext toolContext = + ToolExecutionContext.builder() + .register(SubAgentTool.CONTEXT_KEY_PARENT_SESSION_ID, parentSessionId) + .build(); + + // Don't pass session_id - should inherit from parent + Map input = new HashMap<>(); + input.put("message", "Hello"); + ToolUseBlock toolUse = + ToolUseBlock.builder().id("1").name("call_testagent").input(input).build(); + + ToolResultBlock result = + tool.callAsync( + ToolCallParam.builder() + .toolUseBlock(toolUse) + .input(input) + .context(toolContext) + .build()) + .block(); + + assertNotNull(result); + String sessionId = extractSessionId(result); + assertNotNull(sessionId); + // Session ID should be derived from parent session + assertTrue( + sessionId.startsWith(parentSessionId + "_"), + "Session ID should be derived from parent: " + sessionId); + } + + @Test + @DisplayName("Should forward images from parent memory in SHARED mode") + void testImageForwardingInSharedMode() { + // Capture the message passed to the sub-agent + final List capturedMessages = new ArrayList<>(); + + Agent mockAgent = mock(Agent.class); + when(mockAgent.getName()).thenReturn("ImageAnalyzer"); + when(mockAgent.getDescription()).thenReturn("Image analysis agent"); + when(mockAgent.call(any(List.class))) + .thenAnswer( + invocation -> { + List msgs = invocation.getArgument(0); + capturedMessages.addAll(msgs); + return Mono.just( + Msg.builder() + .role(MsgRole.ASSISTANT) + .content( + TextBlock.builder() + .text("Image analyzed") + .build()) + .build()); + }); + + SubAgentTool tool = + new SubAgentTool( + context -> mockAgent, + SubAgentConfig.builder() + .forwardEvents(false) + .contextSharingMode(ContextSharingMode.SHARED) + .build()); + + // Create parent agent with memory containing an image + InMemoryMemory parentMemory = new InMemoryMemory(); + ImageBlock imageBlock = + ImageBlock.builder() + .source( + new io.agentscope.core.message.Base64Source( + "image/png", "base64imagedata")) + .build(); + Msg userMsgWithImage = + Msg.builder() + .role(MsgRole.USER) + .content(TextBlock.builder().text("Analyze this image").build()) + .content(imageBlock) + .build(); + parentMemory.addMessage(userMsgWithImage); + + // Use ReActAgent mock since getMemory() is defined there + io.agentscope.core.ReActAgent parentAgent = mock(io.agentscope.core.ReActAgent.class); + when(parentAgent.getMemory()).thenReturn(parentMemory); + + Map input = new HashMap<>(); + input.put("message", "Please analyze"); + ToolUseBlock toolUse = + ToolUseBlock.builder().id("1").name("call_imageanalyzer").input(input).build(); + + ToolResultBlock result = + tool.callAsync( + ToolCallParam.builder() + .toolUseBlock(toolUse) + .input(input) + .agent(parentAgent) + .build()) + .block(); + + assertNotNull(result); + assertFalse(capturedMessages.isEmpty()); + + // Verify the message sent to sub-agent contains the image + Msg sentMsg = capturedMessages.get(0); + List sentImages = sentMsg.getContentBlocks(ImageBlock.class); + assertFalse(sentImages.isEmpty(), "Image should be forwarded to sub-agent"); + assertEquals(1, sentImages.size()); + } + @Test @DisplayName("Should register sub-agent via Toolkit") void testToolkitRegistration() { Agent mockAgent = createMockAgent("HelperAgent", "A helpful agent"); Toolkit toolkit = new Toolkit(); - toolkit.registration().subAgent(() -> mockAgent).apply(); + toolkit.registration().subAgent(context -> mockAgent).apply(); assertNotNull(toolkit.getTool("call_helperagent")); assertEquals("A helpful agent", toolkit.getTool("call_helperagent").getDescription()); @@ -225,7 +407,7 @@ void testToolkitRegistrationWithConfig() { .build(); Toolkit toolkit = new Toolkit(); - toolkit.registration().subAgent(() -> mockAgent, config).apply(); + toolkit.registration().subAgent(context -> mockAgent, config).apply(); assertNotNull(toolkit.getTool("ask_expert")); assertEquals("Ask the expert a question", toolkit.getTool("ask_expert").getDescription()); @@ -238,7 +420,7 @@ void testToolkitRegistrationWithGroup() { Toolkit toolkit = new Toolkit(); toolkit.createToolGroup("workers", "Worker agents group", true); - toolkit.registration().subAgent(() -> mockAgent).group("workers").apply(); + toolkit.registration().subAgent(context -> mockAgent).group("workers").apply(); assertNotNull(toolkit.getTool("call_worker")); assertTrue(toolkit.getToolGroup("workers").getTools().contains("call_worker")); @@ -266,7 +448,7 @@ void testEventForwardingEnabled() { // Configure with forwardEvents=true (default) SubAgentConfig config = SubAgentConfig.builder().forwardEvents(true).build(); - SubAgentTool tool = new SubAgentTool(() -> mockAgent, config); + SubAgentTool tool = new SubAgentTool(context -> mockAgent, config); // Track emitted chunks List emittedChunks = new ArrayList<>(); @@ -311,7 +493,7 @@ void testEventForwardingDisabled() { // Configure with forwardEvents=false SubAgentConfig config = SubAgentConfig.builder().forwardEvents(false).build(); - SubAgentTool tool = new SubAgentTool(() -> mockAgent, config); + SubAgentTool tool = new SubAgentTool(context -> mockAgent, config); List emittedChunks = new ArrayList<>(); ToolEmitter testEmitter = emittedChunks::add; @@ -355,7 +537,7 @@ void testStreamingWithNoOpEmitter() { .thenReturn(Flux.just(event)); // forwardEvents=true by default, but no emitter provided - SubAgentTool tool = new SubAgentTool(() -> mockAgent, SubAgentConfig.defaults()); + SubAgentTool tool = new SubAgentTool(context -> mockAgent, SubAgentConfig.defaults()); Map input = new HashMap<>(); input.put("message", "Hello"); @@ -406,7 +588,7 @@ void testCustomStreamOptions() { assertEquals(customOptions, config.getStreamOptions()); - SubAgentTool tool = new SubAgentTool(() -> mockAgent, config); + SubAgentTool tool = new SubAgentTool(context -> mockAgent, config); List emittedChunks = new ArrayList<>(); ToolEmitter testEmitter = emittedChunks::add; @@ -428,6 +610,353 @@ void testCustomStreamOptions() { verify(mockAgent).stream(any(List.class), any(StreamOptions.class)); } + // Context Sharing Mode Tests + + @Test + @DisplayName("SubAgentConfig should have SHARED context mode by default") + void testDefaultContextSharingMode() { + SubAgentConfig config = SubAgentConfig.defaults(); + assertEquals(ContextSharingMode.SHARED, config.getContextSharingMode()); + } + + @Test + @DisplayName("Should share memory in SHARED mode") + void testSharedMemoryMode() { + // Create parent agent with memory + Memory parentMemory = new InMemoryMemory(); + parentMemory.addMessage( + Msg.builder() + .role(MsgRole.USER) + .content(TextBlock.builder().text("Parent message").build()) + .build()); + + ReActAgent parentAgent = mock(ReActAgent.class); + when(parentAgent.getName()).thenReturn("ParentAgent"); + when(parentAgent.getMemory()).thenReturn(parentMemory); + + // Create sub-agent + ReActAgent mockSubAgent = mock(ReActAgent.class); + when(mockSubAgent.getName()).thenReturn("SubAgent"); + when(mockSubAgent.getDescription()).thenReturn("Sub agent"); + when(mockSubAgent.getMemory()).thenReturn(parentMemory); // Returns shared memory + when(mockSubAgent.call(anyList())) + .thenAnswer( + invocation -> + Mono.just( + Msg.builder() + .role(MsgRole.ASSISTANT) + .content(TextBlock.builder().text("Done").build()) + .build())); + + // Create a context-aware provider that captures the memory passed in context + final Memory[] capturedMemory = new Memory[1]; + SubAgentProvider provider = + new SubAgentProvider<>() { + @Override + public ReActAgent provide() { + // Called by constructor to derive name/description + return mockSubAgent; + } + + @Override + public ReActAgent provideWithContext(SubAgentContext context) { + capturedMemory[0] = context.getMemoryToUse(); + return mockSubAgent; + } + }; + + SubAgentConfig config = + SubAgentConfig.builder() + .contextSharingMode(ContextSharingMode.SHARED) + .forwardEvents(false) + .build(); + + SubAgentTool tool = new SubAgentTool(provider, config); + + Map input = new HashMap<>(); + input.put("message", "Test"); + ToolUseBlock toolUse = + ToolUseBlock.builder().id("1").name("call_subagent").input(input).build(); + + tool.callAsync( + ToolCallParam.builder() + .toolUseBlock(toolUse) + .input(input) + .agent(parentAgent) + .build()) + .block(); + + // Verify the provider received a forked copy with same content (SHARED mode) + // Note: SHARED mode now forks the memory to remove pending tool calls, + // so it's a different instance but with the same message content + assertNotNull(capturedMemory[0]); + assertEquals(parentMemory.getMessages().size(), capturedMemory[0].getMessages().size()); + } + + @Test + @DisplayName("Should fork memory in FORK mode") + void testForkMemoryMode() { + // Create parent agent with memory + Memory parentMemory = new InMemoryMemory(); + parentMemory.addMessage( + Msg.builder() + .role(MsgRole.USER) + .content(TextBlock.builder().text("Parent message").build()) + .build()); + + ReActAgent parentAgent = mock(ReActAgent.class); + when(parentAgent.getName()).thenReturn("ParentAgent"); + when(parentAgent.getMemory()).thenReturn(parentMemory); + + // Create sub-agent + ReActAgent mockSubAgent = mock(ReActAgent.class); + when(mockSubAgent.getName()).thenReturn("SubAgent"); + when(mockSubAgent.getDescription()).thenReturn("Sub agent"); + when(mockSubAgent.call(anyList())) + .thenAnswer( + invocation -> + Mono.just( + Msg.builder() + .role(MsgRole.ASSISTANT) + .content(TextBlock.builder().text("Done").build()) + .build())); + + // Create a context-aware provider that captures the memory passed in context + final Memory[] capturedMemory = new Memory[1]; + SubAgentProvider provider = + new SubAgentProvider<>() { + @Override + public ReActAgent provide() { + // Called by constructor to derive name/description + return mockSubAgent; + } + + @Override + public ReActAgent provideWithContext(SubAgentContext context) { + capturedMemory[0] = context.getMemoryToUse(); + return mockSubAgent; + } + }; + + SubAgentConfig config = + SubAgentConfig.builder() + .contextSharingMode(ContextSharingMode.FORK) + .forwardEvents(false) + .build(); + + SubAgentTool tool = new SubAgentTool(provider, config); + + Map input = new HashMap<>(); + input.put("message", "Test"); + ToolUseBlock toolUse = + ToolUseBlock.builder().id("1").name("call_subagent").input(input).build(); + + tool.callAsync( + ToolCallParam.builder() + .toolUseBlock(toolUse) + .input(input) + .agent(parentAgent) + .build()) + .block(); + + // Verify the provider received a forked memory (not the same instance, but a copy) + assertNotNull(capturedMemory[0]); + assertNotSame(parentMemory, capturedMemory[0]); + assertEquals(1, capturedMemory[0].getMessages().size()); + + // Parent memory should not be modified + assertEquals(1, parentMemory.getMessages().size()); + } + + @Test + @DisplayName("Should use independent memory in NEW mode") + void testNewMemoryMode() { + // Create parent agent with memory + Memory parentMemory = new InMemoryMemory(); + parentMemory.addMessage( + Msg.builder() + .role(MsgRole.USER) + .content(TextBlock.builder().text("Parent message").build()) + .build()); + + ReActAgent parentAgent = mock(ReActAgent.class); + when(parentAgent.getName()).thenReturn("ParentAgent"); + when(parentAgent.getMemory()).thenReturn(parentMemory); + + // Create sub-agent + ReActAgent mockSubAgent = mock(ReActAgent.class); + when(mockSubAgent.getName()).thenReturn("SubAgent"); + when(mockSubAgent.getDescription()).thenReturn("Sub agent"); + when(mockSubAgent.call(anyList())) + .thenAnswer( + invocation -> + Mono.just( + Msg.builder() + .role(MsgRole.ASSISTANT) + .content(TextBlock.builder().text("Done").build()) + .build())); + + // Create a context-aware provider that captures the memory passed in context + final Memory[] capturedMemory = new Memory[1]; + SubAgentProvider provider = + new SubAgentProvider<>() { + @Override + public ReActAgent provide() { + // Called by constructor to derive name/description + return mockSubAgent; + } + + @Override + public ReActAgent provideWithContext(SubAgentContext context) { + capturedMemory[0] = context.getMemoryToUse(); + return mockSubAgent; + } + }; + + SubAgentConfig config = + SubAgentConfig.builder() + .contextSharingMode(ContextSharingMode.NEW) + .forwardEvents(false) + .build(); + + SubAgentTool tool = new SubAgentTool(provider, config); + + Map input = new HashMap<>(); + input.put("message", "Test"); + ToolUseBlock toolUse = + ToolUseBlock.builder().id("1").name("call_subagent").input(input).build(); + + tool.callAsync( + ToolCallParam.builder() + .toolUseBlock(toolUse) + .input(input) + .agent(parentAgent) + .build()) + .block(); + + // In NEW mode, the provider should receive null memory (indicating independent memory) + assertNull(capturedMemory[0]); + } + + @Test + @DisplayName("SHARED mode should share images in memory") + void testSharedMemoryWithImages() { + // Create parent agent with memory containing images + Memory parentMemory = new InMemoryMemory(); + Msg msgWithImage = + Msg.builder() + .role(MsgRole.USER) + .content( + List.of( + TextBlock.builder().text("Look at this").build(), + ImageBlock.builder() + .source( + URLSource.builder() + .url( + "https://example.com/photo.jpg") + .build()) + .build())) + .build(); + parentMemory.addMessage(msgWithImage); + + ReActAgent parentAgent = mock(ReActAgent.class); + when(parentAgent.getName()).thenReturn("ParentAgent"); + when(parentAgent.getMemory()).thenReturn(parentMemory); + + // Create sub-agent + ReActAgent mockSubAgent = mock(ReActAgent.class); + when(mockSubAgent.getName()).thenReturn("SubAgent"); + when(mockSubAgent.getDescription()).thenReturn("Sub agent"); + when(mockSubAgent.getMemory()).thenReturn(parentMemory); // Shared memory + when(mockSubAgent.call(anyList())) + .thenAnswer( + invocation -> + Mono.just( + Msg.builder() + .role(MsgRole.ASSISTANT) + .content( + TextBlock.builder() + .text("Analyzed") + .build()) + .build())); + + // Create a context-aware provider that captures the memory passed in context + final Memory[] capturedMemory = new Memory[1]; + SubAgentProvider provider = + new SubAgentProvider<>() { + @Override + public ReActAgent provide() { + // Called by constructor to derive name/description + return mockSubAgent; + } + + @Override + public ReActAgent provideWithContext(SubAgentContext context) { + capturedMemory[0] = context.getMemoryToUse(); + return mockSubAgent; + } + }; + + SubAgentConfig config = + SubAgentConfig.builder() + .contextSharingMode(ContextSharingMode.SHARED) + .forwardEvents(false) + .build(); + + SubAgentTool tool = new SubAgentTool(provider, config); + + Map input = new HashMap<>(); + input.put("message", "Analyze the image"); + ToolUseBlock toolUse = + ToolUseBlock.builder().id("1").name("call_subagent").input(input).build(); + + tool.callAsync( + ToolCallParam.builder() + .toolUseBlock(toolUse) + .input(input) + .agent(parentAgent) + .build()) + .block(); + + // Verify provider received a forked copy with the image content + // Note: SHARED mode now forks the memory to remove pending tool calls + assertNotNull(capturedMemory[0]); + assertEquals(parentMemory.getMessages().size(), capturedMemory[0].getMessages().size()); + } + + @Test + @DisplayName("Memory.fork() should create independent copy") + void testMemoryFork() { + Memory original = new InMemoryMemory(); + original.addMessage( + Msg.builder() + .role(MsgRole.USER) + .content(TextBlock.builder().text("Message 1").build()) + .build()); + original.addMessage( + Msg.builder() + .role(MsgRole.ASSISTANT) + .content(TextBlock.builder().text("Response 1").build()) + .build()); + + // Fork the memory + Memory forked = original.fork(); + + // Verify fork has same messages + assertEquals(2, forked.getMessages().size()); + assertEquals("Message 1", forked.getMessages().get(0).getTextContent()); + + // Add to fork should not affect original + forked.addMessage( + Msg.builder() + .role(MsgRole.USER) + .content(TextBlock.builder().text("Message 2").build()) + .build()); + + assertEquals(3, forked.getMessages().size()); + assertEquals(2, original.getMessages().size()); + } + // Helper methods private Agent createMockAgent(String name, String description) { diff --git a/agentscope-dependencies-bom/pom.xml b/agentscope-dependencies-bom/pom.xml index fb6856246..7f9c6086e 100644 --- a/agentscope-dependencies-bom/pom.xml +++ b/agentscope-dependencies-bom/pom.xml @@ -60,7 +60,7 @@ - 1.0.9 + 1.0.10-SNAPSHOT UTF-8 UTF-8 17 diff --git a/agentscope-distribution/agentscope-bom/pom.xml b/agentscope-distribution/agentscope-bom/pom.xml index 464200d93..32913d73e 100644 --- a/agentscope-distribution/agentscope-bom/pom.xml +++ b/agentscope-distribution/agentscope-bom/pom.xml @@ -59,7 +59,7 @@ - 1.0.9 + 1.0.10-SNAPSHOT UTF-8 UTF-8 17 diff --git a/agentscope-extensions/agentscope-extensions-autocontext-memory/src/main/java/io/agentscope/core/memory/autocontext/AutoContextMemory.java b/agentscope-extensions/agentscope-extensions-autocontext-memory/src/main/java/io/agentscope/core/memory/autocontext/AutoContextMemory.java index 32fb30c5e..41c0b6071 100644 --- a/agentscope-extensions/agentscope-extensions-autocontext-memory/src/main/java/io/agentscope/core/memory/autocontext/AutoContextMemory.java +++ b/agentscope-extensions/agentscope-extensions-autocontext-memory/src/main/java/io/agentscope/core/memory/autocontext/AutoContextMemory.java @@ -1818,6 +1818,47 @@ public List getCompressionEvents() { return compressionEvents; } + // ==================== Memory API ==================== + + /** + * Creates a fork of this memory with copies of all messages. + * + *

      The forked memory will have: + *

        + *
      • Copies of all messages from working memory storage
      • + *
      • Copies of all messages from original memory storage
      • + *
      • Copies of offload context
      • + *
      • Copies of compression events
      • + *
      • The same configuration and model reference
      • + *
      + * + *

      Changes to the forked memory will not affect the original memory and vice versa. + * This is used by the ContextSharingMode.FORK feature for sub-agent memory management. + * + * @return a new AutoContextMemory instance with copied messages + */ + @Override + public Memory fork() { + AutoContextMemory forked = new AutoContextMemory(autoContextConfig, model); + + // Copy working memory messages + forked.workingMemoryStorage = new ArrayList<>(this.workingMemoryStorage); + + // Copy original memory messages + forked.originalMemoryStorage = new ArrayList<>(this.originalMemoryStorage); + + // Copy offload context + forked.offloadContext = new HashMap<>(this.offloadContext); + + // Copy compression events + forked.compressionEvents = new ArrayList<>(this.compressionEvents); + + // Copy plan notebook reference (shared, not deep copied) + forked.planNotebook = this.planNotebook; + + return forked; + } + // ==================== StateModule API ==================== /** diff --git a/pom.xml b/pom.xml index 694995717..63e5af149 100644 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ AgentScope Java - Parent - 1.0.9 + 1.0.10-SNAPSHOT UTF-8 UTF-8 17 From 206717f55dbfd938e0da128c4c687d315bac9884 Mon Sep 17 00:00:00 2001 From: xun Date: Fri, 27 Feb 2026 19:42:17 +0800 Subject: [PATCH 2/5] refactor(skill): improve code quality and fix documentation issues - Remove redundant createSubAgentForSkill call in registerSkill() to prevent duplicate sub-agent creation with SkillRegistration.apply() - Add ContextSharingMode.fromString() factory method and consolidate parsing logic from SkillBox and SkillToolFactory (DRY principle) - Update SHARED mode documentation to accurately reflect fork-based implementation due to pending tool call validation constraints - Fix shallow copy of offloadContext in AutoContextMemory.fork() to use deep copy for proper memory isolation - Refactor duplicate createSubAgentIfHasModel code into shared helper method SkillBox.createSubAgentToolForSkill() to reduce ~200 lines of code duplication Co-Authored-By: Claude Opus 4.6 --- .../io/agentscope/core/skill/SkillBox.java | 379 ++++++------------ .../core/skill/SkillToolFactory.java | 173 +------- .../tool/subagent/ContextSharingMode.java | 67 +++- .../core/tool/subagent/SubAgentTool.java | 14 +- .../memory/autocontext/AutoContextMemory.java | 7 +- 5 files changed, 215 insertions(+), 425 deletions(-) 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 833200fb8..3d85d5881 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 @@ -281,125 +281,6 @@ public void registerSkill(AgentSkill skill) { skillRegistry.registerSkill(skillId, skill, registered); logger.info("Registered skill '{}'", skillId); - - // Create sub-agent tool if skill has model configured - createSubAgentForSkill(skill, skillId); - } - - /** - * Creates a SubAgentTool for a skill if it has a model configured. - * - *

      This method is called after skill registration to automatically create - * sub-agent tools for skills with model configuration. - * - * @param skill The skill to check for model configuration - * @param skillId The skill ID - */ - private void createSubAgentForSkill(AgentSkill skill, String skillId) { - String modelRef = skill.getModel(); - if (modelRef == null || modelRef.isBlank()) { - logger.debug( - "Skill '{}' has no model configured, skipping sub-agent creation", skillId); - return; - } - - if (toolkit == null) { - logger.warn( - "No toolkit configured for skill '{}', cannot create sub-agent with model '{}'", - skillId, - modelRef); - return; - } - - if (modelProvider == null) { - logger.warn( - "No SkillModelProvider configured for skill '{}', " - + "cannot create sub-agent with model '{}'", - skillId, - modelRef); - return; - } - - Model model = modelProvider.getModel(modelRef); - if (model == null) { - logger.warn( - "Model '{}' not found for skill '{}', skipping sub-agent creation", - modelRef, - skillId); - return; - } - - // Create tool group if needed - String skillToolGroup = skillId + "_skill_tools"; - if (toolkit.getToolGroup(skillToolGroup) == null) { - toolkit.createToolGroup(skillToolGroup, skillToolGroup, false); - } - - // Build system prompt using SkillSubagentPromptBuilder - final Model resolvedModel = model; - final Toolkit toolkitCopy = toolkit.copy(); - final String systemPrompt = - SkillSubagentPromptBuilder.builder() - .skill(skill) - .modelName(resolvedModel.getModelName()) - .build(); - - // Parse context sharing mode from skill - final ContextSharingMode contextMode = parseContextSharingMode(skill.getContext()); - - // Create SubAgentProvider - context-aware for memory sharing - SubAgentProvider provider = - new SubAgentProvider<>() { - @Override - public ReActAgent provideWithContext(SubAgentContext context) { - ReActAgent.Builder agentBuilder = - ReActAgent.builder() - .name(skill.getName() + "_agent") - .description(skill.getDescription()) - .model(resolvedModel) - .toolkit(toolkitCopy); - - // Check if context provides a memory to use (SHARED or FORK mode) - Memory memoryToUse = context.getMemoryToUse(); - if (memoryToUse != null) { - // Use the provided memory (shared or forked from parent) - agentBuilder.memory(memoryToUse); - } else { - // No memory provided - use independent memory with our system - // prompt - agentBuilder.sysPrompt(systemPrompt).memory(new InMemoryMemory()); - } - - return agentBuilder.build(); - } - }; - - // Register sub-agent tool - String toolName = "call_" + skill.getName(); - toolkit.registration() - .group(skillToolGroup) - .subAgent( - provider, - SubAgentConfig.builder() - .toolName(toolName) - .description( - "Execute " - + skill.getName() - + " skill task using model '" - + model.getModelName() - + "'") - .contextSharingMode(contextMode) - .build()) - .apply(); - - // Activate the tool group - toolkit.updateToolGroups(List.of(skillToolGroup), true); - - logger.info( - "Created sub-agent tool '{}' for skill '{}' with model '{}'", - toolName, - skillId, - model.getModelName()); } /** @@ -757,148 +638,154 @@ private String resolveEffectiveModel() { */ private void createSubAgentIfHasModel() { String effectiveModel = resolveEffectiveModel(); + Toolkit effectiveToolkit = toolkit != null ? toolkit : skillBox.toolkit; + skillBox.createSubAgentToolForSkill(skill, effectiveToolkit, effectiveModel); + } + } + + // ==================== SubAgent Creation Helper ==================== + + /** + * Creates a SubAgentTool for a skill with the specified model. + * + *

      This is a shared helper method used by both {@link SkillRegistration} and {@link + * SkillToolFactory} to create sub-agent tools for skills with model configuration. + * + *

      The method handles: + * + *

        + *
      • Resolving the model from the model provider + *
      • Creating the tool group if needed + *
      • Building the system prompt + *
      • Creating the SubAgentProvider with context-aware memory handling + *
      • Registering the sub-agent tool + *
      + * + * @param skill The skill to create sub-agent for + * @param effectiveToolkit The toolkit to register the tool with + * @param modelRef The model reference to resolve + * @return true if sub-agent was created successfully, false otherwise + */ + boolean createSubAgentToolForSkill( + AgentSkill skill, Toolkit effectiveToolkit, String modelRef) { + if (modelRef == null || modelRef.isBlank()) { logger.debug( - "createSubAgentIfHasModel called for skill '{}', effectiveModel='{}', " - + "toolkit={}, modelProvider={}", + "Skill '{}' has no model configured, skipping sub-agent creation", + skill.getName()); + return false; + } + + if (effectiveToolkit == null) { + logger.warn( + "No toolkit configured for skill '{}', cannot create sub-agent with model '{}'", skill.getName(), - effectiveModel, - skillBox.toolkit != null ? "present" : "null", - skillBox.modelProvider != null ? "present" : "null"); - - if (effectiveModel == null || effectiveModel.isBlank()) { - logger.debug( - "Skill '{}' has no model configured, skipping sub-agent creation", - skill.getName()); - return; // No model specified - } + modelRef); + return false; + } - if (skillBox.toolkit == null) { - logger.warn( - "No toolkit configured for skill '{}', cannot create sub-agent with model" - + " '{}'", - skill.getName(), - effectiveModel); - return; - } + if (modelProvider == null) { + logger.warn( + "No SkillModelProvider configured for skill '{}', " + + "cannot create sub-agent with model '{}'", + skill.getName(), + modelRef); + return false; + } - if (skillBox.modelProvider == null) { - logger.warn( - "No SkillModelProvider configured for skill '{}', " - + "cannot create sub-agent with model '{}'", - skill.getName(), - effectiveModel); - return; - } + Model model = modelProvider.getModel(modelRef); + if (model == null) { + logger.warn( + "Model '{}' not found for skill '{}', skipping sub-agent creation", + modelRef, + skill.getName()); + return false; + } - Model model = skillBox.modelProvider.getModel(effectiveModel); - if (model == null) { - logger.warn( - "Model '{}' not found for skill '{}', skipping sub-agent creation", - effectiveModel, - skill.getName()); - return; - } + // Create tool group if needed + String skillToolGroup = skill.getSkillId() + "_skill_tools"; + if (effectiveToolkit.getToolGroup(skillToolGroup) == null) { + effectiveToolkit.createToolGroup(skillToolGroup, skillToolGroup, false); + } - // Use the same toolkit reference as skillBox - Toolkit effectiveToolkit = toolkit != null ? toolkit : skillBox.toolkit; + // Build system prompt using SkillSubagentPromptBuilder + final Model resolvedModel = model; + final Toolkit toolkitCopy = effectiveToolkit.copy(); + final String systemPrompt = + SkillSubagentPromptBuilder.builder() + .skill(skill) + .modelName(resolvedModel.getModelName()) + .build(); - // Create SubAgentProvider with structured system prompt - final Model resolvedModel = model; - final Toolkit toolkitCopy = effectiveToolkit.copy(); - final String systemPrompt = - SkillSubagentPromptBuilder.builder() - .skill(skill) - .modelName(resolvedModel.getModelName()) - .build(); - - // Parse context sharing mode from skill - final ContextSharingMode contextMode = parseContextSharingMode(skill.getContext()); - - // Create SubAgentProvider - context-aware for memory sharing - SubAgentProvider provider = - new SubAgentProvider<>() { - @Override - public ReActAgent provideWithContext(SubAgentContext context) { - ReActAgent.Builder agentBuilder = - ReActAgent.builder() - .name(skill.getName() + "_agent") - .description(skill.getDescription()) - .model(resolvedModel) - .toolkit(toolkitCopy); - - // Check if context provides a memory to use (SHARED or FORK mode) - Memory memoryToUse = context.getMemoryToUse(); - if (memoryToUse != null) { - // Use the provided memory (shared or forked from parent) - agentBuilder.memory(memoryToUse); - } else { - // No memory provided - use independent memory with our system - // prompt - agentBuilder.sysPrompt(systemPrompt).memory(new InMemoryMemory()); - } - - return agentBuilder.build(); - } - }; - - // Create tool group if needed - String skillToolGroup = skill.getSkillId() + "_skill_tools"; - if (effectiveToolkit.getToolGroup(skillToolGroup) == null) { - effectiveToolkit.createToolGroup(skillToolGroup, skillToolGroup, false); - } + // Parse context sharing mode from skill + final ContextSharingMode contextMode = ContextSharingMode.fromString(skill.getContext()); - // Register sub-agent tool - effectiveToolkit - .registration() - .group(skillToolGroup) - .subAgent( - provider, - SubAgentConfig.builder() - .toolName("call_" + skill.getName()) - .description( - "Execute " - + skill.getName() - + " skill task using configured model") - .contextSharingMode(contextMode) - .build()) - .apply(); + // Create SubAgentProvider - context-aware for memory sharing + SubAgentProvider provider = + createSubAgentProvider(skill, resolvedModel, toolkitCopy, systemPrompt); - logger.info( - "Created sub-agent tool 'call_{}' for skill '{}' with model '{}'", - skill.getName(), - skill.getName(), - model.getModelName()); - } + // Register sub-agent tool + String toolName = "call_" + skill.getName(); + effectiveToolkit + .registration() + .group(skillToolGroup) + .subAgent( + provider, + SubAgentConfig.builder() + .toolName(toolName) + .description( + "Execute " + + skill.getName() + + " skill task using configured model") + .contextSharingMode(contextMode) + .build()) + .apply(); + + logger.info( + "Created sub-agent tool '{}' for skill '{}' with model '{}'", + toolName, + skill.getName(), + model.getModelName()); + + return true; } /** - * Parses the context sharing mode from skill's context field. + * Creates a SubAgentProvider for the given skill configuration. * - *

      Supported values: + *

      This method creates a context-aware provider that handles memory sharing based on the + * context sharing mode. * - *

        - *
      • null, empty, "shared" - SHARED (default) - *
      • "fork" - FORK - *
      • "new" - NEW - *
      - * - * @param context The context string from skill - * @return The corresponding ContextSharingMode + * @param skill The skill to create provider for + * @param resolvedModel The resolved model to use + * @param toolkitCopy A copy of the toolkit for the sub-agent + * @param systemPrompt The system prompt for NEW mode + * @return A SubAgentProvider that creates ReActAgent instances */ - private static ContextSharingMode parseContextSharingMode(String context) { - if (context == null || context.isEmpty() || "shared".equalsIgnoreCase(context)) { - return ContextSharingMode.SHARED; - } else if ("fork".equalsIgnoreCase(context)) { - return ContextSharingMode.FORK; - } else if ("new".equalsIgnoreCase(context)) { - return ContextSharingMode.NEW; - } else { - logger.warn( - "Unknown context mode '{}', defaulting to SHARED. " - + "Supported values: shared, fork, new", - context); - return ContextSharingMode.SHARED; - } + private SubAgentProvider createSubAgentProvider( + AgentSkill skill, Model resolvedModel, Toolkit toolkitCopy, String systemPrompt) { + return new SubAgentProvider<>() { + @Override + public ReActAgent provideWithContext(SubAgentContext context) { + ReActAgent.Builder agentBuilder = + ReActAgent.builder() + .name(skill.getName() + "_agent") + .description(skill.getDescription()) + .model(resolvedModel) + .toolkit(toolkitCopy); + + // Check if context provides a memory to use (SHARED or FORK mode) + Memory memoryToUse = context.getMemoryToUse(); + if (memoryToUse != null) { + // Use the provided memory (shared or forked from parent) + agentBuilder.memory(memoryToUse); + } else { + // No memory provided - use independent memory with our system prompt + agentBuilder.sysPrompt(systemPrompt).memory(new InMemoryMemory()); + } + + return agentBuilder.build(); + } + }; } // ==================== Skill Build-In Tools ==================== diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillToolFactory.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillToolFactory.java index d4082f6a3..a9a9e6781 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillToolFactory.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillToolFactory.java @@ -15,18 +15,10 @@ */ package io.agentscope.core.skill; -import io.agentscope.core.ReActAgent; -import io.agentscope.core.memory.InMemoryMemory; -import io.agentscope.core.memory.Memory; import io.agentscope.core.message.ToolResultBlock; -import io.agentscope.core.model.Model; import io.agentscope.core.tool.AgentTool; import io.agentscope.core.tool.ToolCallParam; import io.agentscope.core.tool.Toolkit; -import io.agentscope.core.tool.subagent.ContextSharingMode; -import io.agentscope.core.tool.subagent.SubAgentConfig; -import io.agentscope.core.tool.subagent.SubAgentContext; -import io.agentscope.core.tool.subagent.SubAgentProvider; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -332,171 +324,24 @@ private AgentSkill validatedActiveSkill(String skillId) { * @param skillId The skill ID for tracking creation status */ private void createSubAgentIfHasModel(AgentSkill skill, String skillId) { - // Check if skill has a model configured - String modelRef = skill.getModel(); - logger.debug( - "createSubAgentIfHasModel called for skill '{}', modelRef='{}', " - + "toolkit={}, skillBox={}, modelProvider={}", - skill.getName(), - modelRef, - toolkit != null ? "present" : "null", - skillBox != null ? "present" : "null", - skillBox != null && skillBox.getModelProvider() != null ? "present" : "null"); - - if (modelRef == null || modelRef.isBlank()) { - logger.debug( - "Skill '{}' has no model configured, skipping sub-agent creation", - skill.getName()); - return; // No model specified - } - // Check if sub-agent tool already created for this skill if (skillsWithSubAgentCreated.contains(skillId)) { logger.debug("Sub-agent tool already exists for skill '{}'", skillId); return; } - // Check prerequisites - if (toolkit == null) { - logger.warn( - "No toolkit available for skill '{}', cannot create sub-agent with model '{}'", - skill.getName(), - modelRef); - return; - } - - if (skillBox == null || skillBox.getModelProvider() == null) { - logger.warn( - "No SkillModelProvider configured for skill '{}', " - + "cannot create sub-agent with model '{}'", - skill.getName(), - modelRef); - return; - } - - // Resolve model - Model model = skillBox.getModelProvider().getModel(modelRef); - if (model == null) { - logger.warn( - "Model '{}' not found for skill '{}', skipping sub-agent creation", - modelRef, - skill.getName()); - return; - } + String modelRef = skill.getModel(); - // Create tool group if needed - String skillToolGroup = skillId + "_skill_tools"; - if (toolkit.getToolGroup(skillToolGroup) == null) { - toolkit.createToolGroup(skillToolGroup, skillToolGroup, false); - } + // Use shared method from SkillBox to create the sub-agent + boolean created = skillBox.createSubAgentToolForSkill(skill, toolkit, modelRef); - // Parse context sharing mode from skill - ContextSharingMode contextMode = parseContextSharingMode(skill.getContext()); - - // Build system prompt using SkillSubagentPromptBuilder (only used for NEW mode) - final Model resolvedModel = model; - final Toolkit toolkitCopy = toolkit.copy(); - final String systemPrompt = - SkillSubagentPromptBuilder.builder() - .skill(skill) - .modelName(resolvedModel.getModelName()) - .build(); - - // Create SubAgentProvider - context-aware for memory sharing - // For SHARED and FORK modes, the context will contain the memory to use - // For NEW mode, the context will have null memory, so we use our own - SubAgentProvider provider = - new SubAgentProvider<>() { - @Override - public ReActAgent provideWithContext(SubAgentContext context) { - ReActAgent.Builder agentBuilder = - ReActAgent.builder() - .name(skill.getName() + "_agent") - .description(skill.getDescription()) - .model(resolvedModel) - .toolkit(toolkitCopy); - - // Check if context provides a memory to use (SHARED or FORK mode) - Memory memoryToUse = context.getMemoryToUse(); - if (memoryToUse != null) { - // Use the provided memory (shared or forked from parent) - agentBuilder.memory(memoryToUse); - logger.debug( - "Sub-agent '{}' using {} memory from context", - skill.getName(), - context.getContextSharingMode()); - } else { - // No memory provided - use independent memory with our system prompt - // This is the NEW mode case - agentBuilder.sysPrompt(systemPrompt).memory(new InMemoryMemory()); - logger.debug( - "Sub-agent '{}' using independent memory with skill system" - + " prompt", - skill.getName()); - } - - return agentBuilder.build(); - } - }; - - // Register sub-agent tool with context sharing mode from skill - String toolName = "call_" + skill.getName(); - toolkit.registration() - .group(skillToolGroup) - .subAgent( - provider, - SubAgentConfig.builder() - .toolName(toolName) - .description( - "Execute " - + skill.getName() - + " skill task using model '" - + model.getModelName() - + "'") - .contextSharingMode(contextMode) - .build()) - .apply(); - - // Mark as created - skillsWithSubAgentCreated.add(skillId); - - // Activate the tool group - toolkit.updateToolGroups(List.of(skillToolGroup), true); - - logger.info( - "Created sub-agent tool '{}' for skill '{}' with model '{}' (dynamic loading)", - toolName, - skill.getName(), - model.getModelName()); - } + if (created) { + // Mark as created to prevent duplicates + skillsWithSubAgentCreated.add(skillId); - /** - * Parses the context sharing mode from skill's context field. - * - *

      Supported values: - * - *

        - *
      • null, empty, "shared" - SHARED (default) - *
      • "fork" - FORK - *
      • "new" - NEW - *
      - * - * @param context The context string from skill - * @return The corresponding ContextSharingMode - */ - private ContextSharingMode parseContextSharingMode(String context) { - if (context == null || context.isEmpty() || "shared".equalsIgnoreCase(context)) { - return ContextSharingMode.SHARED; - } else if ("fork".equalsIgnoreCase(context)) { - return ContextSharingMode.FORK; - } else if ("new".equalsIgnoreCase(context)) { - return ContextSharingMode.NEW; - } else { - logger.warn( - "Unknown context mode '{}', defaulting to SHARED. " - + "Supported values: shared, fork, new", - context); - return ContextSharingMode.SHARED; + // Activate the tool group + String skillToolGroup = skillId + "_skill_tools"; + toolkit.updateToolGroups(List.of(skillToolGroup), true); } } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/ContextSharingMode.java b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/ContextSharingMode.java index 32869a34a..69ce8e94a 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/ContextSharingMode.java +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/ContextSharingMode.java @@ -15,28 +15,44 @@ */ package io.agentscope.core.tool.subagent; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * Mode for sharing memory context between parent agent and sub-agent. * *

      This enum defines how memory is shared between the parent agent and sub-agent: * *

        - *
      • shared (default): SubAgent shares the same memory instance with parent. All messages - * are immediately visible to both agents. SubAgent uses parent's system prompt. - *
      • fork: SubAgent gets a copy (fork) of parent's memory at invocation time. SubAgent's - * execution doesn't affect parent's memory. SubAgent uses parent's system prompt. + *
      • shared (default): SubAgent receives a forked copy of parent's memory at invocation + * time, with pending tool calls removed. The sub-agent can see the conversation context but + * changes don't affect parent's memory. SubAgent uses parent's system prompt. This provides + * context visibility while avoiding validation issues with pending tool calls. + *
      • fork: SubAgent gets a copy (fork) of parent's memory at invocation time, with + * pending tool calls removed. SubAgent's execution doesn't affect parent's memory. SubAgent + * uses parent's system prompt. *
      • new: SubAgent has completely independent memory with its own system prompt. No * context from parent. *
      * + *

      Implementation Note: Both SHARED and FORK modes use forked memory copies because the + * parent's memory contains the pending tool_use block that invoked the sub-agent. Directly sharing + * this memory would cause validation errors when the sub-agent tries to add new messages. The + * pending tool calls are removed from the forked copy to ensure proper message sequence. + * *

      Aligned with skill specification: https://code.claude.com/docs/zh-CN/skills */ public enum ContextSharingMode { /** * Shared memory mode (default). * - *

      SubAgent uses the same memory instance as the parent agent. All messages are immediately - * visible to both. SubAgent uses parent's system prompt context. + *

      SubAgent receives a forked copy of parent's memory at invocation time, with pending tool + * calls removed. This provides the sub-agent with full conversation context visibility while + * ensuring isolation - changes made by the sub-agent don't affect the parent's memory. + * + *

      Note: Despite the name "shared", this mode uses a forked copy for technical + * reasons. The parent's memory cannot be directly shared because it contains the pending + * tool_use block that invoked this sub-agent, which would cause validation errors. * *

      This is the default mode when context is not specified in skill.md. */ @@ -45,8 +61,9 @@ public enum ContextSharingMode { /** * Fork memory mode. * - *

      SubAgent gets a copy (fork) of parent's memory at invocation time. Changes made by - * SubAgent don't affect parent's memory. SubAgent uses parent's system prompt context. + *

      SubAgent gets a copy (fork) of parent's memory at invocation time, with pending tool calls + * removed. Changes made by SubAgent don't affect parent's memory. SubAgent uses parent's system + * prompt context. * *

      Use this when you need context awareness but want isolation from parent's memory. */ @@ -60,5 +77,37 @@ public enum ContextSharingMode { * *

      Use this for isolated tasks that don't need parent context. */ - NEW + NEW; + + private static final Logger logger = LoggerFactory.getLogger(ContextSharingMode.class); + + /** + * Parses the context sharing mode from a string value. + * + *

      Supported values: + * + *

        + *
      • null, empty, "shared" - SHARED (default) + *
      • "fork" - FORK + *
      • "new" - NEW + *
      + * + * @param context The context string to parse + * @return The corresponding ContextSharingMode, defaults to SHARED for unknown values + */ + public static ContextSharingMode fromString(String context) { + if (context == null || context.isEmpty() || "shared".equalsIgnoreCase(context)) { + return SHARED; + } else if ("fork".equalsIgnoreCase(context)) { + return FORK; + } else if ("new".equalsIgnoreCase(context)) { + return NEW; + } else { + logger.warn( + "Unknown context mode '{}', defaulting to SHARED. " + + "Supported values: shared, fork, new", + context); + return SHARED; + } + } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java index 18cc59c78..cd0e363a1 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java @@ -61,13 +61,19 @@ *

      Context Sharing Modes: * *

        - *
      • SHARED (default): Sub-agent shares the same memory instance with parent. All - * messages are immediately visible to both. Sub-agent uses parent's system prompt context. - *
      • FORK: Sub-agent gets a copy of parent's memory at invocation time. Changes don't - * affect parent's memory. Sub-agent uses parent's system prompt context. + *
      • SHARED (default): Sub-agent receives a forked copy of parent's memory with pending + * tool calls removed. Provides context visibility while avoiding validation issues. Changes + * don't affect parent's memory. Sub-agent uses parent's system prompt context. + *
      • FORK: Sub-agent gets a forked copy of parent's memory at invocation time, with + * pending tool calls removed. Changes don't affect parent's memory. Sub-agent uses parent's + * system prompt context. *
      • NEW: Sub-agent has completely independent memory with its own system prompt. No * context from parent. *
      + * + *

      Note: Both SHARED and FORK modes fork the parent's memory because the parent's memory + * contains the pending tool_use block that invoked the sub-agent, which would cause validation + * errors if shared directly. */ public class SubAgentTool implements AgentTool { diff --git a/agentscope-extensions/agentscope-extensions-autocontext-memory/src/main/java/io/agentscope/core/memory/autocontext/AutoContextMemory.java b/agentscope-extensions/agentscope-extensions-autocontext-memory/src/main/java/io/agentscope/core/memory/autocontext/AutoContextMemory.java index 41c0b6071..11502b1a0 100644 --- a/agentscope-extensions/agentscope-extensions-autocontext-memory/src/main/java/io/agentscope/core/memory/autocontext/AutoContextMemory.java +++ b/agentscope-extensions/agentscope-extensions-autocontext-memory/src/main/java/io/agentscope/core/memory/autocontext/AutoContextMemory.java @@ -1847,8 +1847,11 @@ public Memory fork() { // Copy original memory messages forked.originalMemoryStorage = new ArrayList<>(this.originalMemoryStorage); - // Copy offload context - forked.offloadContext = new HashMap<>(this.offloadContext); + // Copy offload context (deep copy to ensure true isolation) + forked.offloadContext = new HashMap<>(); + for (Map.Entry> entry : this.offloadContext.entrySet()) { + forked.offloadContext.put(entry.getKey(), new ArrayList<>(entry.getValue())); + } // Copy compression events forked.compressionEvents = new ArrayList<>(this.compressionEvents); From 1724f01d23fcbcd21bdca5eab962624fa1d305dd Mon Sep 17 00:00:00 2001 From: xun Date: Sat, 28 Feb 2026 13:13:12 +0800 Subject: [PATCH 3/5] refactor(skill): add tests for ContextSharingMode.fromString() and coverage improvements --- .../core/skill/SkillBoxRegistrationTest.java | 60 ++++++++++++++ .../tool/subagent/ContextSharingModeTest.java | 71 +++++++++++++++++ .../autocontext/AutoContextMemoryTest.java | 79 +++++++++++++++++++ 3 files changed, 210 insertions(+) create mode 100644 agentscope-core/src/test/java/io/agentscope/core/tool/subagent/ContextSharingModeTest.java diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java index cdeea1793..122a0bd89 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java @@ -186,4 +186,64 @@ void shouldNotCreateSubAgentWhenRuntimeModelIsBlank() { assertNull(toolkit.getTool("call_blank_runtime_skill")); } } + + @Nested + @DisplayName("createSubAgentToolForSkill Tests") + class CreateSubAgentToolForSkillTests { + + @Test + @DisplayName("Should return false when modelRef is null") + void shouldReturnFalseWhenModelRefIsNull() { + AgentSkill skill = + AgentSkill.builder().name("test").description("Test").skillContent("Content").build(); + + boolean result = skillBox.createSubAgentToolForSkill(skill, toolkit, null); + + assertNull(toolkit.getTool("call_test")); + } + + @Test + @DisplayName("Should return false when modelRef is blank") + void shouldReturnFalseWhenModelRefIsBlank() { + AgentSkill skill = + AgentSkill.builder().name("test").description("Test").skillContent("Content").build(); + + boolean result = skillBox.createSubAgentToolForSkill(skill, toolkit, " "); + + assertNull(toolkit.getTool("call_test")); + } + + @Test + @DisplayName("Should return false when toolkit is null") + void shouldReturnFalseWhenToolkitIsNull() { + AgentSkill skill = + AgentSkill.builder() + .name("test") + .description("Test") + .skillContent("Content") + .model("test-model") + .build(); + + boolean result = skillBox.createSubAgentToolForSkill(skill, null, "test-model"); + + assertNull(toolkit.getTool("call_test")); + } + + @Test + @DisplayName("Should return false when no model provider configured") + void shouldReturnFalseWhenNoModelProvider() { + SkillBox boxWithoutProvider = new SkillBox(toolkit); + AgentSkill skill = + AgentSkill.builder() + .name("test") + .description("Test") + .skillContent("Content") + .model("test-model") + .build(); + + boolean result = boxWithoutProvider.createSubAgentToolForSkill(skill, toolkit, "test-model"); + + assertNull(toolkit.getTool("call_test")); + } + } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/ContextSharingModeTest.java b/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/ContextSharingModeTest.java new file mode 100644 index 000000000..4ee03856d --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/ContextSharingModeTest.java @@ -0,0 +1,71 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.tool.subagent; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Unit tests for ContextSharingMode enum. + */ +@DisplayName("ContextSharingMode Tests") +class ContextSharingModeTest { + + @Test + @DisplayName("fromString should return SHARED for 'shared' (case insensitive)") + void testFromStringShared() { + assertSame(ContextSharingMode.SHARED, ContextSharingMode.fromString("shared")); + assertSame(ContextSharingMode.SHARED, ContextSharingMode.fromString("SHARED")); + assertSame(ContextSharingMode.SHARED, ContextSharingMode.fromString("Shared")); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {" ", "shared"}) + @DisplayName("fromString should return SHARED for null, empty, or 'shared'") + void testFromStringSharedDefault(String value) { + assertSame(ContextSharingMode.SHARED, ContextSharingMode.fromString(value)); + } + + @Test + @DisplayName("fromString should return FORK for 'fork' (case insensitive)") + void testFromStringFork() { + assertSame(ContextSharingMode.FORK, ContextSharingMode.fromString("fork")); + assertSame(ContextSharingMode.FORK, ContextSharingMode.fromString("FORK")); + assertSame(ContextSharingMode.FORK, ContextSharingMode.fromString("Fork")); + } + + @Test + @DisplayName("fromString should return NEW for 'new' (case insensitive)") + void testFromStringNew() { + assertSame(ContextSharingMode.NEW, ContextSharingMode.fromString("new")); + assertSame(ContextSharingMode.NEW, ContextSharingMode.fromString("NEW")); + assertSame(ContextSharingMode.NEW, ContextSharingMode.fromString("New")); + } + + @ParameterizedTest + @ValueSource(strings = {"unknown", "invalid", "random", "SHARED_MODE"}) + @DisplayName("fromString should return SHARED (default) for unknown values") + void testFromStringUnknownValues(String value) { + assertSame(ContextSharingMode.SHARED, ContextSharingMode.fromString(value)); + } +} diff --git a/agentscope-extensions/agentscope-extensions-autocontext-memory/src/test/java/io/agentscope/core/memory/autocontext/AutoContextMemoryTest.java b/agentscope-extensions/agentscope-extensions-autocontext-memory/src/test/java/io/agentscope/core/memory/autocontext/AutoContextMemoryTest.java index 4b74e9f66..bb06d122a 100644 --- a/agentscope-extensions/agentscope-extensions-autocontext-memory/src/test/java/io/agentscope/core/memory/autocontext/AutoContextMemoryTest.java +++ b/agentscope-extensions/agentscope-extensions-autocontext-memory/src/test/java/io/agentscope/core/memory/autocontext/AutoContextMemoryTest.java @@ -1677,4 +1677,83 @@ void testGetPlanStateContextWithDifferentPlanStates() throws Exception { resultDone.contains("Goal: Test Description"), "Should contain goal for DONE state"); } + + // ==================== Fork Tests ==================== + + @Test + @DisplayName("Fork should create deep copy of offloadContext") + void testForkCreatesDeepCopyOfOffloadContext() { + // Add messages to trigger compression and create offload context + for (int i = 0; i < 15; i++) { + memory.addMessage(createTextMessage("Message " + i, MsgRole.USER)); + } + + // Trigger compression to create offload context entries + memory.compressIfNeeded(); + + // Get original offload context reference for comparison + Map> originalOffloadContext = memory.getOffloadContext(); + + // Only proceed if we have offload context (compression occurred) + if (originalOffloadContext.isEmpty()) { + // Force offload by adding large content + memory.offload("test-uuid", List.of(createTextMessage("Offloaded content", MsgRole.USER))); + originalOffloadContext = memory.getOffloadContext(); + } + + // Create fork + AutoContextMemory forked = (AutoContextMemory) memory.fork(); + assertNotNull(forked, "Forked memory should not be null"); + + // Get forked offload context + Map> forkedOffloadContext = forked.getOffloadContext(); + + // Verify the maps are different instances (shallow copy check) + assertNotSame(originalOffloadContext, forkedOffloadContext, "Offload context maps should be different instances"); + + // Verify the map contents are deep copied (list instances are different) + for (String key : originalOffloadContext.keySet()) { + List originalList = originalOffloadContext.get(key); + List forkedList = forkedOffloadContext.get(key); + assertNotSame(originalList, forkedList, "List for key " + key + " should be different instances"); + assertEquals(originalList.size(), forkedList.size(), "List sizes should match for key " + key); + } + + // Verify isolation: modifying forked offload context doesn't affect original + String testKey = "isolation-test-key"; + forked.offload(testKey, List.of(createTextMessage("New message", MsgRole.USER))); + assertFalse(originalOffloadContext.containsKey(testKey), "Original should not contain new key added to forked"); + assertTrue(forked.getOffloadContext().containsKey(testKey), "Forked should contain new key"); + + // Verify isolation: modifying list in forked doesn't affect original + if (!originalOffloadContext.isEmpty()) { + String firstKey = originalOffloadContext.keySet().iterator().next(); + int originalSize = originalOffloadContext.get(firstKey).size(); + forkedOffloadContext.get(firstKey).add(createTextMessage("Extra message", MsgRole.USER)); + assertEquals( + originalSize, + originalOffloadContext.get(firstKey).size(), + "Original list should not be affected by modification to forked list"); + } + } + + @Test + @DisplayName("Fork should copy working and original memory storage") + void testForkCopiesMemoryStorage() { + // Add messages + memory.addMessage(createTextMessage("Message 1", MsgRole.USER)); + memory.addMessage(createTextMessage("Message 2", MsgRole.ASSISTANT)); + + // Create fork + AutoContextMemory forked = (AutoContextMemory) memory.fork(); + + // Verify messages are copied + assertEquals(2, forked.getMessages().size(), "Forked should have same number of messages"); + assertEquals(2, forked.getOriginalMemoryMsgs().size(), "Forked original storage should have same messages"); + + // Verify isolation - adding to forked doesn't affect original + forked.addMessage(createTextMessage("Message 3", MsgRole.USER)); + assertEquals(2, memory.getMessages().size(), "Original should still have 2 messages"); + assertEquals(3, forked.getMessages().size(), "Forked should have 3 messages"); + } } From 75b44467ca25ba88b2364bd0ab24f6fc61bc6d29 Mon Sep 17 00:00:00 2001 From: xun Date: Sat, 28 Feb 2026 13:28:22 +0800 Subject: [PATCH 4/5] test: add missing import and fix test compilation - Add assertNotSame import to AutoContextMemoryTest Co-Authored-By: Claude Opus 4.6 --- .../core/skill/SkillBoxRegistrationTest.java | 15 ++++++-- .../tool/subagent/ContextSharingModeTest.java | 1 - .../autocontext/AutoContextMemoryTest.java | 35 ++++++++++++++----- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java index 122a0bd89..79b85ad2a 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxRegistrationTest.java @@ -195,7 +195,11 @@ class CreateSubAgentToolForSkillTests { @DisplayName("Should return false when modelRef is null") void shouldReturnFalseWhenModelRefIsNull() { AgentSkill skill = - AgentSkill.builder().name("test").description("Test").skillContent("Content").build(); + AgentSkill.builder() + .name("test") + .description("Test") + .skillContent("Content") + .build(); boolean result = skillBox.createSubAgentToolForSkill(skill, toolkit, null); @@ -206,7 +210,11 @@ void shouldReturnFalseWhenModelRefIsNull() { @DisplayName("Should return false when modelRef is blank") void shouldReturnFalseWhenModelRefIsBlank() { AgentSkill skill = - AgentSkill.builder().name("test").description("Test").skillContent("Content").build(); + AgentSkill.builder() + .name("test") + .description("Test") + .skillContent("Content") + .build(); boolean result = skillBox.createSubAgentToolForSkill(skill, toolkit, " "); @@ -241,7 +249,8 @@ void shouldReturnFalseWhenNoModelProvider() { .model("test-model") .build(); - boolean result = boxWithoutProvider.createSubAgentToolForSkill(skill, toolkit, "test-model"); + boolean result = + boxWithoutProvider.createSubAgentToolForSkill(skill, toolkit, "test-model"); assertNull(toolkit.getTool("call_test")); } diff --git a/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/ContextSharingModeTest.java b/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/ContextSharingModeTest.java index 4ee03856d..f2ff9d979 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/ContextSharingModeTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/tool/subagent/ContextSharingModeTest.java @@ -15,7 +15,6 @@ */ package io.agentscope.core.tool.subagent; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import org.junit.jupiter.api.DisplayName; diff --git a/agentscope-extensions/agentscope-extensions-autocontext-memory/src/test/java/io/agentscope/core/memory/autocontext/AutoContextMemoryTest.java b/agentscope-extensions/agentscope-extensions-autocontext-memory/src/test/java/io/agentscope/core/memory/autocontext/AutoContextMemoryTest.java index bb06d122a..ea2b20256 100644 --- a/agentscope-extensions/agentscope-extensions-autocontext-memory/src/test/java/io/agentscope/core/memory/autocontext/AutoContextMemoryTest.java +++ b/agentscope-extensions/agentscope-extensions-autocontext-memory/src/test/java/io/agentscope/core/memory/autocontext/AutoContextMemoryTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -1697,7 +1698,8 @@ void testForkCreatesDeepCopyOfOffloadContext() { // Only proceed if we have offload context (compression occurred) if (originalOffloadContext.isEmpty()) { // Force offload by adding large content - memory.offload("test-uuid", List.of(createTextMessage("Offloaded content", MsgRole.USER))); + memory.offload( + "test-uuid", List.of(createTextMessage("Offloaded content", MsgRole.USER))); originalOffloadContext = memory.getOffloadContext(); } @@ -1709,27 +1711,41 @@ void testForkCreatesDeepCopyOfOffloadContext() { Map> forkedOffloadContext = forked.getOffloadContext(); // Verify the maps are different instances (shallow copy check) - assertNotSame(originalOffloadContext, forkedOffloadContext, "Offload context maps should be different instances"); + assertNotSame( + originalOffloadContext, + forkedOffloadContext, + "Offload context maps should be different instances"); // Verify the map contents are deep copied (list instances are different) for (String key : originalOffloadContext.keySet()) { List originalList = originalOffloadContext.get(key); List forkedList = forkedOffloadContext.get(key); - assertNotSame(originalList, forkedList, "List for key " + key + " should be different instances"); - assertEquals(originalList.size(), forkedList.size(), "List sizes should match for key " + key); + assertNotSame( + originalList, + forkedList, + "List for key " + key + " should be different instances"); + assertEquals( + originalList.size(), + forkedList.size(), + "List sizes should match for key " + key); } // Verify isolation: modifying forked offload context doesn't affect original String testKey = "isolation-test-key"; forked.offload(testKey, List.of(createTextMessage("New message", MsgRole.USER))); - assertFalse(originalOffloadContext.containsKey(testKey), "Original should not contain new key added to forked"); - assertTrue(forked.getOffloadContext().containsKey(testKey), "Forked should contain new key"); + assertFalse( + originalOffloadContext.containsKey(testKey), + "Original should not contain new key added to forked"); + assertTrue( + forked.getOffloadContext().containsKey(testKey), "Forked should contain new key"); // Verify isolation: modifying list in forked doesn't affect original if (!originalOffloadContext.isEmpty()) { String firstKey = originalOffloadContext.keySet().iterator().next(); int originalSize = originalOffloadContext.get(firstKey).size(); - forkedOffloadContext.get(firstKey).add(createTextMessage("Extra message", MsgRole.USER)); + forkedOffloadContext + .get(firstKey) + .add(createTextMessage("Extra message", MsgRole.USER)); assertEquals( originalSize, originalOffloadContext.get(firstKey).size(), @@ -1749,7 +1765,10 @@ void testForkCopiesMemoryStorage() { // Verify messages are copied assertEquals(2, forked.getMessages().size(), "Forked should have same number of messages"); - assertEquals(2, forked.getOriginalMemoryMsgs().size(), "Forked original storage should have same messages"); + assertEquals( + 2, + forked.getOriginalMemoryMsgs().size(), + "Forked original storage should have same messages"); // Verify isolation - adding to forked doesn't affect original forked.addMessage(createTextMessage("Message 3", MsgRole.USER)); From 7c6faef48e96b729303a3dc06a6436000688e1fe Mon Sep 17 00:00:00 2001 From: xun Date: Sat, 28 Feb 2026 13:41:03 +0800 Subject: [PATCH 5/5] docs(subagent): add design note explaining SHARED vs FORK implementation - Clarify why SHARED and FORK have identical implementations - Explain technical limitation: cannot directly share memory due to pending tool calls - Document rationale for keeping both modes despite identical behavior Co-Authored-By: Claude Opus 4.6 --- .../core/tool/subagent/SubAgentTool.java | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java index cd0e363a1..8e26dc4cc 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/subagent/SubAgentTool.java @@ -297,11 +297,22 @@ private Memory computeMemoryToUse(Agent parentAgent, ContextSharingMode contextM switch (contextMode) { case SHARED: - // Fork parent's memory and remove pending tool calls - // Note: We cannot directly share parent's memory because it contains - // the pending tool call to this sub-agent, which would cause - // validation errors when the sub-agent tries to add new messages. - // We use a forked copy with pending tool calls removed. + // DESIGN NOTE: SHARED and FORK currently have identical implementations. + // Both fork the parent's memory and remove pending tool calls. + // + // Why we cannot implement true memory sharing: + // - The parent's memory contains the pending tool_use block that invoked this + // sub-agent + // - Directly sharing this memory would cause validation errors when the sub-agent + // tries to add new messages (pending tool calls must be resolved first) + // - True sharing would require complex synchronization and state management + // + // Why we keep SHARED as a separate mode: + // - API compatibility with skill.md specifications + // - Semantic distinction: SHARED implies "context visibility" while FORK implies + // "explicit copy" + // - Future extensibility: if we find a way to implement true sharing, SHARED can + // be updated Memory sharedMemory = parentMemory.fork(); removePendingToolCalls(sharedMemory); logger.debug( @@ -312,6 +323,8 @@ private Memory computeMemoryToUse(Agent parentAgent, ContextSharingMode contextM case FORK: // Fork parent's memory and remove pending tool calls + // NOTE: This is identical to SHARED mode implementation. See SHARED case for + // explanation. Memory forkedMemory = parentMemory.fork(); removePendingToolCalls(forkedMemory); logger.debug(