feat(skill): scope skill workspace filesystem paths by workspace to isolate same-named skills

This commit is contained in:
matevip 2026-07-24 10:26:57 +08:00
parent e294b32542
commit c18ff31ae8
24 changed files with 314 additions and 127 deletions

View File

@ -432,7 +432,7 @@ public class SkillController {
SkillFileEntity row = skillFileService.upsertFile(id, normalized, content); SkillFileEntity row = skillFileService.upsertFile(id, normalized, content);
try { try {
workspaceManager.writeWorkspaceFile(skill.getName(), normalized, content); workspaceManager.writeWorkspaceFile(skill.getName(), normalized, content, skill.getWorkspaceId());
} catch (Exception e) { } catch (Exception e) {
// Canonical store is updated; the syncer heals the cache later. // Canonical store is updated; the syncer heals the cache later.
} }
@ -464,7 +464,7 @@ public class SkillController {
} }
boolean removed = skillFileService.deleteFile(id, normalized); boolean removed = skillFileService.deleteFile(id, normalized);
try { try {
workspaceManager.deleteWorkspaceFile(skill.getName(), normalized); workspaceManager.deleteWorkspaceFile(skill.getName(), normalized, skill.getWorkspaceId());
} catch (Exception e) { } catch (Exception e) {
// Cache cleanup is best-effort; the canonical row is gone. // Cache cleanup is best-effort; the canonical row is gone.
} }
@ -956,7 +956,7 @@ public class SkillController {
@RequestHeader(value = "X-Workspace-Id", required = false) Long workspaceId) { @RequestHeader(value = "X-Workspace-Id", required = false) Long workspaceId) {
SkillEntity skill = skillService.getSkill(id); SkillEntity skill = skillService.getSkill(id);
verifyResourceWorkspace(skill, workspaceId); verifyResourceWorkspace(skill, workspaceId);
var path = workspaceManager.exportToWorkspace(skill.getName(), skill.getSkillContent()); var path = workspaceManager.exportToWorkspace(skill.getName(), skill.getSkillContent(), skill.getWorkspaceId());
if (path == null) { if (path == null) {
return R.ok(Map.of("success", false, "message", "Failed to export workspace")); return R.ok(Map.of("success", false, "message", "Failed to export workspace"));
} }
@ -970,7 +970,7 @@ public class SkillController {
@RequestHeader(value = "X-Workspace-Id", required = false) Long workspaceId) { @RequestHeader(value = "X-Workspace-Id", required = false) Long workspaceId) {
SkillEntity skill = skillService.getSkill(id); SkillEntity skill = skillService.getSkill(id);
verifyResourceWorkspace(skill, workspaceId); verifyResourceWorkspace(skill, workspaceId);
return R.ok(workspaceManager.getWorkspaceInfo(skill.getName())); return R.ok(workspaceManager.getWorkspaceInfo(skill.getName(), skill.getWorkspaceId()));
} }
// ==================== Skill lifecycle & curator ==================== // ==================== Skill lifecycle & curator ====================

View File

@ -161,10 +161,10 @@ public class SkillInstaller {
} }
// 4. Materialize SKILL.md (overwrite on reinstall, keep on first create). // 4. Materialize SKILL.md (overwrite on reinstall, keep on first create).
workspaceManager.initWorkspace(skillName, bundle.content(), exists); workspaceManager.initWorkspace(skillName, bundle.content(), exists, request.getWorkspaceId());
if (task.isCancelRequested()) { if (task.isCancelRequested()) {
workspaceManager.archiveWorkspace(skillName); workspaceManager.archiveWorkspace(skillName, request.getWorkspaceId());
task.markCancelled(); task.markCancelled();
return CompletableFuture.completedFuture(null); return CompletableFuture.completedFuture(null);
} }
@ -182,12 +182,12 @@ public class SkillInstaller {
// Empty-bundle guard protects both sides from a malformed bundle // Empty-bundle guard protects both sides from a malformed bundle
// silently wiping pre-existing scripts/references. // silently wiping pre-existing scripts/references.
boolean force = Boolean.TRUE.equals(request.getForcePrune()); boolean force = Boolean.TRUE.equals(request.getForcePrune());
persistBundleFiles(skillEntity, bundle, force, "url"); persistBundleFiles(skillEntity, bundle, force, "url", request.getWorkspaceId());
// 7. Publish event for runtime refresh / sibling-node materialization. // 7. Publish event for runtime refresh / sibling-node materialization.
eventPublisher.publishEvent(new SkillWorkspaceEvent( eventPublisher.publishEvent(new SkillWorkspaceEvent(
skillName, SkillWorkspaceEvent.Type.INSTALLED, skillName, SkillWorkspaceEvent.Type.INSTALLED,
workspaceManager.resolveConventionPath(skillName))); workspaceManager.resolveConventionPath(skillName, request.getWorkspaceId())));
task.markCompleted(InstallResult.builder() task.markCompleted(InstallResult.builder()
.name(skillName) .name(skillName)
@ -226,17 +226,17 @@ public class SkillInstaller {
} }
// Materialize SKILL.md (always overwrite on reinstall path). // Materialize SKILL.md (always overwrite on reinstall path).
workspaceManager.initWorkspace(skillName, bundle.content(), exists); workspaceManager.initWorkspace(skillName, bundle.content(), exists, workspaceId);
// Register/update skill row first so we have an id to anchor the file rows. // Register/update skill row first so we have an id to anchor the file rows.
SkillEntity skillEntity = upsertSkillRow(bundle, skillName, exists, enable, workspaceId); SkillEntity skillEntity = upsertSkillRow(bundle, skillName, exists, enable, workspaceId);
// DB-canonical, FS-cache. Empty-bundle guard on both sides. // DB-canonical, FS-cache. Empty-bundle guard on both sides.
persistBundleFiles(skillEntity, bundle, false, "zip"); persistBundleFiles(skillEntity, bundle, false, "zip", workspaceId);
eventPublisher.publishEvent(new SkillWorkspaceEvent( eventPublisher.publishEvent(new SkillWorkspaceEvent(
skillName, SkillWorkspaceEvent.Type.INSTALLED, skillName, SkillWorkspaceEvent.Type.INSTALLED,
workspaceManager.resolveConventionPath(skillName))); workspaceManager.resolveConventionPath(skillName, workspaceId)));
int filesCount = (bundle.references() != null ? bundle.references().size() : 0) int filesCount = (bundle.references() != null ? bundle.references().size() : 0)
+ (bundle.scripts() != null ? bundle.scripts().size() : 0) + 1; + (bundle.scripts() != null ? bundle.scripts().size() : 0) + 1;
@ -296,7 +296,8 @@ public class SkillInstaller {
* same prefixed-key map. Logs a single combined summary so multi-instance * same prefixed-key map. Logs a single combined summary so multi-instance
* deployments can see what each node persisted vs preserved. * deployments can see what each node persisted vs preserved.
*/ */
private void persistBundleFiles(SkillEntity skillEntity, SkillBundle bundle, boolean force, String origin) { private void persistBundleFiles(SkillEntity skillEntity, SkillBundle bundle, boolean force, String origin,
Long workspaceId) {
Map<String, String> combined = new LinkedHashMap<>(); Map<String, String> combined = new LinkedHashMap<>();
if (bundle.references() != null) { if (bundle.references() != null) {
for (var e : bundle.references().entrySet()) { for (var e : bundle.references().entrySet()) {
@ -313,7 +314,7 @@ public class SkillInstaller {
var dbApply = skillFileService.applyBundleFiles(skillEntity.getId(), combined, force); var dbApply = skillFileService.applyBundleFiles(skillEntity.getId(), combined, force);
var fsApply = workspaceManager.applyBundleFiles(skillEntity.getName(), var fsApply = workspaceManager.applyBundleFiles(skillEntity.getName(),
bundle.references(), bundle.scripts(), force); bundle.references(), bundle.scripts(), force, workspaceId);
log.info("Persisted bundle for '{}' ({}): db(write={}, prune={}, preservedScripts={}, preservedRefs={}) " + log.info("Persisted bundle for '{}' ({}): db(write={}, prune={}, preservedScripts={}, preservedRefs={}) " +
"fs(refs write={}, prune={}, preserved={} | scripts write={}, prune={}, preserved={})", "fs(refs write={}, prune={}, preserved={} | scripts write={}, prune={}, preserved={})",

View File

@ -205,7 +205,7 @@ public class SkillLessonsService {
private Path resolveWorkspace(ResolvedSkill resolved) { private Path resolveWorkspace(ResolvedSkill resolved) {
if (resolved == null || resolved.getName() == null) return null; if (resolved == null || resolved.getName() == null) return null;
if (resolved.getSkillDir() != null) return resolved.getSkillDir(); if (resolved.getSkillDir() != null) return resolved.getSkillDir();
Path convention = workspaceManager.resolveConventionPath(resolved.getName()); Path convention = workspaceManager.resolveConventionPath(resolved.getName(), resolved.getWorkspaceId());
return Files.exists(convention) && Files.isDirectory(convention) ? convention : null; return Files.exists(convention) && Files.isDirectory(convention) ? convention : null;
} }

View File

@ -269,7 +269,7 @@ public class SkillCuratorJob {
List<SkillEntity> archived = skillMapper.selectList(new LambdaQueryWrapper<SkillEntity>() List<SkillEntity> archived = skillMapper.selectList(new LambdaQueryWrapper<SkillEntity>()
.eq(SkillEntity::getLifecycleState, "archived")); .eq(SkillEntity::getLifecycleState, "archived"));
for (SkillEntity skill : archived) { for (SkillEntity skill : archived) {
if (skill.getName() == null || !workspaceManager.conventionWorkspaceExists(skill.getName())) { if (skill.getName() == null || !workspaceManager.conventionWorkspaceExists(skill.getName(), skill.getWorkspaceId())) {
continue; continue;
} }
report.reconciliation("skill '" + skill.getName() + "' (id=" + skill.getId() report.reconciliation("skill '" + skill.getName() + "' (id=" + skill.getId()

View File

@ -166,7 +166,7 @@ public class SkillLifecycleService {
"Skill is not archived: " + skill.getName()); "Skill is not archived: " + skill.getName());
} }
SkillWorkspaceManager.RestoreResult fs = workspaceManager.restoreWorkspace(skill.getName()); SkillWorkspaceManager.RestoreResult fs = workspaceManager.restoreWorkspace(skill.getName(), skill.getWorkspaceId());
switch (fs) { switch (fs) {
case MOVED -> { /* normal path */ } case MOVED -> { /* normal path */ }
case MISSING -> { case MISSING -> {
@ -266,7 +266,7 @@ public class SkillLifecycleService {
// FAILED defers the whole transition. // FAILED defers the whole transition.
SkillWorkspaceManager.ArchiveResult fsResult = SkillWorkspaceManager.ArchiveResult.MISSING; SkillWorkspaceManager.ArchiveResult fsResult = SkillWorkspaceManager.ArchiveResult.MISSING;
if ("archive".equals(workspaceProperties.getDeletePolicy())) { if ("archive".equals(workspaceProperties.getDeletePolicy())) {
fsResult = workspaceManager.archiveWorkspace(skill.getName()); fsResult = workspaceManager.archiveWorkspace(skill.getName(), skill.getWorkspaceId());
} }
if (fsResult == SkillWorkspaceManager.ArchiveResult.FAILED) { if (fsResult == SkillWorkspaceManager.ArchiveResult.FAILED) {
log.warn("Skill '{}' workspace archive failed; deferring DB transition", skill.getName()); log.warn("Skill '{}' workspace archive failed; deferring DB transition", skill.getName());
@ -288,7 +288,7 @@ public class SkillLifecycleService {
if (rows == 0) { if (rows == 0) {
log.warn("Skill '{}' DB archive update touched 0 rows; compensating workspace", skill.getName()); log.warn("Skill '{}' DB archive update touched 0 rows; compensating workspace", skill.getName());
if (fsResult == SkillWorkspaceManager.ArchiveResult.MOVED) { if (fsResult == SkillWorkspaceManager.ArchiveResult.MOVED) {
workspaceManager.restoreWorkspace(skill.getName()); workspaceManager.restoreWorkspace(skill.getName(), skill.getWorkspaceId());
} }
return false; return false;
} }

View File

@ -149,7 +149,7 @@ public class SkillPackageResolver {
resolved = resolveFromDirectory(entity, skillDir, configuredDir, "directory"); resolved = resolveFromDirectory(entity, skillDir, configuredDir, "directory");
} else { } else {
// 2. 约定路径 {workspace-root}/{skillName}/ // 2. 约定路径 {workspace-root}/{skillName}/
Path conventionPath = workspaceManager.resolveConventionPath(entity.getName()); Path conventionPath = workspaceManager.resolveConventionPath(entity.getName(), entity.getWorkspaceId());
if (Files.exists(conventionPath) && Files.isDirectory(conventionPath)) { if (Files.exists(conventionPath) && Files.isDirectory(conventionPath)) {
resolved = resolveFromDirectory(entity, conventionPath, conventionPath.toString(), "convention"); resolved = resolveFromDirectory(entity, conventionPath, conventionPath.toString(), "convention");
} else { } else {
@ -213,7 +213,7 @@ public class SkillPackageResolver {
} }
} }
Path convention = workspaceManager.resolveConventionPath(entity.getName()); Path convention = workspaceManager.resolveConventionPath(entity.getName(), entity.getWorkspaceId());
if (!Files.exists(convention) || !Files.isDirectory(convention)) return false; if (!Files.exists(convention) || !Files.isDirectory(convention)) return false;
SkillContentReconciler.Outcome outcome = contentReconciler.reconcile(entity, convention); SkillContentReconciler.Outcome outcome = contentReconciler.reconcile(entity, convention);
return outcome.action() == SkillContentReconciler.Action.INGESTED_TO_DB return outcome.action() == SkillContentReconciler.Action.INGESTED_TO_DB

View File

@ -361,7 +361,7 @@ public class SkillService {
// 自动初始化工作区目录 // 自动初始化工作区目录
if (workspaceProperties.isAutoInit() && !hasExplicitSkillDir(skill)) { if (workspaceProperties.isAutoInit() && !hasExplicitSkillDir(skill)) {
workspaceManager.initWorkspace(skill.getName(), skill.getSkillContent()); workspaceManager.initWorkspace(skill.getName(), skill.getSkillContent(), skill.getWorkspaceId());
} }
// 刷新 runtime cache // 刷新 runtime cache
@ -514,7 +514,7 @@ public class SkillService {
eventPublisher.publishEvent(new SkillRemovedEvent(id, skill.getName())); eventPublisher.publishEvent(new SkillRemovedEvent(id, skill.getName()));
if ("archive".equals(workspaceProperties.getDeletePolicy())) { if ("archive".equals(workspaceProperties.getDeletePolicy())) {
workspaceManager.archiveWorkspace(skill.getName()); workspaceManager.archiveWorkspace(skill.getName(), skill.getWorkspaceId());
} }
// RFC-090 review #3 refresh won't deregister wrappers for a // RFC-090 review #3 refresh won't deregister wrappers for a
// soft-deleted row (it only resolves rows still in // soft-deleted row (it only resolves rows still in
@ -562,7 +562,7 @@ public class SkillService {
log.warn("Failed to purge secrets for skill {}: {}", skill.getName(), e.getMessage()); log.warn("Failed to purge secrets for skill {}: {}", skill.getName(), e.getMessage());
} }
workspaceManager.purgeWorkspace(skill.getName()); workspaceManager.purgeWorkspace(skill.getName(), skill.getWorkspaceId());
// RFC-090 review #3 same explicit deregister as uninstall. // RFC-090 review #3 same explicit deregister as uninstall.
if (runtimeService != null) { if (runtimeService != null) {
@ -786,8 +786,8 @@ public class SkillService {
if (skill.getSkillContent() == null || skill.getSkillContent().isBlank()) { if (skill.getSkillContent() == null || skill.getSkillContent().isBlank()) {
return; return;
} }
if (workspaceManager.conventionWorkspaceExists(skill.getName())) { if (workspaceManager.conventionWorkspaceExists(skill.getName(), skill.getWorkspaceId())) {
Path workspaceDir = workspaceManager.resolveConventionPath(skill.getName()); Path workspaceDir = workspaceManager.resolveConventionPath(skill.getName(), skill.getWorkspaceId());
Path skillMd = workspaceDir.resolve("SKILL.md"); Path skillMd = workspaceDir.resolve("SKILL.md");
try { try {
Files.writeString(skillMd, skill.getSkillContent()); Files.writeString(skillMd, skill.getSkillContent());

View File

@ -127,7 +127,7 @@ public class SkillSynthesisService {
skillService.createSkill(skill); skillService.createSkill(skill);
try { try {
workspaceManager.exportToWorkspace(name, skillMd); workspaceManager.exportToWorkspace(name, skillMd, workspaceId);
} catch (Exception e) { } catch (Exception e) {
log.warn("[SkillSynthesis] Workspace export failed for '{}': {}", name, e.getMessage()); log.warn("[SkillSynthesis] Workspace export failed for '{}': {}", name, e.getMessage());
} }

View File

@ -106,7 +106,7 @@ public class SkillTemplateService {
// classpath:{bundlePath}/** scripts, references, fonts, etc. // classpath:{bundlePath}/** scripts, references, fonts, etc.
// Top-level SKILL.md in the bundle is skipped automatically so // Top-level SKILL.md in the bundle is skipped automatically so
// the rendered manifest from step 2 stays authoritative. // the rendered manifest from step 2 stays authoritative.
overlayBundle(template, created.getName()); overlayBundle(template, created.getName(), created.getWorkspaceId());
// 5. Persist any `secret` field values into mate_skill_secret so // 5. Persist any `secret` field values into mate_skill_secret so
// the runtime can decrypt + inject them as env vars at exec // the runtime can decrypt + inject them as env vars at exec
@ -138,13 +138,13 @@ public class SkillTemplateService {
} }
} }
private void overlayBundle(SkillTemplate template, String skillName) { private void overlayBundle(SkillTemplate template, String skillName, Long workspaceId) {
String bundlePath = template.getBundlePath(); String bundlePath = template.getBundlePath();
if (bundlePath == null || bundlePath.isBlank()) { if (bundlePath == null || bundlePath.isBlank()) {
return; return;
} }
SkillBundleSource source = new ClasspathBundleSource(resourceResolver, bundlePath); SkillBundleSource source = new ClasspathBundleSource(resourceResolver, bundlePath);
Path workspaceDir = workspaceManager.resolveConventionPath(skillName); Path workspaceDir = workspaceManager.resolveConventionPath(skillName, workspaceId);
try { try {
SkillBundleMaterializer.Result result = bundleMaterializer.materialize( SkillBundleMaterializer.Result result = bundleMaterializer.materialize(
source, workspaceDir, MaterializeOptions.templateOverlay()); source, workspaceDir, MaterializeOptions.templateOverlay());

View File

@ -63,6 +63,9 @@ public class BundledSkillSyncer {
private static final Pattern VERSION_PATTERN = private static final Pattern VERSION_PATTERN =
Pattern.compile("^version:\\s*[\"']?([^\"'\\s]+)[\"']?", Pattern.MULTILINE); Pattern.compile("^version:\\s*[\"']?([^\"'\\s]+)[\"']?", Pattern.MULTILINE);
/** Builtin/bundled skills are global and materialized under the default workspace. */
private static final Long BUILTIN_WORKSPACE_ID = 1L;
private final SkillWorkspaceProperties properties; private final SkillWorkspaceProperties properties;
private final SkillWorkspaceManager workspaceManager; private final SkillWorkspaceManager workspaceManager;
private final SkillBundleMaterializer bundleMaterializer; private final SkillBundleMaterializer bundleMaterializer;
@ -109,7 +112,8 @@ public class BundledSkillSyncer {
*/ */
private boolean syncOne(ResourcePatternResolver resolver, String bundledPath, private boolean syncOne(ResourcePatternResolver resolver, String bundledPath,
String skillName, Resource manifest) { String skillName, Resource manifest) {
Path targetDir = workspaceManager.resolveConventionPath(skillName); // Builtin/bundled skills are global and seeded into workspace 1.
Path targetDir = workspaceManager.resolveConventionPath(skillName, BUILTIN_WORKSPACE_ID);
boolean firstInstall = !Files.exists(targetDir); boolean firstInstall = !Files.exists(targetDir);
SkillBundleSource source = new ClasspathBundleSource(resolver, SkillBundleSource source = new ClasspathBundleSource(resolver,
@ -133,7 +137,7 @@ public class BundledSkillSyncer {
skillName, missing); skillName, missing);
} }
// Archive (never overwrite in place) so local edits stay recoverable. // Archive (never overwrite in place) so local edits stay recoverable.
workspaceManager.archiveWorkspace(skillName); workspaceManager.archiveWorkspace(skillName, BUILTIN_WORKSPACE_ID);
} }
copyBundle(source, targetDir); copyBundle(source, targetDir);

View File

@ -102,7 +102,7 @@ public class SkillFileSyncer {
* are restored. * are restored.
*/ */
public PerSkillReport syncOne(SkillEntity skill) { public PerSkillReport syncOne(SkillEntity skill) {
Path workspaceDir = workspaceManager.resolveConventionPath(skill.getName()); Path workspaceDir = workspaceManager.resolveConventionPath(skill.getName(), skill.getWorkspaceId());
List<SkillFileEntity> dbFiles = skillFileService.listBySkillId(skill.getId()); List<SkillFileEntity> dbFiles = skillFileService.listBySkillId(skill.getId());
boolean didBackfill = false; boolean didBackfill = false;

View File

@ -6,6 +6,8 @@ import org.springframework.boot.ApplicationArguments;
import org.springframework.boot.ApplicationRunner; import org.springframework.boot.ApplicationRunner;
import org.springframework.core.annotation.Order; import org.springframework.core.annotation.Order;
import org.springframework.stereotype.Component; import org.springframework.stereotype.Component;
import vip.mate.skill.model.SkillEntity;
import vip.mate.skill.service.SkillService;
import java.util.List; import java.util.List;
@ -37,12 +39,20 @@ public class SkillWorkspaceBootstrapRunner implements ApplicationRunner {
private final SkillWorkspaceManager workspaceManager; private final SkillWorkspaceManager workspaceManager;
private final BundledSkillSyncer bundledSkillSyncer; private final BundledSkillSyncer bundledSkillSyncer;
private final SkillFileSyncer skillFileSyncer; private final SkillFileSyncer skillFileSyncer;
private final SkillService skillService;
@Override @Override
public void run(ApplicationArguments args) { public void run(ApplicationArguments args) {
var root = workspaceManager.getWorkspaceRoot(); var root = workspaceManager.getWorkspaceRoot();
log.info("Skill workspace root ready: {}", root); log.info("Skill workspace root ready: {}", root);
// Step 0 one-time layout migration BEFORE any sync: move legacy flat
// {root}/{name} dirs into their workspace-scoped {root}/{workspaceId}/{name}
// location. Must run before skillFileSyncer.syncAll(), else the syncer
// would materialize DB content at the new scoped path first and leave the
// old flat dir (and any on-disk-only files it holds) orphaned.
migrateLegacyLayout();
List<String> synced = bundledSkillSyncer.sync(); List<String> synced = bundledSkillSyncer.sync();
if (!synced.isEmpty()) { if (!synced.isEmpty()) {
log.info("Synced {} bundled skill(s) to workspace: {}", synced.size(), synced); log.info("Synced {} bundled skill(s) to workspace: {}", synced.size(), synced);
@ -57,4 +67,25 @@ public class SkillWorkspaceBootstrapRunner implements ApplicationRunner {
report.filesAlreadyCurrent(), report.filesAlreadyCurrent(),
report.skillsBackfilled(), report.filesBackfilledFromDisk()); report.skillsBackfilled(), report.filesBackfilledFromDisk());
} }
/**
* Walk every persisted skill and migrate its legacy flat workspace directory
* into the workspace-scoped layout. Idempotent once migrated, subsequent
* starts find nothing to move.
*/
private void migrateLegacyLayout() {
List<SkillEntity> skills = skillService.listSkills();
int moved = 0;
for (SkillEntity skill : skills) {
if (skill.getName() == null) {
continue;
}
if (workspaceManager.migrateLegacyFlatDir(skill.getName(), skill.getWorkspaceId())) {
moved++;
}
}
if (moved > 0) {
log.info("Migrated {} legacy skill workspace dir(s) to the workspace-scoped layout", moved);
}
}
} }

View File

@ -47,18 +47,34 @@ public class SkillWorkspaceManager {
} }
/** /**
* Resolve the conventional skill workspace path: {@code {root}/{sanitizedName}}. * Resolve the conventional skill workspace path:
* {@code {root}/{workspaceId}/{sanitizedName}}.
* <p> * <p>
* Deterministic in {@code skillName} alone (no filesystem-state dependency). The * Scoping the path by {@code workspaceId} keeps same-named skills in different
* non-ASCII collision fixed in #254 comes from {@link #sanitizeNameForFs} preserving * workspaces on disjoint disk directories, so they no longer share a workspace
* Unicode letters/digits, so distinct names already map to distinct directories. No * directory, overwrite each other, or cross script execution between tenants.
* {@code -hash} suffix is appended: skill names are charset-constrained, so two names * A {@code null} workspaceId falls back to workspace {@code 1} (the default
* sanitizing to the same string is not a real case, and keeping the bare name avoids * workspace) this is a defensive fallback only; real call sites must pass the
* changing the path scheme for every existing skill (which would orphan already-created * skill's true owning workspace.
* workspaces with no migration). * <p>
* Deterministic in {@code (skillName, workspaceId)} 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.
*/ */
public Path resolveConventionPath(String skillName) { public Path resolveConventionPath(String skillName, Long workspaceId) {
return getWorkspaceRoot().resolve(sanitizeNameForFs(skillName)); return workspaceScopedRoot(workspaceId).resolve(sanitizeNameForFs(skillName));
}
/**
* Per-workspace root: {@code {root}/{workspaceId}}. A {@code null} workspaceId
* maps to workspace {@code 1} so a missing scope never resolves to the bare root
* (which would reintroduce the cross-workspace collision this scheme prevents).
*/
private Path workspaceScopedRoot(Long workspaceId) {
return getWorkspaceRoot().resolve(String.valueOf(workspaceId == null ? 1L : workspaceId));
} }
/** /**
@ -87,7 +103,7 @@ public class SkillWorkspaceManager {
* <li>null无目录回退数据库</li> * <li>null无目录回退数据库</li>
* </ol> * </ol>
*/ */
public Path resolveEffectivePath(String skillName, String configuredDir) { public Path resolveEffectivePath(String skillName, String configuredDir, Long workspaceId) {
// 1. 显式配置 // 1. 显式配置
if (configuredDir != null && !configuredDir.isBlank()) { if (configuredDir != null && !configuredDir.isBlank()) {
Path explicit = Paths.get(configuredDir); Path explicit = Paths.get(configuredDir);
@ -96,7 +112,7 @@ public class SkillWorkspaceManager {
} }
} }
// 2. 约定路径 // 2. 约定路径
Path convention = resolveConventionPath(skillName); Path convention = resolveConventionPath(skillName, workspaceId);
if (Files.exists(convention) && Files.isDirectory(convention)) { if (Files.exists(convention) && Files.isDirectory(convention)) {
return convention; return convention;
} }
@ -107,11 +123,53 @@ public class SkillWorkspaceManager {
/** /**
* 检查约定路径的 workspace 是否存在 * 检查约定路径的 workspace 是否存在
*/ */
public boolean conventionWorkspaceExists(String skillName) { public boolean conventionWorkspaceExists(String skillName, Long workspaceId) {
Path convention = resolveConventionPath(skillName); Path convention = resolveConventionPath(skillName, workspaceId);
return Files.exists(convention) && Files.isDirectory(convention); return Files.exists(convention) && Files.isDirectory(convention);
} }
/**
* One-time layout migration: the legacy scheme stored every skill flat at
* {@code {root}/{name}}; the workspace-scoped scheme stores it at
* {@code {root}/{workspaceId}/{name}}. Move a pre-existing flat directory
* into its scoped location so the skill's scripts/references survive the
* scheme change instead of being silently orphaned (the runtime would
* otherwise fall back to DB content and lose on-disk-only files).
*
* <p>Idempotent and safe to run on every startup:
* <ul>
* <li>no-op if the flat directory is absent (already migrated, or new install);</li>
* <li>no-op if the scoped target already exists (never overwrites);</li>
* <li>skips anything without a top-level {@code SKILL.md} so a workspace-scoped
* root like {@code {root}/1} (whose children are skills, not a skill itself)
* is never mistaken for a legacy flat skill directory.</li>
* </ul>
*
* @return {@code true} only when a move actually happened.
*/
public boolean migrateLegacyFlatDir(String skillName, Long workspaceId) {
Path legacy = getWorkspaceRoot().resolve(sanitizeNameForFs(skillName));
Path scoped = resolveConventionPath(skillName, workspaceId);
if (legacy.equals(scoped)) {
return false; // scoped always adds a {workspaceId} segment; defensive only
}
if (!Files.isDirectory(legacy) || !Files.exists(legacy.resolve("SKILL.md"))) {
return false; // absent, or not a real flat skill workspace
}
if (Files.exists(scoped)) {
return false; // already migrated / target present never overwrite
}
try {
Files.createDirectories(scoped.getParent());
Files.move(legacy, scoped, StandardCopyOption.ATOMIC_MOVE);
log.info("Migrated legacy skill workspace {} -> {}", legacy, scoped);
return true;
} catch (IOException e) {
log.warn("Failed to migrate legacy skill workspace {} -> {}: {}", legacy, scoped, e.getMessage());
return false;
}
}
// ==================== 生命周期操作 ==================== // ==================== 生命周期操作 ====================
/** /**
@ -121,8 +179,8 @@ public class SkillWorkspaceManager {
* @param initialContent SKILL.md 初始内容可为 null * @param initialContent SKILL.md 初始内容可为 null
* @return 创建的工作区路径 * @return 创建的工作区路径
*/ */
public Path initWorkspace(String skillName, String initialContent) { public Path initWorkspace(String skillName, String initialContent, Long workspaceId) {
return initWorkspace(skillName, initialContent, false); return initWorkspace(skillName, initialContent, false, workspaceId);
} }
/** /**
@ -134,8 +192,8 @@ public class SkillWorkspaceManager {
* false 时仅在 SKILL.md 不存在时写入用于首次创建 * false 时仅在 SKILL.md 不存在时写入用于首次创建
* @return 创建的工作区路径 * @return 创建的工作区路径
*/ */
public Path initWorkspace(String skillName, String initialContent, boolean overwrite) { public Path initWorkspace(String skillName, String initialContent, boolean overwrite, Long workspaceId) {
Path workspaceDir = resolveConventionPath(skillName); Path workspaceDir = resolveConventionPath(skillName, workspaceId);
try { try {
Files.createDirectories(workspaceDir); Files.createDirectories(workspaceDir);
Files.createDirectories(workspaceDir.resolve("references")); Files.createDirectories(workspaceDir.resolve("references"));
@ -170,8 +228,8 @@ public class SkillWorkspaceManager {
* by hard-delete only; uninstall still calls * by hard-delete only; uninstall still calls
* {@link #archiveWorkspace} so users can recover by re-installing. * {@link #archiveWorkspace} so users can recover by re-installing.
*/ */
public void purgeWorkspace(String skillName) { public void purgeWorkspace(String skillName, Long workspaceId) {
Path workspaceDir = resolveConventionPath(skillName); Path workspaceDir = resolveConventionPath(skillName, workspaceId);
if (!Files.exists(workspaceDir)) return; if (!Files.exists(workspaceDir)) return;
try { try {
// Walk and delete bottom-up so non-empty dirs go away too. // Walk and delete bottom-up so non-empty dirs go away too.
@ -218,7 +276,8 @@ public class SkillWorkspaceManager {
} }
/** /**
* Move {@code {root}/{name}/} to {@code {root}/.archived/{name}-{ts}/}. * Move {@code {root}/{workspaceId}/{name}/} to
* {@code {root}/{workspaceId}/.archived/{name}-{ts}/}.
* *
* <p>Returns {@link ArchiveResult#MISSING} when the workspace doesn't * <p>Returns {@link ArchiveResult#MISSING} when the workspace doesn't
* exist callers may treat this as a successful no-op since the runtime * exist callers may treat this as a successful no-op since the runtime
@ -228,14 +287,14 @@ public class SkillWorkspaceManager {
* {@link ArchiveResult#MOVED} on success, having already published * {@link ArchiveResult#MOVED} on success, having already published
* {@link SkillWorkspaceEvent.Type#ARCHIVED}. * {@link SkillWorkspaceEvent.Type#ARCHIVED}.
*/ */
public ArchiveResult archiveWorkspace(String skillName) { public ArchiveResult archiveWorkspace(String skillName, Long workspaceId) {
Path workspaceDir = resolveConventionPath(skillName); Path workspaceDir = resolveConventionPath(skillName, workspaceId);
if (!Files.exists(workspaceDir)) { if (!Files.exists(workspaceDir)) {
return ArchiveResult.MISSING; return ArchiveResult.MISSING;
} }
try { try {
Path archiveRoot = getWorkspaceRoot().resolve(".archived"); Path archiveRoot = workspaceScopedRoot(workspaceId).resolve(".archived");
Files.createDirectories(archiveRoot); Files.createDirectories(archiveRoot);
String archiveName = sanitizeName(skillName) + "-" + LocalDateTime.now().format(ARCHIVE_TS); String archiveName = sanitizeName(skillName) + "-" + LocalDateTime.now().format(ARCHIVE_TS);
@ -261,13 +320,13 @@ public class SkillWorkspaceManager {
* {@link RestoreResult#FAILED} when an archive directory exists but the * {@link RestoreResult#FAILED} when an archive directory exists but the
* move-back fails. * move-back fails.
*/ */
public RestoreResult restoreWorkspace(String skillName) { public RestoreResult restoreWorkspace(String skillName, Long workspaceId) {
Path target = resolveConventionPath(skillName); Path target = resolveConventionPath(skillName, workspaceId);
if (Files.exists(target)) { if (Files.exists(target)) {
log.warn("restoreWorkspace skipped: target {} already exists", target); log.warn("restoreWorkspace skipped: target {} already exists", target);
return RestoreResult.MISSING; return RestoreResult.MISSING;
} }
Path archiveRoot = getWorkspaceRoot().resolve(".archived"); Path archiveRoot = workspaceScopedRoot(workspaceId).resolve(".archived");
if (!Files.exists(archiveRoot)) { if (!Files.exists(archiveRoot)) {
return RestoreResult.MISSING; return RestoreResult.MISSING;
} }
@ -313,9 +372,9 @@ public class SkillWorkspaceManager {
/** /**
* 将数据库 skill 内容导出到工作区目录 * 将数据库 skill 内容导出到工作区目录
*/ */
public Path exportToWorkspace(String skillName, String skillContent) { public Path exportToWorkspace(String skillName, String skillContent, Long workspaceId) {
// 始终覆写 SKILL.mdinitWorkspace 内部已写入无需再做一次冗余 IO // 始终覆写 SKILL.mdinitWorkspace 内部已写入无需再做一次冗余 IO
Path workspaceDir = initWorkspace(skillName, skillContent, true); Path workspaceDir = initWorkspace(skillName, skillContent, true, workspaceId);
if (workspaceDir != null) { if (workspaceDir != null) {
log.info("Exported skill '{}' to workspace: {}", skillName, workspaceDir); log.info("Exported skill '{}' to workspace: {}", skillName, workspaceDir);
eventPublisher.publishEvent(new SkillWorkspaceEvent(skillName, SkillWorkspaceEvent.Type.EXPORTED, workspaceDir)); eventPublisher.publishEvent(new SkillWorkspaceEvent(skillName, SkillWorkspaceEvent.Type.EXPORTED, workspaceDir));
@ -338,8 +397,8 @@ public class SkillWorkspaceManager {
* @param content 文件内容 * @param content 文件内容
* @throws IllegalArgumentException 如果路径不安全 * @throws IllegalArgumentException 如果路径不安全
*/ */
public void writeWorkspaceFile(String skillName, String relativePath, String content) { public void writeWorkspaceFile(String skillName, String relativePath, String content, Long workspaceId) {
Path workspaceDir = resolveConventionPath(skillName); Path workspaceDir = resolveConventionPath(skillName, workspaceId);
// 路径安全校验 // 路径安全校验
Path safePath = validateWritePath(workspaceDir, relativePath); Path safePath = validateWritePath(workspaceDir, relativePath);
@ -363,8 +422,8 @@ public class SkillWorkspaceManager {
* *
* @throws IllegalArgumentException 如果路径不安全 * @throws IllegalArgumentException 如果路径不安全
*/ */
public void deleteWorkspaceFile(String skillName, String relativePath) { public void deleteWorkspaceFile(String skillName, String relativePath, Long workspaceId) {
Path workspaceDir = resolveConventionPath(skillName); Path workspaceDir = resolveConventionPath(skillName, workspaceId);
Path safePath = validateWritePath(workspaceDir, relativePath); Path safePath = validateWritePath(workspaceDir, relativePath);
if (safePath == null) { if (safePath == null) {
throw new IllegalArgumentException("Unsafe file path rejected: " + relativePath); throw new IllegalArgumentException("Unsafe file path rejected: " + relativePath);
@ -394,8 +453,8 @@ public class SkillWorkspaceManager {
* 清空 skill 工作区中的 references/ scripts/ 目录内容保留目录本身 * 清空 skill 工作区中的 references/ scripts/ 目录内容保留目录本身
* 用于 overwrite 安装前清除旧版本残留文件 * 用于 overwrite 安装前清除旧版本残留文件
*/ */
public void cleanWorkspaceDataDirs(String skillName) { public void cleanWorkspaceDataDirs(String skillName, Long workspaceId) {
Path workspaceDir = resolveConventionPath(skillName); Path workspaceDir = resolveConventionPath(skillName, workspaceId);
cleanDirectoryContents(workspaceDir.resolve("references")); cleanDirectoryContents(workspaceDir.resolve("references"));
cleanDirectoryContents(workspaceDir.resolve("scripts")); cleanDirectoryContents(workspaceDir.resolve("scripts"));
} }
@ -439,8 +498,9 @@ public class SkillWorkspaceManager {
public ApplyBundleResult applyBundleFiles(String skillName, public ApplyBundleResult applyBundleFiles(String skillName,
Map<String, String> references, Map<String, String> references,
Map<String, String> scripts, Map<String, String> scripts,
boolean force) { boolean force,
Path workspaceDir = resolveConventionPath(skillName); Long workspaceId) {
Path workspaceDir = resolveConventionPath(skillName, workspaceId);
try { try {
Files.createDirectories(workspaceDir.resolve("references")); Files.createDirectories(workspaceDir.resolve("references"));
Files.createDirectories(workspaceDir.resolve("scripts")); Files.createDirectories(workspaceDir.resolve("scripts"));
@ -448,8 +508,8 @@ public class SkillWorkspaceManager {
log.warn("Failed to ensure data dirs for skill '{}': {}", skillName, e.getMessage()); log.warn("Failed to ensure data dirs for skill '{}': {}", skillName, e.getMessage());
} }
int refsWritten = applyBucket(skillName, "references/", references); int refsWritten = applyBucket(skillName, "references/", references, workspaceId);
int scriptsWritten = applyBucket(skillName, "scripts/", scripts); int scriptsWritten = applyBucket(skillName, "scripts/", scripts, workspaceId);
var refsPrune = pruneBucket(workspaceDir.resolve("references"), var refsPrune = pruneBucket(workspaceDir.resolve("references"),
normalizeKeys(references), force, skillName, "references"); normalizeKeys(references), force, skillName, "references");
@ -462,14 +522,14 @@ public class SkillWorkspaceManager {
); );
} }
private int applyBucket(String skillName, String bucketPrefix, Map<String, String> entries) { private int applyBucket(String skillName, String bucketPrefix, Map<String, String> entries, Long workspaceId) {
if (entries == null || entries.isEmpty()) return 0; if (entries == null || entries.isEmpty()) return 0;
int written = 0; int written = 0;
for (var e : entries.entrySet()) { for (var e : entries.entrySet()) {
String key = e.getKey(); String key = e.getKey();
String relative = key.startsWith(bucketPrefix) ? key : (bucketPrefix + key); String relative = key.startsWith(bucketPrefix) ? key : (bucketPrefix + key);
try { try {
writeWorkspaceFile(skillName, relative, e.getValue()); writeWorkspaceFile(skillName, relative, e.getValue(), workspaceId);
written++; written++;
} catch (RuntimeException ex) { } catch (RuntimeException ex) {
log.warn("Failed to write {} for skill '{}': {}", relative, skillName, ex.getMessage()); log.warn("Failed to write {} for skill '{}': {}", relative, skillName, ex.getMessage());
@ -618,8 +678,8 @@ public class SkillWorkspaceManager {
/** /**
* 获取 skill 工作区信息 * 获取 skill 工作区信息
*/ */
public Map<String, Object> getWorkspaceInfo(String skillName) { public Map<String, Object> getWorkspaceInfo(String skillName, Long workspaceId) {
Path workspaceDir = resolveConventionPath(skillName); Path workspaceDir = resolveConventionPath(skillName, workspaceId);
Map<String, Object> info = new LinkedHashMap<>(); Map<String, Object> info = new LinkedHashMap<>();
info.put("skillName", skillName); info.put("skillName", skillName);
info.put("conventionPath", workspaceDir.toString()); info.put("conventionPath", workspaceDir.toString());

View File

@ -210,7 +210,7 @@ public class SkillManageTool {
// 同步到 workspace 文件系统 // 同步到 workspace 文件系统
try { try {
workspaceManager.exportToWorkspace(name, content); workspaceManager.exportToWorkspace(name, content, skill.getWorkspaceId());
} catch (Exception e) { } catch (Exception e) {
log.warn("[SkillManage] Workspace export failed for '{}': {}", name, e.getMessage()); log.warn("[SkillManage] Workspace export failed for '{}': {}", name, e.getMessage());
} }
@ -254,7 +254,7 @@ public class SkillManageTool {
skillService.updateSkill(existing); skillService.updateSkill(existing);
try { try {
workspaceManager.exportToWorkspace(name, content); workspaceManager.exportToWorkspace(name, content, existing.getWorkspaceId());
} catch (Exception e) { } catch (Exception e) {
log.warn("[SkillManage] Workspace export failed for '{}': {}", name, e.getMessage()); log.warn("[SkillManage] Workspace export failed for '{}': {}", name, e.getMessage());
} }
@ -335,7 +335,7 @@ public class SkillManageTool {
skillService.updateSkill(existing); skillService.updateSkill(existing);
try { try {
workspaceManager.exportToWorkspace(name, patchedContent); workspaceManager.exportToWorkspace(name, patchedContent, existing.getWorkspaceId());
} catch (Exception e) { } catch (Exception e) {
log.warn("[SkillManage] Workspace export failed for '{}': {}", name, e.getMessage()); log.warn("[SkillManage] Workspace export failed for '{}': {}", name, e.getMessage());
} }
@ -385,7 +385,7 @@ public class SkillManageTool {
} }
try { try {
workspaceManager.writeWorkspaceFile(name, filePath, content); workspaceManager.writeWorkspaceFile(name, filePath, content, existing.getWorkspaceId());
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
return "Error: " + e.getMessage() return "Error: " + e.getMessage()
+ " (paths must start with references/, scripts/ or templates/, and may not contain '..')."; + " (paths must start with references/, scripts/ or templates/, and may not contain '..').";

View File

@ -54,6 +54,7 @@ class SkillControllerBundleFilesTest {
private SkillEntity skill(boolean builtin) { private SkillEntity skill(boolean builtin) {
SkillEntity s = new SkillEntity(); SkillEntity s = new SkillEntity();
s.setId(SID); s.setId(SID);
s.setWorkspaceId(1L);
s.setName("demo-skill"); s.setName("demo-skill");
s.setBuiltin(builtin); s.setBuiltin(builtin);
return s; return s;
@ -131,7 +132,7 @@ class SkillControllerBundleFilesTest {
assertThat(resp.getData()).containsEntry("path", "templates/report.html"); assertThat(resp.getData()).containsEntry("path", "templates/report.html");
verify(fileService).upsertFile(SID, "templates/report.html", "<html/>"); verify(fileService).upsertFile(SID, "templates/report.html", "<html/>");
verify(workspaceManager).writeWorkspaceFile("demo-skill", "templates/report.html", "<html/>"); verify(workspaceManager).writeWorkspaceFile("demo-skill", "templates/report.html", "<html/>", 1L);
verify(runtimeService).rescanSingle(any(SkillEntity.class)); verify(runtimeService).rescanSingle(any(SkillEntity.class));
} }
@ -170,7 +171,7 @@ class SkillControllerBundleFilesTest {
R<Map<String, Object>> resp = controller.deleteBundleFile(SID, "scripts/run.py", null); R<Map<String, Object>> resp = controller.deleteBundleFile(SID, "scripts/run.py", null);
assertThat(resp.getData()).containsEntry("removed", true); assertThat(resp.getData()).containsEntry("removed", true);
verify(workspaceManager).deleteWorkspaceFile("demo-skill", "scripts/run.py"); verify(workspaceManager).deleteWorkspaceFile("demo-skill", "scripts/run.py", 1L);
verify(runtimeService).rescanSingle(any(SkillEntity.class)); verify(runtimeService).rescanSingle(any(SkillEntity.class));
} }

View File

@ -48,7 +48,7 @@ class SkillLessonsServiceTest {
workspaceManager = mock(SkillWorkspaceManager.class); workspaceManager = mock(SkillWorkspaceManager.class);
publishedEvents = new ArrayList<>(); publishedEvents = new ArrayList<>();
publisher = event -> publishedEvents.add(event); publisher = event -> publishedEvents.add(event);
when(workspaceManager.resolveConventionPath(anyString())) when(workspaceManager.resolveConventionPath(anyString(), any()))
.thenAnswer(inv -> tempDir.resolve(inv.getArgument(0, String.class))); .thenAnswer(inv -> tempDir.resolve(inv.getArgument(0, String.class)));
service = new SkillLessonsService(workspaceManager, publisher); service = new SkillLessonsService(workspaceManager, publisher);
} }
@ -141,7 +141,7 @@ class SkillLessonsServiceTest {
void noWorkspaceNoOp() { void noWorkspaceNoOp() {
ResolvedSkill skill = ResolvedSkill.builder().id(1L).name("nope").build(); ResolvedSkill skill = ResolvedSkill.builder().id(1L).name("nope").build();
// Force a non-existent convention path so resolveWorkspace returns null. // Force a non-existent convention path so resolveWorkspace returns null.
when(workspaceManager.resolveConventionPath("nope")) when(workspaceManager.resolveConventionPath("nope", null))
.thenReturn(tempDir.resolve("does-not-exist")); .thenReturn(tempDir.resolve("does-not-exist"));
String id = service.recordLesson(skill, null, null, "won't write", 50); String id = service.recordLesson(skill, null, null, "won't write", 50);
assertNull(id); assertNull(id);

View File

@ -78,6 +78,7 @@ class SkillCuratorJobTest {
private SkillEntity candidate(long id, String state, LocalDateTime lastActivity) { private SkillEntity candidate(long id, String state, LocalDateTime lastActivity) {
SkillEntity s = new SkillEntity(); SkillEntity s = new SkillEntity();
s.setId(id); s.setId(id);
s.setWorkspaceId(1L);
s.setName("skill-" + id); s.setName("skill-" + id);
s.setSkillType("dynamic"); s.setSkillType("dynamic");
s.setBuiltin(false); s.setBuiltin(false);
@ -205,7 +206,7 @@ class SkillCuratorJobTest {
SkillEntity orphan = candidate(9L, "archived", now.minusDays(100)); SkillEntity orphan = candidate(9L, "archived", now.minusDays(100));
// 1st selectList = reconcile (archived rows); 2nd = loadCandidates. // 1st selectList = reconcile (archived rows); 2nd = loadCandidates.
when(skillMapper.selectList(any())).thenReturn(List.of(orphan), List.of()); when(skillMapper.selectList(any())).thenReturn(List.of(orphan), List.of());
when(workspaceManager.conventionWorkspaceExists("skill-9")).thenReturn(true); when(workspaceManager.conventionWorkspaceExists("skill-9", 1L)).thenReturn(true);
job.run(); job.run();

View File

@ -71,6 +71,7 @@ class SkillLifecycleServiceTest {
private SkillEntity skill(String type, String state, LocalDateTime lastActivity) { private SkillEntity skill(String type, String state, LocalDateTime lastActivity) {
SkillEntity s = new SkillEntity(); SkillEntity s = new SkillEntity();
s.setId(1L); s.setId(1L);
s.setWorkspaceId(1L);
s.setName("demo-skill"); s.setName("demo-skill");
s.setSkillType(type); s.setSkillType(type);
s.setBuiltin(false); s.setBuiltin(false);
@ -175,7 +176,7 @@ class SkillLifecycleServiceTest {
@Test @Test
void restoreMovesWorkspaceBackAndFlipsTheRow() { void restoreMovesWorkspaceBackAndFlipsTheRow() {
when(skillMapper.selectById(1L)).thenReturn(archivedSkill()); when(skillMapper.selectById(1L)).thenReturn(archivedSkill());
when(workspaceManager.restoreWorkspace("demo-skill")) when(workspaceManager.restoreWorkspace("demo-skill", 1L))
.thenReturn(SkillWorkspaceManager.RestoreResult.MOVED); .thenReturn(SkillWorkspaceManager.RestoreResult.MOVED);
service.restore(1L); service.restore(1L);
@ -187,7 +188,7 @@ class SkillLifecycleServiceTest {
@Test @Test
void restoreDbOnlySkillFlipsRowWithoutWorkspace() { void restoreDbOnlySkillFlipsRowWithoutWorkspace() {
when(skillMapper.selectById(1L)).thenReturn(archivedSkill()); when(skillMapper.selectById(1L)).thenReturn(archivedSkill());
when(workspaceManager.restoreWorkspace("demo-skill")) when(workspaceManager.restoreWorkspace("demo-skill", 1L))
.thenReturn(SkillWorkspaceManager.RestoreResult.MISSING); .thenReturn(SkillWorkspaceManager.RestoreResult.MISSING);
service.restore(1L); service.restore(1L);
@ -200,7 +201,7 @@ class SkillLifecycleServiceTest {
SkillEntity s = archivedSkill(); SkillEntity s = archivedSkill();
s.setSkillContent(" "); s.setSkillContent(" ");
when(skillMapper.selectById(1L)).thenReturn(s); when(skillMapper.selectById(1L)).thenReturn(s);
when(workspaceManager.restoreWorkspace("demo-skill")) when(workspaceManager.restoreWorkspace("demo-skill", 1L))
.thenReturn(SkillWorkspaceManager.RestoreResult.MISSING); .thenReturn(SkillWorkspaceManager.RestoreResult.MISSING);
assertThrows(MateClawException.class, () -> service.restore(1L)); assertThrows(MateClawException.class, () -> service.restore(1L));
@ -210,7 +211,7 @@ class SkillLifecycleServiceTest {
@Test @Test
void restoreRejectsWhenWorkspaceMoveBackFails() { void restoreRejectsWhenWorkspaceMoveBackFails() {
when(skillMapper.selectById(1L)).thenReturn(archivedSkill()); when(skillMapper.selectById(1L)).thenReturn(archivedSkill());
when(workspaceManager.restoreWorkspace("demo-skill")) when(workspaceManager.restoreWorkspace("demo-skill", 1L))
.thenReturn(SkillWorkspaceManager.RestoreResult.FAILED); .thenReturn(SkillWorkspaceManager.RestoreResult.FAILED);
assertThrows(MateClawException.class, () -> service.restore(1L)); assertThrows(MateClawException.class, () -> service.restore(1L));
@ -234,7 +235,7 @@ class SkillLifecycleServiceTest {
@Test @Test
void archiveDefersWhenWorkspaceMoveFails() { void archiveDefersWhenWorkspaceMoveFails() {
SkillEntity s = skill("dynamic", "stale", now.minusDays(100)); SkillEntity s = skill("dynamic", "stale", now.minusDays(100));
when(workspaceManager.archiveWorkspace(anyString())) when(workspaceManager.archiveWorkspace(anyString(), any()))
.thenReturn(SkillWorkspaceManager.ArchiveResult.FAILED); .thenReturn(SkillWorkspaceManager.ArchiveResult.FAILED);
boolean applied = service.apply(s, LifecycleTransition.TO_ARCHIVED, now); boolean applied = service.apply(s, LifecycleTransition.TO_ARCHIVED, now);
@ -246,7 +247,7 @@ class SkillLifecycleServiceTest {
@Test @Test
void archiveCommitsForDbOnlySkillWithNoWorkspace() { void archiveCommitsForDbOnlySkillWithNoWorkspace() {
SkillEntity s = skill("dynamic", "stale", now.minusDays(100)); SkillEntity s = skill("dynamic", "stale", now.minusDays(100));
when(workspaceManager.archiveWorkspace(anyString())) when(workspaceManager.archiveWorkspace(anyString(), any()))
.thenReturn(SkillWorkspaceManager.ArchiveResult.MISSING); .thenReturn(SkillWorkspaceManager.ArchiveResult.MISSING);
when(skillMapper.update(any(), any())).thenReturn(1); when(skillMapper.update(any(), any())).thenReturn(1);
@ -260,13 +261,13 @@ class SkillLifecycleServiceTest {
@Test @Test
void archiveCompensatesWorkspaceWhenDbWriteTouchesNoRows() { void archiveCompensatesWorkspaceWhenDbWriteTouchesNoRows() {
SkillEntity s = skill("dynamic", "stale", now.minusDays(100)); SkillEntity s = skill("dynamic", "stale", now.minusDays(100));
when(workspaceManager.archiveWorkspace(anyString())) when(workspaceManager.archiveWorkspace(anyString(), any()))
.thenReturn(SkillWorkspaceManager.ArchiveResult.MOVED); .thenReturn(SkillWorkspaceManager.ArchiveResult.MOVED);
when(skillMapper.update(any(), any())).thenReturn(0); when(skillMapper.update(any(), any())).thenReturn(0);
boolean applied = service.apply(s, LifecycleTransition.TO_ARCHIVED, now); boolean applied = service.apply(s, LifecycleTransition.TO_ARCHIVED, now);
assertFalse(applied); assertFalse(applied);
verify(workspaceManager).restoreWorkspace("demo-skill"); verify(workspaceManager).restoreWorkspace("demo-skill", 1L);
} }
} }

View File

@ -17,6 +17,7 @@ import java.nio.file.Path;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
@ -67,6 +68,7 @@ class SkillServiceUpdatePartialTest {
SkillEntity existing = new SkillEntity(); SkillEntity existing = new SkillEntity();
existing.setId(101L); existing.setId(101L);
existing.setWorkspaceId(1L);
existing.setName("docx"); existing.setName("docx");
existing.setDescription("placeholder"); existing.setDescription("placeholder");
existing.setSkillType("dynamic"); existing.setSkillType("dynamic");
@ -86,8 +88,8 @@ class SkillServiceUpdatePartialTest {
Path tempRoot = Files.createTempDirectory("skill-svc-test"); Path tempRoot = Files.createTempDirectory("skill-svc-test");
Path skillDir = tempRoot.resolve("docx"); Path skillDir = tempRoot.resolve("docx");
Files.createDirectories(skillDir); Files.createDirectories(skillDir);
when(workspaceManager.conventionWorkspaceExists("docx")).thenReturn(true); when(workspaceManager.conventionWorkspaceExists("docx", 1L)).thenReturn(true);
when(workspaceManager.resolveConventionPath("docx")).thenReturn(skillDir); when(workspaceManager.resolveConventionPath("docx", 1L)).thenReturn(skillDir);
// What the controller deserializes from the partial PUT body: // What the controller deserializes from the partial PUT body:
// only id + skillContent + sourceCode. // only id + skillContent + sourceCode.
@ -121,7 +123,7 @@ class SkillServiceUpdatePartialTest {
"skill_content from the partial PUT must be applied"); "skill_content from the partial PUT must be applied");
// Workspace sync runs using the merged name, not the partial null. // Workspace sync runs using the merged name, not the partial null.
verify(workspaceManager).conventionWorkspaceExists("docx"); verify(workspaceManager).conventionWorkspaceExists("docx", 1L);
// Best-effort cleanup of the temp workspace. // Best-effort cleanup of the temp workspace.
Files.deleteIfExists(skillDir.resolve("SKILL.md")); Files.deleteIfExists(skillDir.resolve("SKILL.md"));
@ -152,7 +154,7 @@ class SkillServiceUpdatePartialTest {
existing.setBuiltin(false); existing.setBuiltin(false);
existing.setSkillContent("---\nname: notes\n---\n# previously authored body\n"); existing.setSkillContent("---\nname: notes\n---\n# previously authored body\n");
when(mapper.selectById(202L)).thenReturn(existing); when(mapper.selectById(202L)).thenReturn(existing);
when(workspaceManager.conventionWorkspaceExists(anyString())).thenReturn(false); when(workspaceManager.conventionWorkspaceExists(anyString(), any())).thenReturn(false);
// Identity edit: nameZh / description only skill_content is // Identity edit: nameZh / description only skill_content is
// never touched and must survive. // never touched and must survive.

View File

@ -83,7 +83,7 @@ class BundledSkillSyncerTest {
@Test @Test
@DisplayName("Sync self-heals a workspace missing scripts/ despite unchanged version") @DisplayName("Sync self-heals a workspace missing scripts/ despite unchanged version")
void syncForcesDiskCopyWhenScriptsDirMissing() throws IOException { void syncForcesDiskCopyWhenScriptsDirMissing() throws IOException {
Path pptxDir = tmp.resolve("pptx"); Path pptxDir = tmp.resolve("1").resolve("pptx");
Files.createDirectories(pptxDir); Files.createDirectories(pptxDir);
// Same SKILL.md as the bundle (identical version) but no scripts/ // Same SKILL.md as the bundle (identical version) but no scripts/
// directory the state left behind by a build that shipped without // directory the state left behind by a build that shipped without
@ -104,7 +104,7 @@ class BundledSkillSyncerTest {
assertTrue(synced.contains("pptx"), "Should re-sync when scripts directory is missing"); assertTrue(synced.contains("pptx"), "Should re-sync when scripts directory is missing");
assertTrue(Files.isDirectory(pptxDir.resolve("scripts")), "scripts directory should now be on disk"); assertTrue(Files.isDirectory(pptxDir.resolve("scripts")), "scripts directory should now be on disk");
verify(skillFileService, atLeastOnce()).applyBundleFiles(eq(100L), anyMap(), eq(false)); verify(skillFileService, atLeastOnce()).applyBundleFiles(eq(100L), anyMap(), eq(false));
try (var archived = Files.list(tmp.resolve(".archived"))) { try (var archived = Files.list(tmp.resolve("1").resolve(".archived"))) {
assertTrue(archived.anyMatch(p -> p.getFileName().toString().startsWith("pptx-")), assertTrue(archived.anyMatch(p -> p.getFileName().toString().startsWith("pptx-")),
"old workspace should be archived, not overwritten in place"); "old workspace should be archived, not overwritten in place");
} }

View File

@ -70,7 +70,7 @@ class SkillFileSyncerTest {
var report = syncer.syncAll(); var report = syncer.syncAll();
Path workspace = tmp.resolve("demo"); Path workspace = tmp.resolve("1").resolve("demo");
assertEquals("print('a')\n", Files.readString(workspace.resolve("scripts/run.py"))); assertEquals("print('a')\n", Files.readString(workspace.resolve("scripts/run.py")));
assertEquals("hello", Files.readString(workspace.resolve("references/notes.md"))); assertEquals("hello", Files.readString(workspace.resolve("references/notes.md")));
assertEquals(2, report.filesMaterialized()); assertEquals(2, report.filesMaterialized());
@ -84,7 +84,7 @@ class SkillFileSyncerTest {
SkillEntity skill = newSkill(10L, "demo"); SkillEntity skill = newSkill(10L, "demo");
when(skillService.listSkills()).thenReturn(List.of(skill)); when(skillService.listSkills()).thenReturn(List.of(skill));
Path workspace = tmp.resolve("demo"); Path workspace = tmp.resolve("1").resolve("demo");
Files.createDirectories(workspace.resolve("scripts")); Files.createDirectories(workspace.resolve("scripts"));
Files.writeString(workspace.resolve("scripts/run.py"), "stable"); Files.writeString(workspace.resolve("scripts/run.py"), "stable");
@ -104,7 +104,7 @@ class SkillFileSyncerTest {
SkillEntity skill = newSkill(10L, "demo"); SkillEntity skill = newSkill(10L, "demo");
when(skillService.listSkills()).thenReturn(List.of(skill)); when(skillService.listSkills()).thenReturn(List.of(skill));
Path workspace = tmp.resolve("demo"); Path workspace = tmp.resolve("1").resolve("demo");
Files.createDirectories(workspace.resolve("scripts")); Files.createDirectories(workspace.resolve("scripts"));
Files.createDirectories(workspace.resolve("references")); Files.createDirectories(workspace.resolve("references"));
Files.writeString(workspace.resolve("scripts/run.py"), "legacy"); Files.writeString(workspace.resolve("scripts/run.py"), "legacy");
@ -152,6 +152,8 @@ class SkillFileSyncerTest {
SkillEntity s = new SkillEntity(); SkillEntity s = new SkillEntity();
s.setId(id); s.setId(id);
s.setName(name); s.setName(name);
// Workspace-scoped FS layout: the syncer resolves {root}/{workspaceId}/{name}.
s.setWorkspaceId(1L);
return s; return s;
} }

View File

@ -36,20 +36,21 @@ class SkillWorkspaceManagerApplyBundleTest {
props.setRoot(tmp.toString()); props.setRoot(tmp.toString());
ApplicationEventPublisher publisher = mock(ApplicationEventPublisher.class); ApplicationEventPublisher publisher = mock(ApplicationEventPublisher.class);
manager = new SkillWorkspaceManager(props, publisher); manager = new SkillWorkspaceManager(props, publisher);
manager.initWorkspace(skill, "---\nname: demo\n---\nbody\n"); manager.initWorkspace(skill, "---\nname: demo\n---\nbody\n", 1L);
} }
@Test @Test
@DisplayName("write-then-prune: new files added, removed files pruned") @DisplayName("write-then-prune: new files added, removed files pruned")
void writeThenPruneNormalCase() throws IOException { void writeThenPruneNormalCase() throws IOException {
Path scripts = tmp.resolve(skill).resolve("scripts"); Path scripts = tmp.resolve("1").resolve(skill).resolve("scripts");
Files.writeString(scripts.resolve("old.py"), "old"); Files.writeString(scripts.resolve("old.py"), "old");
Files.writeString(scripts.resolve("keep.py"), "v1"); Files.writeString(scripts.resolve("keep.py"), "v1");
var result = manager.applyBundleFiles(skill, var result = manager.applyBundleFiles(skill,
Map.of(), Map.of(),
Map.of("keep.py", "v2", "new.py", "fresh"), Map.of("keep.py", "v2", "new.py", "fresh"),
false); false,
1L);
assertEquals(2, result.scriptsWritten()); assertEquals(2, result.scriptsWritten());
assertEquals(1, result.scriptsPruned(), "old.py should be pruned"); assertEquals(1, result.scriptsPruned(), "old.py should be pruned");
@ -62,14 +63,15 @@ class SkillWorkspaceManagerApplyBundleTest {
@Test @Test
@DisplayName("empty-bundle guard: existing scripts preserved when new bundle has none") @DisplayName("empty-bundle guard: existing scripts preserved when new bundle has none")
void emptyBundleGuardPreservesExistingScripts() throws IOException { void emptyBundleGuardPreservesExistingScripts() throws IOException {
Path scripts = tmp.resolve(skill).resolve("scripts"); Path scripts = tmp.resolve("1").resolve(skill).resolve("scripts");
Files.writeString(scripts.resolve("run.py"), "important"); Files.writeString(scripts.resolve("run.py"), "important");
Files.writeString(scripts.resolve("helper.py"), "more important"); Files.writeString(scripts.resolve("helper.py"), "more important");
var result = manager.applyBundleFiles(skill, var result = manager.applyBundleFiles(skill,
Map.of("notes.md", "ref"), Map.of("notes.md", "ref"),
Map.of(), // empty scripts simulates the issue #104 extractor bug Map.of(), // empty scripts simulates the issue #104 extractor bug
false); false,
1L);
assertEquals(0, result.scriptsWritten()); assertEquals(0, result.scriptsWritten());
assertEquals(0, result.scriptsPruned()); assertEquals(0, result.scriptsPruned());
@ -83,13 +85,14 @@ class SkillWorkspaceManagerApplyBundleTest {
@Test @Test
@DisplayName("force=true bypasses empty-bundle guard and prunes everything") @DisplayName("force=true bypasses empty-bundle guard and prunes everything")
void forceFlagPrunesEvenWhenBundleEmpty() throws IOException { void forceFlagPrunesEvenWhenBundleEmpty() throws IOException {
Path scripts = tmp.resolve(skill).resolve("scripts"); Path scripts = tmp.resolve("1").resolve(skill).resolve("scripts");
Files.writeString(scripts.resolve("doomed.py"), "x"); Files.writeString(scripts.resolve("doomed.py"), "x");
var result = manager.applyBundleFiles(skill, var result = manager.applyBundleFiles(skill,
Map.of(), Map.of(),
Map.of(), Map.of(),
true); true,
1L);
assertFalse(result.scriptsPreservedDueToEmptyBundle()); assertFalse(result.scriptsPreservedDueToEmptyBundle());
assertEquals(1, result.scriptsPruned()); assertEquals(1, result.scriptsPruned());
@ -99,15 +102,16 @@ class SkillWorkspaceManagerApplyBundleTest {
@Test @Test
@DisplayName("references and scripts buckets prune independently") @DisplayName("references and scripts buckets prune independently")
void bucketsAreIndependent() throws IOException { void bucketsAreIndependent() throws IOException {
Path scripts = tmp.resolve(skill).resolve("scripts"); Path scripts = tmp.resolve("1").resolve(skill).resolve("scripts");
Path references = tmp.resolve(skill).resolve("references"); Path references = tmp.resolve("1").resolve(skill).resolve("references");
Files.writeString(scripts.resolve("run.py"), "stay-on-disk"); Files.writeString(scripts.resolve("run.py"), "stay-on-disk");
Files.writeString(references.resolve("notes.md"), "stale-ref"); Files.writeString(references.resolve("notes.md"), "stale-ref");
var result = manager.applyBundleFiles(skill, var result = manager.applyBundleFiles(skill,
Map.of("notes.md", "fresh-ref"), Map.of("notes.md", "fresh-ref"),
Map.of(), // empty scripts preserved Map.of(), // empty scripts preserved
false); false,
1L);
assertTrue(result.scriptsPreservedDueToEmptyBundle()); assertTrue(result.scriptsPreservedDueToEmptyBundle());
assertFalse(result.referencesPreservedDueToEmptyBundle()); assertFalse(result.referencesPreservedDueToEmptyBundle());

View File

@ -5,19 +5,26 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.api.io.TempDir;
import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisher;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
/** /**
* Regression tests for {@link SkillWorkspaceManager#resolveConventionPath}. * Tests for {@link SkillWorkspaceManager#resolveConventionPath} and the
* workspace-scoped layout.
* *
* <p>Issue #254: non-ASCII (e.g. Chinese) skill names collapsed to underscores in the * <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 * 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 * 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 * directories.
* workspace paths stay stable across upgrades. *
* <p>Workspace isolation: the path is scoped by {@code workspaceId}
* ({@code {root}/{workspaceId}/{name}}) so two skills with the SAME name in DIFFERENT
* workspaces no longer share a directory. {@link #migrateLegacyFlatDir} moves a
* pre-existing flat {@code {root}/{name}} directory into its scoped location.
*/ */
class SkillWorkspaceManagerPathTest { class SkillWorkspaceManagerPathTest {
@ -34,24 +41,96 @@ class SkillWorkspaceManagerPathTest {
@DisplayName("distinct non-ASCII names resolve to distinct directories (no collision)") @DisplayName("distinct non-ASCII names resolve to distinct directories (no collision)")
void nonAsciiNamesDoNotCollide() { void nonAsciiNamesDoNotCollide() {
SkillWorkspaceManager m = newManager(); SkillWorkspaceManager m = newManager();
Path a = m.resolveConventionPath("我的技能"); Path a = m.resolveConventionPath("我的技能", 1L);
Path b = m.resolveConventionPath("你的技能"); Path b = m.resolveConventionPath("你的技能", 1L);
assertNotEquals(a, b, "Chinese names must not collapse to the same directory"); assertNotEquals(a, b, "Chinese names must not collapse to the same directory");
assertTrue(a.getFileName().toString().contains("我的技能"), "Unicode letters must be preserved"); assertTrue(a.getFileName().toString().contains("我的技能"), "Unicode letters must be preserved");
assertTrue(b.getFileName().toString().contains("你的技能"), "Unicode letters must be preserved"); assertTrue(b.getFileName().toString().contains("你的技能"), "Unicode letters must be preserved");
} }
@Test @Test
@DisplayName("ASCII name maps to the bare sanitized name with no -hash suffix") @DisplayName("ASCII name maps to {root}/{workspaceId}/{name}")
void asciiNameKeepsBarePath() { void asciiNameScopedByWorkspace() {
SkillWorkspaceManager m = newManager(); SkillWorkspaceManager m = newManager();
assertEquals(tmp.resolve("my-skill"), m.resolveConventionPath("my-skill")); assertEquals(tmp.resolve("1").resolve("my-skill"), m.resolveConventionPath("my-skill", 1L));
} }
@Test @Test
@DisplayName("path is deterministic for the same name") @DisplayName("path is deterministic for the same (name, workspaceId)")
void deterministicForSameName() { void deterministicForSameName() {
SkillWorkspaceManager m = newManager(); SkillWorkspaceManager m = newManager();
assertEquals(m.resolveConventionPath("demo"), m.resolveConventionPath("demo")); assertEquals(m.resolveConventionPath("demo", 1L), m.resolveConventionPath("demo", 1L));
}
// Workspace isolation
@Test
@DisplayName("same name in different workspaces resolves to disjoint directories")
void sameNameDifferentWorkspacesAreDisjoint() {
SkillWorkspaceManager m = newManager();
Path ws1 = m.resolveConventionPath("预约会议", 1L);
Path ws2 = m.resolveConventionPath("预约会议", 2055554700078690305L);
assertNotEquals(ws1, ws2, "same-named skills in different workspaces must not share a directory");
assertTrue(ws1.startsWith(tmp.resolve("1")), "workspace-1 skill lives under {root}/1");
assertTrue(ws2.startsWith(tmp.resolve("2055554700078690305")), "workspace-2 skill lives under {root}/<wsId>");
}
@Test
@DisplayName("null workspaceId falls back to workspace 1 (defensive only)")
void nullWorkspaceIdFallsBackToOne() {
SkillWorkspaceManager m = newManager();
assertEquals(m.resolveConventionPath("demo", 1L), m.resolveConventionPath("demo", null));
}
// Legacy layout migration
@Test
@DisplayName("migrateLegacyFlatDir moves {root}/{name} to {root}/{wsId}/{name}")
void migratesLegacyFlatDir() throws IOException {
SkillWorkspaceManager m = newManager();
Path legacy = tmp.resolve("demo");
Files.createDirectories(legacy.resolve("scripts"));
Files.writeString(legacy.resolve("SKILL.md"), "# demo");
Files.writeString(legacy.resolve("scripts").resolve("run.py"), "print(1)");
boolean moved = m.migrateLegacyFlatDir("demo", 7L);
assertTrue(moved, "a flat dir with SKILL.md should migrate");
Path scoped = tmp.resolve("7").resolve("demo");
assertTrue(Files.exists(scoped.resolve("SKILL.md")), "SKILL.md follows to the scoped location");
assertTrue(Files.exists(scoped.resolve("scripts").resolve("run.py")), "on-disk scripts survive the move");
assertFalse(Files.exists(legacy), "the legacy flat dir is gone after migration");
}
@Test
@DisplayName("migrateLegacyFlatDir is idempotent and never overwrites an existing scoped dir")
void migrationIdempotentAndNonDestructive() throws IOException {
SkillWorkspaceManager m = newManager();
Path legacy = tmp.resolve("demo");
Files.createDirectories(legacy);
Files.writeString(legacy.resolve("SKILL.md"), "# legacy");
Path scoped = tmp.resolve("7").resolve("demo");
Files.createDirectories(scoped);
Files.writeString(scoped.resolve("SKILL.md"), "# already-migrated");
boolean moved = m.migrateLegacyFlatDir("demo", 7L);
assertFalse(moved, "must not move when the scoped target already exists");
assertEquals("# already-migrated", Files.readString(scoped.resolve("SKILL.md")),
"existing scoped content must never be overwritten");
}
@Test
@DisplayName("migrateLegacyFlatDir skips a directory without SKILL.md (e.g. a scoped root)")
void migrationSkipsNonSkillDirs() throws IOException {
SkillWorkspaceManager m = newManager();
// A workspace-scoped root {root}/1 whose child is a skill no top-level SKILL.md.
Path scopedRoot = tmp.resolve("1");
Files.createDirectories(scopedRoot.resolve("inner").resolve("scripts"));
boolean moved = m.migrateLegacyFlatDir("1", 1L);
assertFalse(moved, "a dir without a top-level SKILL.md is not a legacy flat skill workspace");
assertTrue(Files.exists(scopedRoot.resolve("inner")), "the scoped root is left untouched");
} }
} }

View File

@ -49,6 +49,7 @@ class SkillManageToolWriteFileTest {
private SkillEntity skill(String name, boolean builtin) { private SkillEntity skill(String name, boolean builtin) {
SkillEntity s = new SkillEntity(); SkillEntity s = new SkillEntity();
s.setId(42L); s.setId(42L);
s.setWorkspaceId(1L);
s.setName(name); s.setName(name);
s.setBuiltin(builtin); s.setBuiltin(builtin);
s.setSkillContent("---\nname: " + name + "\n---\n# x"); s.setSkillContent("---\nname: " + name + "\n---\n# x");
@ -72,7 +73,7 @@ class SkillManageToolWriteFileTest {
null, null, "scripts/run.sh", null); null, null, "scripts/run.sh", null);
assertTrue(result.startsWith("File 'scripts/run.sh' written"), result); assertTrue(result.startsWith("File 'scripts/run.sh' written"), result);
verify(workspaceManager, times(1)).writeWorkspaceFile("my-skill", "scripts/run.sh", "echo hi"); verify(workspaceManager, times(1)).writeWorkspaceFile("my-skill", "scripts/run.sh", "echo hi", 1L);
// The canonical store row must be written too, not just the FS cache. // The canonical store row must be written too, not just the FS cache.
verify(skillFileService, times(1)).upsertFile(42L, "scripts/run.sh", "echo hi"); verify(skillFileService, times(1)).upsertFile(42L, "scripts/run.sh", "echo hi");
} }
@ -87,7 +88,7 @@ class SkillManageToolWriteFileTest {
null, null, "templates/report.html", null); null, null, "templates/report.html", null);
assertTrue(result.startsWith("File 'templates/report.html' written"), result); assertTrue(result.startsWith("File 'templates/report.html' written"), result);
verify(workspaceManager, times(1)).writeWorkspaceFile("my-skill", "templates/report.html", "<html></html>"); verify(workspaceManager, times(1)).writeWorkspaceFile("my-skill", "templates/report.html", "<html></html>", 1L);
verify(skillFileService, times(1)).upsertFile(42L, "templates/report.html", "<html></html>"); verify(skillFileService, times(1)).upsertFile(42L, "templates/report.html", "<html></html>");
} }
@ -97,7 +98,7 @@ class SkillManageToolWriteFileTest {
String result = tool.skill_manage("write_file", "my-skill", "body", String result = tool.skill_manage("write_file", "my-skill", "body",
null, null, null, null); null, null, null, null);
assertTrue(result.startsWith("Error"), result); assertTrue(result.startsWith("Error"), result);
verify(workspaceManager, never()).writeWorkspaceFile(any(), any(), any()); verify(workspaceManager, never()).writeWorkspaceFile(any(), any(), any(), any());
} }
@Test @Test
@ -107,7 +108,7 @@ class SkillManageToolWriteFileTest {
String result = tool.skill_manage("write_file", "core", "body", String result = tool.skill_manage("write_file", "core", "body",
null, null, "references/x.md", null); null, null, "references/x.md", null);
assertTrue(result.contains("builtin"), result); assertTrue(result.contains("builtin"), result);
verify(workspaceManager, never()).writeWorkspaceFile(any(), any(), any()); verify(workspaceManager, never()).writeWorkspaceFile(any(), any(), any(), any());
} }
@Test @Test
@ -117,7 +118,7 @@ class SkillManageToolWriteFileTest {
String result = tool.skill_manage("write_file", "ghost", "body", String result = tool.skill_manage("write_file", "ghost", "body",
null, null, "references/x.md", null); null, null, "references/x.md", null);
assertTrue(result.contains("not found"), result); assertTrue(result.contains("not found"), result);
verify(workspaceManager, never()).writeWorkspaceFile(any(), any(), any()); verify(workspaceManager, never()).writeWorkspaceFile(any(), any(), any(), any());
} }
@Test @Test
@ -126,7 +127,7 @@ class SkillManageToolWriteFileTest {
when(skillService.findByName("my-skill")).thenReturn(skill("my-skill", false)); when(skillService.findByName("my-skill")).thenReturn(skill("my-skill", false));
scanPasses(); scanPasses();
doThrow(new IllegalArgumentException("Unsafe file path rejected: ../etc/passwd")) doThrow(new IllegalArgumentException("Unsafe file path rejected: ../etc/passwd"))
.when(workspaceManager).writeWorkspaceFile(eq("my-skill"), any(), any()); .when(workspaceManager).writeWorkspaceFile(eq("my-skill"), any(), any(), any());
String result = tool.skill_manage("write_file", "my-skill", "body", String result = tool.skill_manage("write_file", "my-skill", "body",
null, null, "../etc/passwd", null); null, null, "../etc/passwd", null);