refactor(skill): harden bundled-skill DB sync and extend self-heal

mateclaw-server/src/main/java/vip/mate/skill/workspace/BundledSkillSyncer.java

@@ -12,6 +12,7 @@ import vip.mate.skill.service.SkillFileService;
 import vip.mate.skill.service.SkillService;
 import vip.mate.skill.workspace.bundle.ClasspathBundleSource;
 import vip.mate.skill.workspace.bundle.MaterializeOptions;
+import vip.mate.skill.workspace.bundle.SkillBundleFiles;
 import vip.mate.skill.workspace.bundle.SkillBundleMaterializer;
 import vip.mate.skill.workspace.bundle.SkillBundleSource;
 
@@ -21,14 +22,13 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 /**
- * Discovers {@code classpath:skills/*\/SKILL.md} at startup and copies
+ * Discovers {@code classpath:skills/*/SKILL.md} at startup and copies
  * each into the user's skill workspace, upgrading on version bumps.
  *
  * <p>Split out of {@link SkillWorkspaceManager} so the workspace service
@@ -41,9 +41,19 @@ import java.util.regex.Pattern;
  *   <li>Re-install: only when the classpath SKILL.md frontmatter
  *       {@code version} is strictly newer than the workspace copy. The
  *       existing workspace is archived (not deleted) before overwrite.</li>
- *   <li>Same / older / unparseable version: leave the workspace alone so
+ *   <li>Self-heal: when the bundle ships {@code scripts/} or
- *       user edits aren't clobbered.</li>
+ *       {@code references/} but the workspace lacks that directory entirely
+ *       (an install performed from a build whose jar was missing those
+ *       folders), the workspace is archived and re-copied even though the
+ *       version is unchanged.</li>
+ *   <li>Same / older / unparseable version with all buckets present: leave
+ *       the workspace alone so user edits aren't clobbered.</li>
  * </ul>
+ *
+ * <p>Whenever a bundle is copied to disk (install, upgrade, or self-heal),
+ * its {@code scripts/} and {@code references/} files are also persisted to
+ * the canonical {@code mate_skill_file} store so multi-instance deployments
+ * see the same content regardless of which node performed the sync.
  */
 @Slf4j
 @Component
@@ -94,7 +104,8 @@ public class BundledSkillSyncer {
 
     /**
      * Sync a single bundled skill. Returns true if the workspace was
-     * created or upgraded. Same/older versions are no-ops.
+     * created, upgraded, or self-healed. Same/older versions with all
+     * bundle buckets present on disk are no-ops.
      */
     private boolean syncOne(ResourcePatternResolver resolver, String bundledPath,
                              String skillName, Resource manifest) {
@@ -103,59 +114,74 @@ public class BundledSkillSyncer {
 
         SkillBundleSource source = new ClasspathBundleSource(resolver,
                 bundledPath + "/" + skillName);
-        Map<String, String> bundleFiles = loadBundleFilesFromSource(source);
-
-        SkillEntity skill = skillService.findByName(skillName);
-        if (skill != null && skill.getId() != null && !bundleFiles.isEmpty()) {
-            skillFileService.applyBundleFiles(skill.getId(), bundleFiles, false);
-        }
 
         if (!firstInstall) {
             String bundledVersion = readVersion(manifest);
             String workspaceVersion = readVersion(targetDir.resolve("SKILL.md"));
-            boolean missingScriptsOnDisk = hasScriptsInBundle(bundleFiles) && !Files.exists(targetDir.resolve("scripts"));
+            boolean upgrade = bundledVersion != null && isNewerVersion(bundledVersion, workspaceVersion);
-
+            if (upgrade) {
-            if (!missingScriptsOnDisk && (bundledVersion == null || !isNewerVersion(bundledVersion, workspaceVersion))) {
-                log.debug("Bundled skill '{}' workspace is current (bundled={}, workspace={}), skipping",
-                        skillName, bundledVersion, workspaceVersion);
-                return false;
-            }
-            if (!missingScriptsOnDisk) {
                 log.info("Bundled skill '{}' version {} > workspace version {}, upgrading",
                         skillName, bundledVersion, workspaceVersion);
-                workspaceManager.archiveWorkspace(skillName);
             } else {
-                log.info("Bundled skill '{}' is missing scripts directory on disk, force copying bundle", skillName);
+                List<String> missing = missingBucketsOnDisk(source, targetDir);
+                if (missing.isEmpty()) {
+                    log.debug("Bundled skill '{}' workspace is current (bundled={}, workspace={}), skipping",
+                            skillName, bundledVersion, workspaceVersion);
+                    return false;
+                }
+                log.info("Bundled skill '{}' is missing {} on disk, restoring from bundle",
+                        skillName, missing);
             }
+            // Archive (never overwrite in place) so local edits stay recoverable.
+            workspaceManager.archiveWorkspace(skillName);
         }
 
         copyBundle(source, targetDir);
+        syncBundleFilesToDb(skillName, source);
         eventPublisher.publishEvent(
                 new SkillWorkspaceEvent(skillName, SkillWorkspaceEvent.Type.CREATED, targetDir));
         log.info("{} bundled skill '{}' → {}", firstInstall ? "Synced" : "Upgraded", skillName, targetDir);
         return true;
     }
 
-    private Map<String, String> loadBundleFilesFromSource(SkillBundleSource source) {
+    /**
-        Map<String, String> files = new LinkedHashMap<>();
+     * Buckets ({@code scripts/}, {@code references/}) that ship in the
+     * bundle but are absent from the workspace directory — the fingerprint
+     * of an install performed from a build whose jar lacked those folders.
+     */
+    private List<String> missingBucketsOnDisk(SkillBundleSource source, Path targetDir) {
+        List<String> missing = new ArrayList<>();
         try {
-            for (SkillBundleSource.BundleAsset asset : source.assets()) {
+            List<SkillBundleSource.BundleAsset> assets = source.assets();
-                String path = asset.relativePath();
+            for (String prefix : SkillBundleFiles.DB_BUCKET_PREFIXES) {
-                if (path.startsWith("scripts/") || path.startsWith("references/")) {
+                boolean inBundle = assets.stream().anyMatch(a -> a.relativePath().startsWith(prefix));
-                    try (InputStream is = asset.open().get()) {
+                String dirName = prefix.substring(0, prefix.length() - 1);
-                        String content = new String(is.readAllBytes(), StandardCharsets.UTF_8);
+                if (inBundle && !Files.isDirectory(targetDir.resolve(dirName))) {
-                        files.put(path, content);
+                    missing.add(dirName);
-                    }
                 }
             }
-        } catch (Exception e) {
+        } catch (IOException e) {
-            log.warn("Failed to load bundle assets from {}: {}", source.origin(), e.getMessage());
+            log.warn("Failed to enumerate bundle assets from {}: {}", source.origin(), e.getMessage());
         }
-        return files;
+        return missing;
     }
 
-    private boolean hasScriptsInBundle(Map<String, String> bundleFiles) {
+    /**
-        return bundleFiles.keySet().stream().anyMatch(p -> p.startsWith("scripts/"));
+     * Mirror the bundle's DB-persisted buckets into {@code mate_skill_file}.
+     * Skipped silently when the skill row doesn't exist yet (first boot
+     * before seeding) — the skill file syncer's disk backfill covers that
+     * case once the row appears.
+     */
+    private void syncBundleFilesToDb(String skillName, SkillBundleSource source) {
+        SkillEntity skill = skillService.findByName(skillName);
+        if (skill == null || skill.getId() == null) return;
+        try {
+            Map<String, String> bundleFiles = SkillBundleFiles.readDbEligible(source);
+            if (bundleFiles.isEmpty()) return;
+            skillFileService.applyBundleFiles(skill.getId(), bundleFiles, false);
+        } catch (IOException e) {
+            log.warn("Failed to load bundle files from {}: {}", source.origin(), e.getMessage());
+        }
     }
 
     private void copyBundle(SkillBundleSource source, Path targetDir) {

mateclaw-server/src/main/java/vip/mate/skill/workspace/SkillFileSyncer.java

@@ -2,11 +2,16 @@ package vip.mate.skill.workspace;
 
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
+import org.springframework.core.io.support.PathMatchingResourcePatternResolver;
+import org.springframework.core.io.support.ResourcePatternResolver;
 import org.springframework.stereotype.Component;
 import vip.mate.skill.model.SkillEntity;
 import vip.mate.skill.model.SkillFileEntity;
 import vip.mate.skill.service.SkillFileService;
 import vip.mate.skill.service.SkillService;
+import vip.mate.skill.workspace.bundle.ClasspathBundleSource;
+import vip.mate.skill.workspace.bundle.SkillBundleFiles;
+import vip.mate.skill.workspace.bundle.SkillBundleSource;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -15,7 +20,9 @@ import java.nio.file.Path;
 import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -25,8 +32,10 @@ import java.util.Set;
  *
  * <p>Also runs a one-time backfill: for any skill that has on-disk files
  * but no DB rows (typically pre-V112 installs), the local files are read
- * up into the DB so the canonical store catches up to reality. Backfill
+ * up into the DB so the canonical store catches up to reality. Builtin
- * is content-hash idempotent and safe to invoke repeatedly.
+ * skills with neither DB rows nor on-disk files fall back to re-reading
+ * the classpath bundle. Backfill is content-hash idempotent and safe to
+ * invoke repeatedly.
  *
  * <p>Triggered:
  * <ul>
@@ -45,6 +54,7 @@ public class SkillFileSyncer {
     private final SkillService skillService;
     private final SkillFileService skillFileService;
     private final SkillWorkspaceManager workspaceManager;
+    private final SkillWorkspaceProperties workspaceProperties;
 
     /** Aggregate counters for one full sync pass. */
     public record SyncReport(int skillsConsidered,
@@ -124,31 +134,27 @@ public class SkillFileSyncer {
     }
 
     /**
-     * Backfills builtin skill bundle files from classpath when both DB and disk are empty.
+     * Backfills builtin skill bundle files from the classpath when both the
+     * DB and the local workspace are empty — the state left behind by an
+     * install whose jar shipped without {@code scripts/}/{@code references/}.
      */
     private int backfillFromClasspathIfNeeded(SkillEntity skill) {
-        org.springframework.core.io.support.ResourcePatternResolver resolver =
+        String bundledPath = workspaceProperties.getBundledSkillsPath();
-                new org.springframework.core.io.support.PathMatchingResourcePatternResolver();
+        if (bundledPath == null || bundledPath.isBlank()) return 0;
-        String bundledPath = "skills/" + skill.getName();
-        vip.mate.skill.workspace.bundle.SkillBundleSource source =
-                new vip.mate.skill.workspace.bundle.ClasspathBundleSource(resolver, bundledPath);
 
-        java.util.Map<String, String> ingested = new java.util.LinkedHashMap<>();
+        ResourcePatternResolver resolver = new PathMatchingResourcePatternResolver();
+        SkillBundleSource source = new ClasspathBundleSource(resolver,
+                bundledPath + "/" + skill.getName());
+
+        Map<String, String> ingested;
         try {
-            for (vip.mate.skill.workspace.bundle.SkillBundleSource.BundleAsset asset : source.assets()) {
+            ingested = SkillBundleFiles.readDbEligible(source);
-                String relative = asset.relativePath();
+        } catch (IOException e) {
-                if (relative.startsWith("scripts/") || relative.startsWith("references/")) {
-                    try (java.io.InputStream is = asset.open().get()) {
-                        ingested.put(relative, new String(is.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8));
-                    }
-                }
-            }
-        } catch (Exception e) {
             log.warn("Failed to backfill builtin skill '{}' from classpath: {}", skill.getName(), e.getMessage());
             return 0;
         }
-
         if (ingested.isEmpty()) return 0;
+
         skillFileService.applyBundleFiles(skill.getId(), ingested, false);
         log.info("Backfilled {} bundle file(s) from classpath into mate_skill_file for builtin skill '{}' (id={})",
                 ingested.size(), skill.getName(), skill.getId());
@@ -209,7 +215,7 @@ public class SkillFileSyncer {
         if (Files.isDirectory(references)) roots.add(references);
         if (roots.isEmpty()) return 0;
 
-        java.util.Map<String, String> ingested = new java.util.LinkedHashMap<>();
+        Map<String, String> ingested = new LinkedHashMap<>();
         Set<String> seen = new HashSet<>();
         for (Path root : roots) {
             String prefix = workspaceDir.relativize(root).toString().replace('\\', '/') + "/";

mateclaw-server/src/main/java/vip/mate/skill/workspace/bundle/SkillBundleFiles.java

@@ -0,0 +1,52 @@
+package vip.mate.skill.workspace.bundle;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Helpers for the bundle file buckets that mirror into the canonical
+ * {@code mate_skill_file} store ({@code scripts/} and {@code references/}).
+ *
+ * <p>Shared by the bundled-skill startup sync and the skill file syncer's
+ * classpath backfill so both agree on which paths are DB-persisted and how
+ * bundle contents are read into memory.
+ */
+public final class SkillBundleFiles {
+
+    /** Path prefixes of the buckets persisted to {@code mate_skill_file}. */
+    public static final List<String> DB_BUCKET_PREFIXES = List.of("scripts/", "references/");
+
+    private SkillBundleFiles() {
+    }
+
+    /** True when the workspace-relative path belongs to a DB-persisted bucket. */
+    public static boolean isDbEligible(String relativePath) {
+        if (relativePath == null) return false;
+        for (String prefix : DB_BUCKET_PREFIXES) {
+            if (relativePath.startsWith(prefix)) return true;
+        }
+        return false;
+    }
+
+    /**
+     * Read every {@code scripts/} and {@code references/} file of the bundle
+     * into memory, keyed by workspace-relative path (the key shape
+     * {@code SkillFileService#applyBundleFiles} expects). Iteration order
+     * follows {@link SkillBundleSource#assets()} enumeration order.
+     */
+    public static Map<String, String> readDbEligible(SkillBundleSource source) throws IOException {
+        Map<String, String> files = new LinkedHashMap<>();
+        for (SkillBundleSource.BundleAsset asset : source.assets()) {
+            String path = asset.relativePath();
+            if (!isDbEligible(path)) continue;
+            try (InputStream is = asset.open().get()) {
+                files.put(path, new String(is.readAllBytes(), StandardCharsets.UTF_8));
+            }
+        }
+        return files;
+    }
+}

mateclaw-server/src/test/java/vip/mate/skill/workspace/BundledSkillSyncerTest.java

@@ -11,17 +11,26 @@ import vip.mate.skill.service.SkillService;
 import vip.mate.skill.workspace.bundle.SkillBundleMaterializer;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
 
 /**
- * Tests for {@link BundledSkillSyncer} ensuring bundled skill scripts are synced to DB and workspace disk.
+ * Tests for {@link BundledSkillSyncer} covering:
+ *
+ * <ul>
+ *   <li>First install mirrors bundle scripts/references into the DB.</li>
+ *   <li>Missing {@code scripts/} on disk triggers a self-heal copy even
+ *       when the SKILL.md version is unchanged, archiving the old
+ *       workspace first.</li>
+ *   <li>A current workspace with all buckets present is a strict no-op —
+ *       no disk copy, no DB writes.</li>
+ * </ul>
  */
 class BundledSkillSyncerTest {
 
@@ -72,12 +81,18 @@ class BundledSkillSyncerTest {
     }
 
     @Test
-    @DisplayName("Sync forces copy to disk when scripts directory is missing despite unchanged version")
+    @DisplayName("Sync self-heals a workspace missing scripts/ despite unchanged version")
     void syncForcesDiskCopyWhenScriptsDirMissing() throws IOException {
         Path pptxDir = tmp.resolve("pptx");
         Files.createDirectories(pptxDir);
-        // Create SKILL.md with current version but omit scripts directory
+        // Same SKILL.md as the bundle (identical version) but no scripts/
-        Files.writeString(pptxDir.resolve("SKILL.md"), "---\nname: pptx\nversion: 1.0.0\n---\n");
+        // directory — the state left behind by a build that shipped without
+        // bundle scripts.
+        try (InputStream is = getClass().getClassLoader()
+                .getResourceAsStream("skills/pptx/SKILL.md")) {
+            assertNotNull(is, "bundled pptx SKILL.md must exist on the test classpath");
+            Files.write(pptxDir.resolve("SKILL.md"), is.readAllBytes());
+        }
 
         SkillEntity pptxSkill = new SkillEntity();
         pptxSkill.setId(100L);
@@ -86,7 +101,30 @@ class BundledSkillSyncerTest {
 
         List<String> synced = syncer.sync();
 
-        assertTrue(synced.contains("pptx"), "Should force re-sync when scripts directory is missing");
+        assertTrue(synced.contains("pptx"), "Should re-sync when scripts directory is missing");
-        assertTrue(Files.exists(pptxDir.resolve("scripts")), "scripts directory should now be copied to disk");
+        assertTrue(Files.isDirectory(pptxDir.resolve("scripts")), "scripts directory should now be on disk");
+        verify(skillFileService, atLeastOnce()).applyBundleFiles(eq(100L), anyMap(), eq(false));
+        try (var archived = Files.list(tmp.resolve(".archived"))) {
+            assertTrue(archived.anyMatch(p -> p.getFileName().toString().startsWith("pptx-")),
+                    "old workspace should be archived, not overwritten in place");
+        }
+    }
+
+    @Test
+    @DisplayName("Sync is a strict no-op when the workspace is current with all buckets present")
+    void syncSkipsDbWhenWorkspaceCurrent() {
+        // First pass installs every bundled skill into the empty root.
+        syncer.sync();
+        clearInvocations(skillFileService);
+
+        SkillEntity pptxSkill = new SkillEntity();
+        pptxSkill.setId(100L);
+        pptxSkill.setName("pptx");
+        when(skillService.findByName("pptx")).thenReturn(pptxSkill);
+
+        List<String> second = syncer.sync();
+
+        assertFalse(second.contains("pptx"), "Unchanged workspace should not re-sync");
+        verify(skillFileService, never()).applyBundleFiles(anyLong(), anyMap(), anyBoolean());
     }
 }

mateclaw-server/src/test/java/vip/mate/skill/workspace/SkillFileSyncerTest.java

@@ -51,8 +51,9 @@ class SkillFileSyncerTest {
         fileService = new SkillFileService(mapper);
         SkillWorkspaceProperties props = new SkillWorkspaceProperties();
         props.setRoot(tmp.toString());
+        props.setBundledSkillsPath("skills");
         workspaceManager = new SkillWorkspaceManager(props, mock(ApplicationEventPublisher.class));
-        syncer = new SkillFileSyncer(skillService, fileService, workspaceManager);
+        syncer = new SkillFileSyncer(skillService, fileService, workspaceManager, props);
     }
 
     @Test