From 5e188cd77b936822def35bebee56b9614a011a96 Mon Sep 17 00:00:00 2001 From: matevip Date: Wed, 22 Jul 2026 14:20:27 +0800 Subject: [PATCH] =?UTF-8?q?feat(skill):=20single-source=20SKILL.md=20?= =?UTF-8?q?=E2=80=94=20DB=20canonical,=20workspace=20file=20as=20tracked?= =?UTF-8?q?=20cache?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime resolved SKILL.md from the workspace directory while the admin console read the skill_content column, so out-of-band file edits (agent shell tools in a chat session) changed runtime behavior but never showed up in the console, and a failed workspace export left agents executing stale content the console claimed was current. - SkillContentReconciler: three-way sync between the canonical DB column and the convention-workspace file, anchored on a sidecar hash marker. File-side edits ingest into the DB, DB-side edits materialize to the file, two-sided conflicts resolve DB-wins with a backup. - Skill detail GET performs a read-time reconcile and triggers a single-skill rescan when the file side changed, so a console query is always current without waiting for the runtime cache TTL. - SkillMarket detail drawer refetches the row and runtime status on open instead of rendering the page-load list snapshot. --- .../skill/controller/SkillController.java | 16 ++ .../skill/runtime/SkillContentReconciler.java | 258 ++++++++++++++++++ .../skill/runtime/SkillPackageResolver.java | 86 +++++- .../SkillControllerLifecycleTest.java | 2 +- .../SkillControllerListEnabledTest.java | 1 + .../SkillControllerVirtualGuardTest.java | 6 +- .../runtime/SkillContentReconcilerTest.java | 235 ++++++++++++++++ mateclaw-ui/src/views/SkillMarket.vue | 29 ++ 8 files changed, 616 insertions(+), 17 deletions(-) create mode 100644 mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillContentReconciler.java create mode 100644 mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillContentReconcilerTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java b/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java index bbee1d22..feae1b20 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java @@ -17,6 +17,7 @@ import vip.mate.skill.lessons.SkillLessonsService; import vip.mate.skill.manifest.SkillManifest; import vip.mate.skill.model.SkillEntity; import vip.mate.skill.runtime.SkillDependencyChecker; +import vip.mate.skill.runtime.SkillPackageResolver; import vip.mate.skill.runtime.SkillCatalogSort; import vip.mate.skill.runtime.SkillCatalogSorter; import vip.mate.skill.service.SkillService; @@ -57,6 +58,7 @@ public class SkillController { private final SkillService skillService; private final SkillRuntimeService skillRuntimeService; + private final SkillPackageResolver skillPackageResolver; private final SkillWorkspaceManager workspaceManager; private final BundledSkillSyncer bundledSkillSyncer; private final SkillFileSyncer skillFileSyncer; @@ -445,6 +447,20 @@ public class SkillController { } SkillEntity skill = skillService.getSkill(id); verifyResourceWorkspace(skill, workspaceId); + // Read-time store reconciliation: the workspace SKILL.md may have + // been edited out-of-band (agent shell tools in a chat session) + // with no refresh in between. One file read + hash compare in the + // steady state; when the file side actually changed, the content + // is ingested and a single-skill rescan re-projects manifest + // columns and drops stale runtime caches — so the detail view + // always shows what the runtime executes. + try { + if (skillPackageResolver.reconcileEntityContent(skill)) { + skillRuntimeService.rescanSingle(skill); + } + } catch (Exception ignored) { + // A reconcile failure must not break the detail view. + } return R.ok(skill); } diff --git a/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillContentReconciler.java b/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillContentReconciler.java new file mode 100644 index 00000000..7a37a4a6 --- /dev/null +++ b/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillContentReconciler.java @@ -0,0 +1,258 @@ +package vip.mate.skill.runtime; + +import com.baomidou.mybatisplus.core.conditions.update.LambdaUpdateWrapper; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Component; +import vip.mate.skill.model.SkillEntity; +import vip.mate.skill.repository.SkillMapper; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.time.LocalDateTime; +import java.util.HexFormat; + +/** + * Reconciles a skill's SKILL.md between its two stores: the canonical + * {@code mate_skill.skill_content} column and the convention-workspace + * file cache ({@code {workspace-root}/{name}/SKILL.md}). + * + *

The database column is the single source of truth; the workspace file + * is a materialized cache kept for script execution and direct-file + * tooling. Because agents (via shell tools) and operators may still edit + * the file in place, reconciliation is a three-way sync anchored on a + * sidecar marker ({@value #SYNC_MARKER}) that records the SHA-256 of the + * content at the last successful sync: + * + *

+ * + *

All writes are idempotent and failure-tolerant: an IO or DB error + * logs a warning and leaves the marker untouched, so the next resolve + * pass retries the same reconciliation. + * + * @author MateClaw Team + */ +@Slf4j +@Component +@RequiredArgsConstructor +public class SkillContentReconciler { + + /** Sidecar file holding the SHA-256 of SKILL.md at the last sync. */ + public static final String SYNC_MARKER = ".skillmd.sha256"; + + /** Backup name for a file-side edit that loses a two-sided conflict. */ + static final String CONFLICT_BACKUP = "SKILL.md.bak"; + + private final SkillMapper skillMapper; + + /** What the reconciliation pass did. */ + public enum Action { + /** Both stores already held the same content. */ + IN_SYNC, + /** DB was blank; the workspace file was ingested into the DB. */ + BACKFILLED_TO_DB, + /** The workspace file changed; its content was written to the DB. */ + INGESTED_TO_DB, + /** The DB changed; its content was written to the workspace file. */ + MATERIALIZED_TO_FS, + /** Both changed; DB won and the file edit was backed up. */ + CONFLICT_DB_WON, + /** A store write failed; stores may still diverge. Retried next pass. */ + FAILED + } + + /** Reconciled content (the value both stores now agree on) + what happened. */ + public record Outcome(String content, Action action) {} + + /** + * Reconcile {@code entity}'s skill_content with the SKILL.md inside + * {@code workspaceDir}. On an ingest/backfill the passed entity's + * in-memory {@code skillContent} is updated too, so downstream resolve + * stages and diff-based write-backs see the merged value. + */ + public Outcome reconcile(SkillEntity entity, Path workspaceDir) { + String dbContent = entity.getSkillContent() == null ? "" : entity.getSkillContent(); + Path skillMd = workspaceDir.resolve("SKILL.md"); + String fsContent = readFileQuietly(skillMd); + + String dbHash = sha256(dbContent); + String fsHash = sha256(fsContent); + Path marker = workspaceDir.resolve(SYNC_MARKER); + String syncedHash = readMarker(marker); + + if (dbHash.equals(fsHash)) { + if (!dbHash.equals(syncedHash)) { + writeMarkerQuietly(marker, dbHash); + } + return new Outcome(dbContent, Action.IN_SYNC); + } + + if (fsContent.isBlank()) { + // File missing or empty while the DB has content: always + // materialize. Blank file content is never treated as an edit — + // that guard blocks the same wipe-on-empty scenario the bundle + // apply path protects against. + return materialize(entity, skillMd, marker, dbContent, dbHash, Action.MATERIALIZED_TO_FS); + } + + if (dbContent.isBlank()) { + return ingest(entity, marker, fsContent, fsHash, Action.BACKFILLED_TO_DB); + } + + // Both sides non-blank and different — use the marker to decide + // which side moved since the last sync. + if (syncedHash == null || syncedHash.equals(dbHash)) { + // DB unchanged since last sync (or legacy pre-marker state, + // where the directory was the effective runtime source): + // the file edit is the newer fact — ingest it. + return ingest(entity, marker, fsContent, fsHash, Action.INGESTED_TO_DB); + } + if (syncedHash.equals(fsHash)) { + // File unchanged since last sync; the DB moved — materialize. + return materialize(entity, skillMd, marker, dbContent, dbHash, Action.MATERIALIZED_TO_FS); + } + + // Both sides changed since the last sync. The DB is canonical, so + // it wins; keep the losing file edit next to the file for manual + // recovery instead of silently discarding it. + backupQuietly(skillMd, workspaceDir.resolve(CONFLICT_BACKUP)); + log.warn("SKILL.md conflict for skill '{}': both DB and workspace file changed since last sync; " + + "DB content wins, file edit saved as {}", entity.getName(), CONFLICT_BACKUP); + return materialize(entity, skillMd, marker, dbContent, dbHash, Action.CONFLICT_DB_WON); + } + + /** + * Mirror a file-authoritative skill's content into the DB column so + * DB-reading consumers (admin console, API) see what the runtime + * actually executes. Used for skills with an explicitly configured + * {@code skillDir}, where the user-managed directory — not the DB — + * is the source of truth and no marker/backfill dance applies. + */ + public void mirrorToDb(SkillEntity entity, String fsContent) { + if (fsContent == null || fsContent.isBlank()) return; + String dbContent = entity.getSkillContent() == null ? "" : entity.getSkillContent(); + if (fsContent.equals(dbContent)) return; + if (writeDb(entity, fsContent)) { + log.info("Mirrored directory SKILL.md into skill_content for skill '{}' (explicit skillDir)", + entity.getName()); + } + } + + private Outcome ingest(SkillEntity entity, Path marker, String fsContent, String fsHash, Action action) { + if (!writeDb(entity, fsContent)) { + return new Outcome(fsContent, Action.FAILED); + } + writeMarkerQuietly(marker, fsHash); + log.info("Ingested workspace SKILL.md into skill_content for skill '{}' ({})", + entity.getName(), action); + return new Outcome(fsContent, action); + } + + private Outcome materialize(SkillEntity entity, Path skillMd, Path marker, + String dbContent, String dbHash, Action action) { + try { + Files.createDirectories(skillMd.getParent()); + Files.writeString(skillMd, dbContent, StandardCharsets.UTF_8); + } catch (IOException e) { + log.warn("Failed to materialize SKILL.md for skill '{}' → {}: {}", + entity.getName(), skillMd, e.getMessage()); + return new Outcome(dbContent, Action.FAILED); + } + writeMarkerQuietly(marker, dbHash); + log.info("Materialized skill_content to workspace SKILL.md for skill '{}' ({})", + entity.getName(), action); + return new Outcome(dbContent, action); + } + + /** + * Column-whitelisted DB write. {@code SkillEntity} declares several + * {@code FieldStrategy.ALWAYS} columns, so a partial + * {@code updateById} would null them out — the update wrapper touches + * only {@code skill_content} and {@code update_time}. + */ + private boolean writeDb(SkillEntity entity, String content) { + if (entity.getId() == null) return false; + try { + skillMapper.update(null, new LambdaUpdateWrapper() + .eq(SkillEntity::getId, entity.getId()) + .set(SkillEntity::getSkillContent, content) + .set(SkillEntity::getUpdateTime, LocalDateTime.now())); + entity.setSkillContent(content); + return true; + } catch (Exception e) { + log.warn("Failed to write skill_content for skill '{}': {}", entity.getName(), e.getMessage()); + return false; + } + } + + private String readFileQuietly(Path file) { + if (!Files.exists(file)) return ""; + try { + return Files.readString(file, StandardCharsets.UTF_8); + } catch (IOException e) { + log.warn("Failed to read {}: {}", file, e.getMessage()); + return ""; + } + } + + private String readMarker(Path marker) { + if (!Files.exists(marker)) return null; + try { + String value = Files.readString(marker, StandardCharsets.UTF_8).strip(); + return value.isEmpty() ? null : value; + } catch (IOException e) { + log.warn("Failed to read sync marker {}: {}", marker, e.getMessage()); + return null; + } + } + + private void writeMarkerQuietly(Path marker, String hash) { + try { + Files.createDirectories(marker.getParent()); + Files.writeString(marker, hash, StandardCharsets.UTF_8); + } catch (IOException e) { + log.warn("Failed to write sync marker {}: {}", marker, e.getMessage()); + } + } + + private void backupQuietly(Path source, Path backup) { + try { + if (Files.exists(source)) { + Files.copy(source, backup, StandardCopyOption.REPLACE_EXISTING); + } + } catch (IOException e) { + log.warn("Failed to back up {} → {}: {}", source, backup, e.getMessage()); + } + } + + static String sha256(String content) { + try { + MessageDigest md = MessageDigest.getInstance("SHA-256"); + return HexFormat.of().formatHex( + md.digest((content == null ? "" : content).getBytes(StandardCharsets.UTF_8))); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 unavailable on this JVM", e); + } + } +} diff --git a/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillPackageResolver.java b/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillPackageResolver.java index 25903b0b..4f78a39e 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillPackageResolver.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillPackageResolver.java @@ -59,6 +59,7 @@ public class SkillPackageResolver { private final ObjectMapper objectMapper; private final SkillWorkspaceManager workspaceManager; private final SkillMapper skillMapper; + private final SkillContentReconciler contentReconciler; /** * RFC-090 §14.4 — knowledge-skill wrapper tool factory. * {@code @Lazy} because WikiSkillWrapperToolFactory pulls in Wiki @@ -113,6 +114,7 @@ public class SkillPackageResolver { ObjectMapper objectMapper, SkillWorkspaceManager workspaceManager, SkillMapper skillMapper, + SkillContentReconciler contentReconciler, @Lazy WikiSkillWrapperToolFactory wikiWrapperFactory, @Lazy AcpSkillWrapperToolFactory acpWrapperFactory, @Lazy ScriptSkillWrapperToolFactory scriptWrapperFactory, @@ -125,6 +127,7 @@ public class SkillPackageResolver { this.objectMapper = objectMapper; this.workspaceManager = workspaceManager; this.skillMapper = skillMapper; + this.contentReconciler = contentReconciler; this.wikiWrapperFactory = wikiWrapperFactory; this.acpWrapperFactory = acpWrapperFactory; this.scriptWrapperFactory = scriptWrapperFactory; @@ -175,6 +178,48 @@ public class SkillPackageResolver { return resolved; } + /** + * Content-store-only reconciliation for read paths (e.g. the admin + * detail view). Runs the same SKILL.md store sync a full resolve + * performs — without the scan / dependency / manifest stages — so a + * detail query always returns the content the runtime would execute, + * even when the workspace file was edited out-of-band (agent shell + * tools, manual edits) and no refresh has run yet. + * + *

Mutates the passed entity's {@code skillContent} when the file + * side wins. + * + * @return {@code true} when the DB side changed — callers should then + * trigger a single-skill rescan so caches and manifest + * projections catch up. + */ + public boolean reconcileEntityContent(SkillEntity entity) { + if (entity == null || entity.getId() == null || entity.getName() == null) return false; + + String configuredDir = extractSkillDirString(entity); + if (configuredDir != null) { + Path explicit = Paths.get(configuredDir); + if (Files.exists(explicit) && Files.isDirectory(explicit)) { + Path skillMd = explicit.resolve("SKILL.md"); + if (!Files.exists(skillMd)) return false; + try { + String before = entity.getSkillContent(); + contentReconciler.mirrorToDb(entity, Files.readString(skillMd)); + return !Objects.equals(before, entity.getSkillContent()); + } catch (Exception e) { + log.warn("Read-time mirror failed for skill '{}': {}", entity.getName(), e.getMessage()); + return false; + } + } + } + + Path convention = workspaceManager.resolveConventionPath(entity.getName()); + if (!Files.exists(convention) || !Files.isDirectory(convention)) return false; + SkillContentReconciler.Outcome outcome = contentReconciler.reconcile(entity, convention); + return outcome.action() == SkillContentReconciler.Action.INGESTED_TO_DB + || outcome.action() == SkillContentReconciler.Action.BACKFILLED_TO_DB; + } + /** * Write back the latest scan status / findings JSON / timestamp when * they differ from what's already on the row. Keeps the DB in sync @@ -308,20 +353,35 @@ public class SkillPackageResolver { // ==================== 阶段 1：内容解析 ==================== private ResolvedSkill resolveFromDirectory(SkillEntity entity, Path skillDir, String configuredDir, String source) { - Path skillMd = skillDir.resolve("SKILL.md"); - - String content = ""; - String description = entity.getDescription(); - - if (Files.exists(skillMd)) { - try { - content = Files.readString(skillMd); - SkillFrontmatterParser.ParsedSkillMd parsed = frontmatterParser.parse(content); - if (!parsed.getDescription().isBlank()) { - description = parsed.getDescription(); + String content; + if ("convention".equals(source)) { + // Convention workspace: the DB column is canonical and the + // directory is a materialized cache. Reconcile both stores so + // runtime content and DB-reading consumers (admin console, API) + // can never diverge — file edits made by agents/shell are + // ingested into the DB, DB edits are materialized to the file. + content = contentReconciler.reconcile(entity, skillDir).content(); + } else { + // Explicit skillDir: the user-managed directory is the source + // of truth. Never write into it; mirror its content into the + // DB column so the console shows what actually executes. + Path skillMd = skillDir.resolve("SKILL.md"); + content = ""; + if (Files.exists(skillMd)) { + try { + content = Files.readString(skillMd); + contentReconciler.mirrorToDb(entity, content); + } catch (Exception e) { + log.warn("Failed to read SKILL.md from {}: {}", skillMd, e.getMessage()); } - } catch (Exception e) { - log.warn("Failed to read SKILL.md from {}: {}", skillMd, e.getMessage()); + } + } + + String description = entity.getDescription(); + if (!content.isBlank()) { + SkillFrontmatterParser.ParsedSkillMd parsed = frontmatterParser.parse(content); + if (!parsed.getDescription().isBlank()) { + description = parsed.getDescription(); } } diff --git a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerLifecycleTest.java b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerLifecycleTest.java index 32db9f24..8bbb28d5 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerLifecycleTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerLifecycleTest.java @@ -56,7 +56,7 @@ class SkillControllerLifecycleTest { @BeforeEach void setUp() { controller = new SkillController( - skillService, null, null, null, null, null, null, null, null, null, + skillService, null, null, null, null, null, null, null, null, null, null, agentBindingService, null, null, skillLifecycleService, skillCuratorJob, skillCuratorReportStore); } diff --git a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java index b8dd7d92..5a107fa5 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java @@ -41,6 +41,7 @@ class SkillControllerListEnabledTest { controller = new SkillController( skillService, /* skillRuntimeService */ null, + /* skillPackageResolver */ null, /* workspaceManager */ null, /* bundledSkillSyncer */ null, /* skillFileSyncer */ null, diff --git a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java index d9463b0c..141df1af 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java @@ -26,7 +26,7 @@ class SkillControllerVirtualGuardTest { private final SkillController controller = new SkillController( null, null, null, null, null, null, null, null, null, null, null, null, null, - null, null, null); + null, null, null, null); @Test @DisplayName("update on a virtual MCP skill id is rejected before hitting the service") @@ -64,7 +64,7 @@ class SkillControllerVirtualGuardTest { void toggleForwardsVirtualMcpToBridge() { McpSkillBridge bridge = mock(McpSkillBridge.class); SkillController c = new SkillController( - null, null, null, null, null, null, null, null, null, null, null, + null, null, null, null, null, null, null, null, null, null, null, null, bridge, null, null, null, null); long virtualMcpId = McpSkillBridge.VIRTUAL_ID_BASE + 42L; SkillEntity toggled = new SkillEntity(); @@ -98,7 +98,7 @@ class SkillControllerVirtualGuardTest { SkillController real = new SkillController( mock(vip.mate.skill.service.SkillService.class), null, null, null, null, null, null, null, null, null, null, null, null, - null, null, null); + null, null, null, null); long snowflakeId = 1_900_000_001_000_000_902L; // updateSkill on a mocked SkillService returns null without throwing, // which is fine — we just need to confirm the guard didn't fire. diff --git a/mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillContentReconcilerTest.java b/mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillContentReconcilerTest.java new file mode 100644 index 00000000..18893638 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillContentReconcilerTest.java @@ -0,0 +1,235 @@ +package vip.mate.skill.runtime; + +import com.baomidou.mybatisplus.core.conditions.Wrapper; +import com.baomidou.mybatisplus.core.conditions.update.LambdaUpdateWrapper; +import com.baomidou.mybatisplus.core.metadata.TableInfoHelper; +import org.apache.ibatis.builder.MapperBuilderAssistant; +import org.apache.ibatis.session.Configuration; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; +import vip.mate.skill.model.SkillEntity; +import vip.mate.skill.repository.SkillMapper; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +/** + * Three-way SKILL.md reconciliation between the canonical DB column and + * the convention-workspace file cache. + */ +class SkillContentReconcilerTest { + + @TempDir + Path workspaceDir; + + private SkillMapper skillMapper; + private SkillContentReconciler reconciler; + + @BeforeAll + static void initTableInfo() { + // Lambda wrappers resolve column names from MyBatis-Plus's static + // TableInfo cache; in a Spring context this happens during mapper + // scan, in a plain unit test we trigger it manually. + TableInfoHelper.initTableInfo( + new MapperBuilderAssistant(new Configuration(), ""), + SkillEntity.class); + } + + @BeforeEach + void setUp() { + skillMapper = mock(SkillMapper.class); + reconciler = new SkillContentReconciler(skillMapper); + } + + private SkillEntity entity(String dbContent) { + SkillEntity e = new SkillEntity(); + e.setId(1L); + e.setName("demo-skill"); + e.setSkillContent(dbContent); + return e; + } + + private void writeFile(String content) throws IOException { + Files.writeString(workspaceDir.resolve("SKILL.md"), content); + } + + private void writeMarker(String ofContent) throws IOException { + Files.writeString(workspaceDir.resolve(SkillContentReconciler.SYNC_MARKER), + SkillContentReconciler.sha256(ofContent)); + } + + private String fileContent() throws IOException { + return Files.readString(workspaceDir.resolve("SKILL.md")); + } + + private String markerContent() throws IOException { + return Files.readString(workspaceDir.resolve(SkillContentReconciler.SYNC_MARKER)).strip(); + } + + // ==================== in sync ==================== + + @Test + void inSyncHealsMissingMarker() throws IOException { + writeFile("same"); + var outcome = reconciler.reconcile(entity("same"), workspaceDir); + + assertThat(outcome.action()).isEqualTo(SkillContentReconciler.Action.IN_SYNC); + assertThat(outcome.content()).isEqualTo("same"); + assertThat(markerContent()).isEqualTo(SkillContentReconciler.sha256("same")); + verify(skillMapper, never()).update(isNull(), any(Wrapper.class)); + } + + // ==================== blank-side guards ==================== + + @Test + void missingFileMaterializesFromDb() throws IOException { + var outcome = reconciler.reconcile(entity("db content"), workspaceDir); + + assertThat(outcome.action()).isEqualTo(SkillContentReconciler.Action.MATERIALIZED_TO_FS); + assertThat(fileContent()).isEqualTo("db content"); + assertThat(markerContent()).isEqualTo(SkillContentReconciler.sha256("db content")); + verify(skillMapper, never()).update(isNull(), any(Wrapper.class)); + } + + @Test + void blankFileNeverIngestedOverDbContent() throws IOException { + // A truncated/emptied file must not wipe the canonical content, + // even when the marker says the DB side is unchanged. + writeFile(""); + writeMarker("db content"); + + var outcome = reconciler.reconcile(entity("db content"), workspaceDir); + + assertThat(outcome.action()).isEqualTo(SkillContentReconciler.Action.MATERIALIZED_TO_FS); + assertThat(fileContent()).isEqualTo("db content"); + verify(skillMapper, never()).update(isNull(), any(Wrapper.class)); + } + + @Test + void blankDbBackfillsFromFile() throws IOException { + writeFile("file content"); + SkillEntity e = entity(null); + + var outcome = reconciler.reconcile(e, workspaceDir); + + assertThat(outcome.action()).isEqualTo(SkillContentReconciler.Action.BACKFILLED_TO_DB); + assertThat(outcome.content()).isEqualTo("file content"); + assertThat(e.getSkillContent()).isEqualTo("file content"); + assertThat(markerContent()).isEqualTo(SkillContentReconciler.sha256("file content")); + verify(skillMapper).update(isNull(), any(Wrapper.class)); + } + + // ==================== single-side changes ==================== + + @Test + void fileEditSinceLastSyncIsIngestedToDb() throws IOException { + // Marker == DB hash → the DB did not move; the file edit wins. + writeFile("edited via shell"); + writeMarker("db content"); + SkillEntity e = entity("db content"); + + var outcome = reconciler.reconcile(e, workspaceDir); + + assertThat(outcome.action()).isEqualTo(SkillContentReconciler.Action.INGESTED_TO_DB); + assertThat(outcome.content()).isEqualTo("edited via shell"); + assertThat(e.getSkillContent()).isEqualTo("edited via shell"); + assertThat(markerContent()).isEqualTo(SkillContentReconciler.sha256("edited via shell")); + verify(skillMapper).update(isNull(), any(Wrapper.class)); + } + + @Test + void dbEditSinceLastSyncIsMaterializedToFile() throws IOException { + // Marker == file hash → the file did not move; the DB edit wins. + writeFile("old content"); + writeMarker("old content"); + + var outcome = reconciler.reconcile(entity("new db content"), workspaceDir); + + assertThat(outcome.action()).isEqualTo(SkillContentReconciler.Action.MATERIALIZED_TO_FS); + assertThat(fileContent()).isEqualTo("new db content"); + assertThat(markerContent()).isEqualTo(SkillContentReconciler.sha256("new db content")); + verify(skillMapper, never()).update(isNull(), any(Wrapper.class)); + } + + // ==================== legacy + conflict ==================== + + @Test + void legacyDivergenceWithoutMarkerLetsFileWinOnce() throws IOException { + // Pre-marker installs resolved runtime content from the directory, + // so on first reconcile the file reflects what was in effect. + writeFile("file version"); + SkillEntity e = entity("db version"); + + var outcome = reconciler.reconcile(e, workspaceDir); + + assertThat(outcome.action()).isEqualTo(SkillContentReconciler.Action.INGESTED_TO_DB); + assertThat(e.getSkillContent()).isEqualTo("file version"); + } + + @Test + void twoSidedConflictDbWinsAndFileIsBackedUp() throws IOException { + writeFile("file edit"); + writeMarker("common ancestor"); + + var outcome = reconciler.reconcile(entity("db edit"), workspaceDir); + + assertThat(outcome.action()).isEqualTo(SkillContentReconciler.Action.CONFLICT_DB_WON); + assertThat(fileContent()).isEqualTo("db edit"); + assertThat(Files.readString(workspaceDir.resolve(SkillContentReconciler.CONFLICT_BACKUP))) + .isEqualTo("file edit"); + assertThat(markerContent()).isEqualTo(SkillContentReconciler.sha256("db edit")); + verify(skillMapper, never()).update(isNull(), any(Wrapper.class)); + } + + // ==================== ingest write shape ==================== + + @Test + void ingestWritesViaColumnWhitelistedWrapper() throws IOException { + writeFile("edited via shell"); + writeMarker("db content"); + + reconciler.reconcile(entity("db content"), workspaceDir); + + // The write must go through an update wrapper (column whitelist), + // not updateById — SkillEntity carries FieldStrategy.ALWAYS columns + // that a partial updateById would null out. + @SuppressWarnings("unchecked") + ArgumentCaptor> captor = + ArgumentCaptor.forClass(LambdaUpdateWrapper.class); + verify(skillMapper).update(isNull(), captor.capture()); + assertThat(captor.getValue().getSqlSet()).contains("skill_content"); + } + + // ==================== explicit-dir mirror ==================== + + @Test + void mirrorToDbWritesOnlyWhenDifferent() { + SkillEntity e = entity("same"); + reconciler.mirrorToDb(e, "same"); + verify(skillMapper, never()).update(isNull(), any(Wrapper.class)); + + reconciler.mirrorToDb(e, "changed"); + verify(skillMapper).update(isNull(), any(Wrapper.class)); + assertThat(e.getSkillContent()).isEqualTo("changed"); + } + + @Test + void mirrorToDbIgnoresBlankFileContent() { + SkillEntity e = entity("db content"); + reconciler.mirrorToDb(e, ""); + reconciler.mirrorToDb(e, null); + verify(skillMapper, never()).update(isNull(), any(Wrapper.class)); + assertThat(e.getSkillContent()).isEqualTo("db content"); + } +} diff --git a/mateclaw-ui/src/views/SkillMarket.vue b/mateclaw-ui/src/views/SkillMarket.vue index c2025196..a53d22ea 100644 --- a/mateclaw-ui/src/views/SkillMarket.vue +++ b/mateclaw-ui/src/views/SkillMarket.vue @@ -894,6 +894,35 @@ function openDetailDrawer( editingIdentity.value = false editingBody.value = false detailDrawerVisible.value = true + // The list rows are a snapshot from page load; the skill may have been + // modified since (e.g. by an agent in a chat session). Refetch the row + // so the drawer never shows stale content; the edit forms are seeded + // only after the fresh row lands. + void refreshDetailSkill(skill, opts) +} + +/** Refetch the drawer's skill from the backend and sync the list row. + * Edit-mode seeding waits for the fresh row so the user never edits a + * stale body. On fetch failure the snapshot stays and edit mode still + * opens (last-known content beats a dead drawer). */ +async function refreshDetailSkill(skill: Skill, opts: { editIdentity?: boolean; editBody?: boolean } = {}) { + if (!isVirtualSkillId(skill.id)) { + try { + const res: any = await skillApi.get(skill.id) + const fresh: Skill | undefined = res?.data + // Apply only if the drawer still shows this skill. + if (fresh && detailSkill.value && String(detailSkill.value.id) === String(fresh.id)) { + detailSkill.value = { ...detailSkill.value, ...fresh } + patchSkillInPlace(fresh) + } + } catch { + // Keep the list snapshot; the drawer still renders. + } + // Runtime-derived tabs (tools / features / security) read from the + // status map — refresh it too so a just-rescanned skill shows current. + void loadRuntimeStatus() + } + if (!detailSkill.value || String(detailSkill.value.id) !== String(skill.id)) return if (opts.editIdentity) startEditIdentity() if (opts.editBody) startEditBody() }