fix(skill): Remove Windows reserved characters from FileSystemSkillRepository source (#947)#1031
Merged
LearningGp merged 7 commits intoagentscope-ai:mainfrom Mar 26, 2026
Conversation
|
|
|
黄新传 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
huangxinchuan
force-pushed
the
fix/issue-947-windows-path
branch
from
820548c to
31452b1
Compare
…pository source Fixes agentscope-ai#947 The buildDefaultSourceSuffix() method in FileSystemSkillRepository generated source strings containing Windows reserved characters (like : from C:\ drive), causing InvalidPathException on Windows. This fix removes all Windows reserved characters (\/:*?\"<>|) from the source suffix to ensure Windows path compatibility.
huangxinchuan
force-pushed
the
fix/issue-947-windows-path
branch
from
2b78e0b to
24d3a0d
Compare
Contributor
Author
|
@cla-assistant recheck I have updated the commit author from company email to my personal GitHub email (87484734@qq.com). The old commits with "黄新传 643160@ky-tech.com.cn" have been force-pushed and replaced. Thanks! |
Contributor
Author
|
Hi @AlbumenJ and team, This PR is ready for review. It fixes the Windows path compatibility issue reported in #947. Summary of changes:
Verification:
The fix ensures that skill repository sources don't contain Windows reserved characters, which previously caused Please let me know if you need any changes or have questions. Thanks for your time! |
huangxinchuan
force-pushed
the
fix/issue-947-windows-path
branch
2 times, most recently
from
57eea67 to
db8dad5
Compare
…y source Related to agentscope-ai#947 GitSkillRepository also generates source strings with Windows reserved characters (like 'git:owner/repo' containing colon). This commit: 1. Changes default source format from 'git:' to 'git-' prefix 2. Adds regex filter to remove Windows reserved characters from branch names 3. Updates tests to reflect the new format This ensures consistency with FileSystemSkillRepository fix.
huangxinchuan
force-pushed
the
fix/issue-947-windows-path
branch
from
db8dad5 to
009655b
Compare
Fix code style to comply with Spotless check requirements.
- Relax assertions in testClone_RepositoryNotFound to handle various CI environment error messages - Strengthen testClone_InvalidUrl assertions to verify non-empty message - Add more error pattern matches for network-related failures This improves test reliability across different CI environments with varying network conditions.
Add System.out.println statements to print actual vs expected source values. This will help identify the discrepancy in CI environment.
…ard slash The regex pattern for removing Windows reserved characters incorrectly included forward slash (/), which caused owner/repo format to become ownerrepo. Changes: 1. Remove forward slash from the regex pattern in buildDefaultSource() 2. Keep forward slash as it's valid in source identifiers (owner/repo format) 3. Improve comments to clarify why forward slash is preserved 4. Minor improvement to extractRepositoryIdentifier() for clarity Fixes test failure: expected <git-test/repo> but was <git-testrepo> Related to agentscope-ai#947
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Collaborator
|
@fang-tech PTAL |
fang-tech
suggested changes
Mar 24, 2026
...tscope-core/src/main/java/io/agentscope/core/skill/repository/FileSystemSkillRepository.java
Outdated
Show resolved
Hide resolved
...ill-git-repository/src/main/java/io/agentscope/core/skill/repository/GitSkillRepository.java
Outdated
Show resolved
Hide resolved
- Revert unnecessary Windows reserved character filtering in FileSystemSkillRepository (the : character issue was already fixed by using filesystem- prefix) - Change replaceAll to use "-" instead of "" in GitSkillRepository for clarity - Remove corresponding test case that was testing the reverted behavior Addressing review comments from @fang-tech: - FileSystemSkillRepository.java:171 - The character filtering was redundant since the source prefix already uses "-" instead of ":" - GitSkillRepository.java:344 - Replace reserved chars with "-" for better clarity
LearningGp
approved these changes
Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On Windows systems,
FileSystemSkillRepository.buildDefaultSourceSuffix()generates source strings containing Windows reserved characters (e.g.,C:drive colon), causingInvalidPathExceptionwhen AgentScope tries tocreate files or directories with these names.
Error example from #947:
java.nio.file.InvalidPathException: Illegal char <:> at index 25: kafka-sampling_filesystem:.agents_skills
Solution
Remove all Windows reserved characters (
\,/,:,*,?,",<,>,|) from the generated source suffix using regex replacement.Changes
buildDefaultSourceSuffix()inFileSystemSkillRepository.javaTesting
testSave_SourceHasNoColonnow passes on WindowsFixes #947