fix(skill): keep skill workspace paths stable while fixing non-ASCII collision

The #254 fix changed resolveConventionPath to {name}-{hashCode} and folded
hyphens to underscores, which re-pathed every existing skill (browser-cdp ->
browser_cdp-<hash>) with no migration, orphaning already-created workspaces and
breaking SkillWorkspaceManagerApplyBundleTest. Drop the hash suffix and keep
hyphens: the bare Unicode-preserving sanitized name already prevents the
non-ASCII collision (distinct CJK names map to distinct dirs) and leaves ASCII
kebab-case paths identical to the legacy scheme. Add path regression tests.
This commit is contained in:
matevip 2026-06-07 20:06:19 +08:00
parent 00b87a4325
commit 86cb449bd5
2 changed files with 76 additions and 16 deletions

View File

@ -47,32 +47,35 @@ public class SkillWorkspaceManager {
}
/**
* 按约定解析 skill 工作区路径{root}/{sanitizedName}-{hash}/
* 路径完全由 skillName 决定不依赖文件系统状态保证确定性
* 同一 skillName 始终返回同一路径不同 skillName 不会碰撞
* Resolve the conventional skill workspace path: {@code {root}/{sanitizedName}}.
* <p>
* Deterministic in {@code skillName} alone (no filesystem-state dependency). The
* non-ASCII collision fixed in #254 comes from {@link #sanitizeNameForFs} preserving
* Unicode letters/digits, so distinct names already map to distinct directories. No
* {@code -hash} suffix is appended: skill names are charset-constrained, so two names
* sanitizing to the same string is not a real case, and keeping the bare name avoids
* changing the path scheme for every existing skill (which would orphan already-created
* workspaces with no migration).
*/
public Path resolveConventionPath(String skillName) {
String base = sanitizeNameForFs(skillName);
String hash = Integer.toHexString(skillName.hashCode());
return getWorkspaceRoot().resolve(base + "-" + hash);
return getWorkspaceRoot().resolve(sanitizeNameForFs(skillName));
}
/**
* 清理文件系统路径不安全字符保留 Unicode 字母及数字的可读性
* 仅移除真正有问题的字符路径分隔符控制字符等
* Sanitize a skill name into a filesystem-safe directory segment.
* <p>
* Keeps the same charset as the legacy {@code [a-zA-Z0-9_.-]} rule but additionally
* preserves Unicode letters/digits ({@code \p{L}\p{N}}), so non-ASCII names no longer
* collapse to underscores and collide (#254). Everything else path separators,
* control characters, whitespace becomes {@code _}. Hyphens are kept (not folded to
* {@code _}) so existing kebab-case skill paths (e.g. {@code browser-cdp}) are unchanged
* across upgrades; only the Unicode handling differs from the legacy behavior.
*/
private String sanitizeNameForFs(String name) {
if (name == null || name.isBlank()) {
return "unnamed";
}
// 第一步移除路径分隔符和控制字符
String cleaned = name.replaceAll("[/\\\\:*?\"<>|\\x00-\\x1F]", "-");
// 第二步保留 Unicode 字母和数字其他替换为下划线
cleaned = cleaned.replaceAll("[^\\p{L}\\p{N}_\\-.\\s]", "_");
// 第三步折叠连续分隔符
cleaned = cleaned.replaceAll("[_\\s]+", "_").replaceAll("[-_]+", "_");
// 第四步去掉首尾分隔符
cleaned = cleaned.replaceAll("^-|-$", "");
String cleaned = name.strip().replaceAll("[^\\p{L}\\p{N}_.\\-]", "_");
return cleaned.isEmpty() ? "unnamed" : cleaned;
}

View File

@ -0,0 +1,57 @@
package vip.mate.skill.workspace;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.springframework.context.ApplicationEventPublisher;
import java.nio.file.Path;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
/**
* Regression tests for {@link SkillWorkspaceManager#resolveConventionPath}.
*
* <p>Issue #254: non-ASCII (e.g. Chinese) skill names collapsed to underscores in the
* workspace path, so distinct names resolved to the same directory and overwrote each
* other. The fix preserves Unicode letters/digits so distinct names map to distinct
* directories. The path is the bare sanitized name (no {@code -hash} suffix), so ASCII
* workspace paths stay stable across upgrades.
*/
class SkillWorkspaceManagerPathTest {
@TempDir
Path tmp;
private SkillWorkspaceManager newManager() {
SkillWorkspaceProperties props = new SkillWorkspaceProperties();
props.setRoot(tmp.toString());
return new SkillWorkspaceManager(props, mock(ApplicationEventPublisher.class));
}
@Test
@DisplayName("distinct non-ASCII names resolve to distinct directories (no collision)")
void nonAsciiNamesDoNotCollide() {
SkillWorkspaceManager m = newManager();
Path a = m.resolveConventionPath("我的技能");
Path b = m.resolveConventionPath("你的技能");
assertNotEquals(a, b, "Chinese names must not collapse to the same directory");
assertTrue(a.getFileName().toString().contains("我的技能"), "Unicode letters must be preserved");
assertTrue(b.getFileName().toString().contains("你的技能"), "Unicode letters must be preserved");
}
@Test
@DisplayName("ASCII name maps to the bare sanitized name with no -hash suffix")
void asciiNameKeepsBarePath() {
SkillWorkspaceManager m = newManager();
assertEquals(tmp.resolve("my-skill"), m.resolveConventionPath("my-skill"));
}
@Test
@DisplayName("path is deterministic for the same name")
void deterministicForSameName() {
SkillWorkspaceManager m = newManager();
assertEquals(m.resolveConventionPath("demo"), m.resolveConventionPath("demo"));
}
}