fix issue #1738 problem 1

[xuhuafeifei]

AgentScope-Java Version

[INFO] io.agentscope:agentscope-parent:pom:2.0.0-SNAPSHOT

Description

Background

Agent calls skill_manage create to create a skill, then immediately calls skill_manage write_file in the same reasoning turn to write resource files, resulting in "Skill not found" error

Solution

Optimize the usage pattern to avoid "create-then-immediately-write" multiple operations in the same reasoning turn:

Add resources parameter to SkillManageTool.create for atomic writing:

• SKILL.md + all resource files written in a single operation
• Eliminates need for subsequent write_file calls, fundamentally avoiding consistency issues
• Underlying WorkspaceSkillRepository.save() already supports multi-file atomic writing, just wasn't exposed in the tool API

Checklist

Please check the following items before code is ready to be reviewed.

• [*] Code has been formatted with mvn spotless:apply
• [*] All tests are passing (mvn test)
• [*] Javadoc comments are complete and follow project conventions
• [*] Related documentation has been updated (e.g. links, examples, etc.)
• [*] Code is ready for review

[codecov]

Codecov Report

❌ Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...agentscope/harness/agent/tool/SkillManageTool.java	95.83%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[AgentScopeJavaBot]

🤖 AI Review

This PR adds a resources parameter to the SkillManageTool.create action, enabling atomic creation of a skill with all its resource files in a single tool call. This directly addresses the race condition described in issue #1738 where an agent calls create then immediately write_file in the same reasoning turn and encounters "Skill not found" errors. The implementation is clean — it leverages the existing SkillUtil.createFrom(content, resources, source) and WorkspaceSkillRepository.writeSkill() infrastructure that already supports multi-file atomic writes. A comprehensive set of new tests (209 lines) covers the happy path, edge cases (null/empty resources, invalid paths, multiple resource types), and draft-mode behavior. The code follows existing patterns consistently.

[AgentScopeJavaBot]

[minor] mapOfStrings silently drops entries whose key or value is not a String. If an LLM passes a non-string value (e.g., {"scripts/count.sh": 42}), that resource is omitted without any error or warning — the tool returns success and the agent assumes the file was created. Consider logging a warning when an entry is skipped, so the issue surfaces in diagnostics:

if (entry.getKey() instanceof String && entry.getValue() instanceof String) {
    result.put((String) entry.getKey(), (String) entry.getValue());
} else {
    log.warn("mapOfStrings: skipping non-String entry key={}", entry.getKey());
}

[AgentScopeJavaBot]

[nit] The JSON schema for resources declares "type": "object" but omits additionalProperties. Adding "additionalProperties", Map.of("type", "string") would give the LLM a stronger hint that values must be strings, and would enable schema-level validation on the tool-call side. Not blocking — the description already conveys the intent clearly.

[AgentScopeJavaBot]

🤖 AI Review

This PR adds a resources parameter to the SkillManageTool.create action, enabling atomic creation of a skill with all its resource files in a single tool call. This directly addresses the race condition described in issue #1738 where an agent calls create then immediately write_file in the same reasoning turn and encounters "Skill not found" errors. The implementation is clean — it leverages the existing SkillUtil.createFrom(content, resources, source) and WorkspaceSkillRepository.writeSkill() infrastructure that already supports multi-file atomic writes. A comprehensive set of new tests (209 lines) covers the happy path, edge cases (null/empty resources, invalid paths, multiple resource types), and draft-mode behavior. The code follows existing patterns consistently.

[AgentScopeJavaBot]

[minor] mapOfStrings silently drops entries whose key or value is not a String. If an LLM passes a non-string value (e.g., {"scripts/count.sh": 42}), that resource is omitted without any error or warning — the tool returns success and the agent assumes the file was created. Consider logging a warning when an entry is skipped, so the issue surfaces in diagnostics:

if (entry.getKey() instanceof String && entry.getValue() instanceof String) {
    result.put((String) entry.getKey(), (String) entry.getValue());
} else {
    log.warn("mapOfStrings: skipping non-String entry key={}", entry.getKey());
}

[xuhuafeifei]

I will fix it

[AgentScopeJavaBot]

[nit] The JSON schema for resources declares "type": "object" but omits additionalProperties. Adding "additionalProperties", Map.of("type", "string") would give the LLM a stronger hint that values must be strings, and would enable schema-level validation on the tool-call side. Not blocking — the description already conveys the intent clearly.