fix(#2441): use AgentSkillRepository instead of WorkspaceSkillRepository in SkillManageTool [FaaFyfxR9WAQrL7FcAgEHJvztd8cVMxvjHRS55rw1nwH] - #2445
Conversation
…File to AgentSkillRepository interface [FaaFyfxR9WAQrL7FcAgEHJvztd8cVMxvjHRS55rw1nwH]
…eSkillRepository in SkillManageTool [FaaFyfxR9WAQrL7FcAgEHJvztd8cVMxvjHRS55rw1nwH]
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
oss-maintainer
left a comment
There was a problem hiding this comment.
Code Review Summary
Overall: The change correctly addresses #2441 by replacing WorkspaceSkillRepository with AgentSkillRepository in SkillManageTool, and adds three new default methods (readSkillFile, writeSkillFile, deleteSkillFile) to the interface for future extensibility.
CI Status: ❌ Build failed on ubuntu-latest and windows-latest
Issue: Spotless formatting violation — import order is incorrect in SkillManageTool.java.
[ERROR] src/main/java/io/agentscope/harness/agent/tool/SkillManageTool.java
[ERROR] @@ -18,10 +18,10 @@
[ERROR] import·io.agentscope.core.skill.AgentSkill;
[ERROR] +import·io.agentscope.core.skill.repository.AgentSkillRepository;
[ERROR] import·io.agentscope.core.skill.util.SkillUtil;
[ERROR] import·io.agentscope.core.tool.AgentTool;
[ERROR] -import·io.agentscope.core.skill.repository.AgentSkillRepository;
Fix: Run mvn spotless:apply locally and commit the formatted result.
Code Quality:
- ✅ Correct abstraction: Using
AgentSkillRepositoryinstead of the concreteWorkspaceSkillRepositoryallows for database-backed implementations - ✅ Default methods: The three new interface methods have sensible defaults (
null/false) so existing implementations won't break - ✅ Javadoc: Well-documented with clear parameter descriptions and return value semantics
Action Required: Fix the import ordering and re-run CI.
|
这样实现是否问题?看看是否应该复用AgentSkillRepository的save delete方法? |
Problem
SkillManageToolis tightly coupled toWorkspaceSkillRepository, which is a filesystem-specific implementation. This means users who configure a database-backed repository (e.g.MysqlSkillRepository,PostgresSkillRepository) cannot use theskill_managetool — the tool is hardwired to local file operations.Root Cause
The
SkillManageToolclass declares its two repository fields asWorkspaceSkillRepository:This prevents the tool from being used with any
AgentSkillRepositoryimplementation that is not filesystem-based.Changes
AgentSkillRepositoryinterface — Added three default methods:readSkillFile(String skillName, String relPath)— returnsnullby defaultwriteSkillFile(String skillName, String relPath, String content)— returnsfalseby defaultdeleteSkillFile(String skillName, String relPath)— returnsfalseby defaultThese methods already exist on
WorkspaceSkillRepositoryand are used bySkillManageToolfor sub-file operations (patch, write_file, remove_file). The default implementations allow other repository types to opt in or gracefully decline.SkillManageTool— Changed field types and constructor parameter types fromWorkspaceSkillRepositorytoAgentSkillRepository:private final WorkspaceSkillRepository mainRepo→private final AgentSkillRepository mainRepoprivate final WorkspaceSkillRepository draftsRepo→private final AgentSkillRepository draftsRepoAgentSkillRepositorylocate()method return type updatedCompatibility
WorkspaceSkillRepositoryimplementsAgentSkillRepository(viaRuntimeContextSkillRepository), so all existing code continues to work without any changes toHarnessAgentorProposeSkillTool.SkillManageTooldirectly. Sub-file operations (patch, write_file, remove_file) will return errors for repositories that don't support them, while the core create/edit/delete operations work natively.Closes #2441