mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-14 19:45:08 +08:00
perf(skill): startup snapshot + builtin scan skip + prompt slim-down
This commit is contained in:
parent
f0fbca803c
commit
ba167e09f4
@ -1,9 +1,11 @@
|
|||||||
package vip.mate.skill.installer;
|
package vip.mate.skill.installer;
|
||||||
|
|
||||||
import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
|
import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
|
||||||
|
import com.fasterxml.jackson.core.type.TypeReference;
|
||||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||||
import lombok.RequiredArgsConstructor;
|
import lombok.RequiredArgsConstructor;
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
|
import org.springframework.beans.factory.annotation.Value;
|
||||||
import org.springframework.boot.ApplicationArguments;
|
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;
|
||||||
@ -15,13 +17,19 @@ import vip.mate.skill.model.SkillEntity;
|
|||||||
import vip.mate.skill.repository.SkillMapper;
|
import vip.mate.skill.repository.SkillMapper;
|
||||||
import vip.mate.skill.runtime.SkillFrontmatterParser;
|
import vip.mate.skill.runtime.SkillFrontmatterParser;
|
||||||
|
|
||||||
|
import java.io.IOException;
|
||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
|
import java.net.URLConnection;
|
||||||
import java.nio.charset.StandardCharsets;
|
import java.nio.charset.StandardCharsets;
|
||||||
|
import java.nio.file.Files;
|
||||||
|
import java.nio.file.Path;
|
||||||
|
import java.nio.file.Paths;
|
||||||
import java.time.LocalDateTime;
|
import java.time.LocalDateTime;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Objects;
|
import java.util.Objects;
|
||||||
|
import java.util.TreeMap;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Builtin skill seed service — RFC-044 §4.2.
|
* Builtin skill seed service — RFC-044 §4.2.
|
||||||
@ -58,10 +66,19 @@ public class BuiltinSkillSeedService implements ApplicationRunner {
|
|||||||
private static final String DEFAULT_VERSION = "1.0.0";
|
private static final String DEFAULT_VERSION = "1.0.0";
|
||||||
private static final String SKILL_TYPE_BUILTIN = "builtin";
|
private static final String SKILL_TYPE_BUILTIN = "builtin";
|
||||||
|
|
||||||
|
/** Snapshot version — bump on any schema change inside the JSON. */
|
||||||
|
private static final int SNAPSHOT_VERSION = 1;
|
||||||
|
|
||||||
private final SkillMapper skillMapper;
|
private final SkillMapper skillMapper;
|
||||||
private final SkillFrontmatterParser frontmatterParser;
|
private final SkillFrontmatterParser frontmatterParser;
|
||||||
private final ObjectMapper objectMapper;
|
private final ObjectMapper objectMapper;
|
||||||
|
|
||||||
|
/** Workspace root, used as the parent of the snapshot file. Mirrors
|
||||||
|
* {@code SkillWorkspaceProperties#root} so we don't drag the whole
|
||||||
|
* properties bean in for one path lookup. */
|
||||||
|
@Value("${mateclaw.skill.workspace.root:#{systemProperties['user.home'] + '/.mateclaw/skills'}}")
|
||||||
|
private String workspaceRoot;
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void run(ApplicationArguments args) {
|
public void run(ApplicationArguments args) {
|
||||||
try {
|
try {
|
||||||
@ -82,6 +99,22 @@ public class BuiltinSkillSeedService implements ApplicationRunner {
|
|||||||
return new SyncStats(0, 0, 0, 0);
|
return new SyncStats(0, 0, 0, 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Fast path — if every SKILL.md's (size, mtime) matches the snapshot
|
||||||
|
// from the previous successful run AND the DB still holds the same
|
||||||
|
// number of builtin rows, nothing on disk changed since last seed
|
||||||
|
// and we can skip the parse / select / update loop entirely.
|
||||||
|
// Hermes-style trick: stat-only check, no content read.
|
||||||
|
Map<String, long[]> currentManifest = buildResourceManifest(resources);
|
||||||
|
SeedSnapshot snapshot = loadSnapshot();
|
||||||
|
if (snapshot != null
|
||||||
|
&& snapshot.version == SNAPSHOT_VERSION
|
||||||
|
&& manifestEquals(snapshot.manifest, currentManifest)
|
||||||
|
&& countBuiltinRows() == snapshot.rowCount) {
|
||||||
|
log.info("[SkillSeed] Manifest unchanged ({} skills); skipping per-row resolve",
|
||||||
|
currentManifest.size());
|
||||||
|
return new SyncStats(0, 0, currentManifest.size(), 0);
|
||||||
|
}
|
||||||
|
|
||||||
int inserted = 0, updated = 0, unchanged = 0, skipped = 0;
|
int inserted = 0, updated = 0, unchanged = 0, skipped = 0;
|
||||||
for (Resource resource : resources) {
|
for (Resource resource : resources) {
|
||||||
try {
|
try {
|
||||||
@ -117,9 +150,152 @@ public class BuiltinSkillSeedService implements ApplicationRunner {
|
|||||||
}
|
}
|
||||||
log.info("[SkillSeed] Builtin skills: {} inserted, {} updated, {} unchanged, {} skipped",
|
log.info("[SkillSeed] Builtin skills: {} inserted, {} updated, {} unchanged, {} skipped",
|
||||||
inserted, updated, unchanged, skipped);
|
inserted, updated, unchanged, skipped);
|
||||||
|
// Persist the manifest so the next startup can take the fast path
|
||||||
|
// when nothing changed. Failure to write is non-fatal — worst case
|
||||||
|
// the next startup re-runs the full loop.
|
||||||
|
writeSnapshot(new SeedSnapshot(SNAPSHOT_VERSION, currentManifest, countBuiltinRows()));
|
||||||
return new SyncStats(inserted, updated, unchanged, skipped);
|
return new SyncStats(inserted, updated, unchanged, skipped);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ==================== Snapshot helpers ====================
|
||||||
|
|
||||||
|
/** Snapshot persisted at {workspace_root}/.builtin-seed-snapshot.json. */
|
||||||
|
private record SeedSnapshot(int version, Map<String, long[]> manifest, long rowCount) {}
|
||||||
|
|
||||||
|
private Path snapshotPath() {
|
||||||
|
return Paths.get(workspaceRoot).resolve(".builtin-seed-snapshot.json");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Build a stable manifest of {@code uri → [size, mtime]} for the
|
||||||
|
* shipped SKILL.md set. {@link TreeMap} keeps the iteration order
|
||||||
|
* deterministic so byte-equality of two manifests means the same
|
||||||
|
* thing across runs.
|
||||||
|
*
|
||||||
|
* <p>{@link Resource#contentLength()} works in both exploded-classpath
|
||||||
|
* dev mode and inside a JAR. {@link URLConnection#getLastModified()}
|
||||||
|
* returns the JAR's mtime when the resource lives inside one — that
|
||||||
|
* still gives us a useful invalidation signal: rebuilding the JAR
|
||||||
|
* shifts every entry's mtime in lockstep, busting the snapshot.
|
||||||
|
*/
|
||||||
|
private Map<String, long[]> buildResourceManifest(Resource[] resources) {
|
||||||
|
Map<String, long[]> out = new TreeMap<>();
|
||||||
|
for (Resource res : resources) {
|
||||||
|
try {
|
||||||
|
String key = res.getURI().toString();
|
||||||
|
long size = res.contentLength();
|
||||||
|
long mtime;
|
||||||
|
try {
|
||||||
|
URLConnection conn = res.getURL().openConnection();
|
||||||
|
mtime = conn.getLastModified();
|
||||||
|
} catch (IOException ignored) {
|
||||||
|
mtime = 0L; // unknown — still hash-stable across runs
|
||||||
|
}
|
||||||
|
out.put(key, new long[]{size, mtime});
|
||||||
|
} catch (IOException e) {
|
||||||
|
// Skip resources we can't stat — they'll be picked up by the
|
||||||
|
// slow path which reads them anyway.
|
||||||
|
log.debug("Failed to stat resource {}: {}", res.getDescription(), e.getMessage());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Element-wise compare of two manifests (TreeMap ordering not assumed). */
|
||||||
|
private static boolean manifestEquals(Map<String, long[]> a, Map<String, long[]> b) {
|
||||||
|
if (a == null || b == null) return a == b;
|
||||||
|
if (a.size() != b.size()) return false;
|
||||||
|
for (Map.Entry<String, long[]> e : a.entrySet()) {
|
||||||
|
long[] other = b.get(e.getKey());
|
||||||
|
if (other == null) return false;
|
||||||
|
long[] mine = e.getValue();
|
||||||
|
if (mine.length != other.length) return false;
|
||||||
|
for (int i = 0; i < mine.length; i++) {
|
||||||
|
if (mine[i] != other[i]) return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
private long countBuiltinRows() {
|
||||||
|
try {
|
||||||
|
Long n = skillMapper.selectCount(
|
||||||
|
new LambdaQueryWrapper<SkillEntity>()
|
||||||
|
.eq(SkillEntity::getSkillType, SKILL_TYPE_BUILTIN)
|
||||||
|
.eq(SkillEntity::getDeleted, 0));
|
||||||
|
return n != null ? n : 0L;
|
||||||
|
} catch (Exception e) {
|
||||||
|
log.debug("Failed to count builtin skills: {}", e.getMessage());
|
||||||
|
return -1L; // mismatch sentinel — forces full re-scan
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private SeedSnapshot loadSnapshot() {
|
||||||
|
Path path = snapshotPath();
|
||||||
|
if (!Files.isRegularFile(path)) return null;
|
||||||
|
try {
|
||||||
|
String json = Files.readString(path, StandardCharsets.UTF_8);
|
||||||
|
Map<String, Object> root = objectMapper.readValue(
|
||||||
|
json, new TypeReference<>() {});
|
||||||
|
int version = toLong(root.get("version"), 0L).intValue();
|
||||||
|
long rowCount = toLong(root.get("rowCount"), 0L);
|
||||||
|
// Manifest entries arrive as List<Object> — each element may be a
|
||||||
|
// JSON number (Integer/Long) OR a quoted string. The mateclaw
|
||||||
|
// global ObjectMapper serializes long values as strings to avoid
|
||||||
|
// JS precision loss (mtime is 13-digit ms-since-epoch). Coerce
|
||||||
|
// both shapes back to long here.
|
||||||
|
Map<String, List<Object>> rawManifest = objectMapper.convertValue(
|
||||||
|
root.getOrDefault("manifest", Map.of()),
|
||||||
|
new TypeReference<>() {});
|
||||||
|
Map<String, long[]> manifest = new TreeMap<>();
|
||||||
|
for (Map.Entry<String, List<Object>> e : rawManifest.entrySet()) {
|
||||||
|
List<Object> arr = e.getValue();
|
||||||
|
if (arr == null || arr.size() < 2) continue;
|
||||||
|
manifest.put(e.getKey(), new long[]{
|
||||||
|
toLong(arr.get(0), 0L),
|
||||||
|
toLong(arr.get(1), 0L),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return new SeedSnapshot(version, manifest, rowCount);
|
||||||
|
} catch (Exception e) {
|
||||||
|
log.debug("Could not read seed snapshot {}: {}", path, e.getMessage());
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Coerce JSON-decoded value to long. Handles both numeric (Integer /
|
||||||
|
* Long / Double) and string-encoded-long forms — the latter is what
|
||||||
|
* mateclaw's ObjectMapper emits for long fields by default. */
|
||||||
|
private static Long toLong(Object value, Long fallback) {
|
||||||
|
if (value == null) return fallback;
|
||||||
|
if (value instanceof Number n) return n.longValue();
|
||||||
|
try {
|
||||||
|
return Long.parseLong(value.toString().trim());
|
||||||
|
} catch (NumberFormatException e) {
|
||||||
|
return fallback;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private void writeSnapshot(SeedSnapshot snapshot) {
|
||||||
|
Path path = snapshotPath();
|
||||||
|
try {
|
||||||
|
Files.createDirectories(path.getParent());
|
||||||
|
// Serialize the long[] entries as plain List<Long> for portable JSON.
|
||||||
|
Map<String, List<Long>> serializableManifest = new LinkedHashMap<>();
|
||||||
|
for (Map.Entry<String, long[]> e : snapshot.manifest().entrySet()) {
|
||||||
|
long[] v = e.getValue();
|
||||||
|
serializableManifest.put(e.getKey(), List.of(v[0], v[1]));
|
||||||
|
}
|
||||||
|
Map<String, Object> root = new LinkedHashMap<>();
|
||||||
|
root.put("version", snapshot.version());
|
||||||
|
root.put("rowCount", snapshot.rowCount());
|
||||||
|
root.put("manifest", serializableManifest);
|
||||||
|
Files.writeString(path, objectMapper.writeValueAsString(root), StandardCharsets.UTF_8);
|
||||||
|
} catch (Exception e) {
|
||||||
|
log.debug("Could not write seed snapshot {}: {}", path, e.getMessage());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private String readContent(Resource resource) throws Exception {
|
private String readContent(Resource resource) throws Exception {
|
||||||
try (InputStream is = resource.getInputStream()) {
|
try (InputStream is = resource.getInputStream()) {
|
||||||
return new String(is.readAllBytes(), StandardCharsets.UTF_8);
|
return new String(is.readAllBytes(), StandardCharsets.UTF_8);
|
||||||
|
|||||||
@ -17,17 +17,22 @@ import vip.mate.skill.runtime.model.ResolvedSkill;
|
|||||||
import vip.mate.skill.workspace.SkillWorkspaceManager;
|
import vip.mate.skill.workspace.SkillWorkspaceManager;
|
||||||
import vip.mate.tool.ToolRegistry;
|
import vip.mate.tool.ToolRegistry;
|
||||||
|
|
||||||
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.nio.file.Files;
|
import java.nio.file.Files;
|
||||||
import java.nio.file.Path;
|
import java.nio.file.Path;
|
||||||
import java.nio.file.Paths;
|
import java.nio.file.Paths;
|
||||||
|
import java.security.MessageDigest;
|
||||||
import java.time.LocalDateTime;
|
import java.time.LocalDateTime;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
|
import java.util.HexFormat;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
import java.util.LinkedHashSet;
|
import java.util.LinkedHashSet;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Objects;
|
import java.util.Objects;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
|
import java.util.concurrent.ConcurrentHashMap;
|
||||||
|
import java.util.concurrent.ConcurrentMap;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -82,6 +87,17 @@ public class SkillPackageResolver {
|
|||||||
private final java.util.concurrent.ConcurrentHashMap<Long, java.util.Set<String>> registeredWrappers
|
private final java.util.concurrent.ConcurrentHashMap<Long, java.util.Set<String>> registeredWrappers
|
||||||
= new java.util.concurrent.ConcurrentHashMap<>();
|
= new java.util.concurrent.ConcurrentHashMap<>();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Content-hash → cached scan outcome, so a refresh on unchanged
|
||||||
|
* SKILL.md content reuses the previous result instead of running
|
||||||
|
* the security scanner again. Builtin skills already short-circuit
|
||||||
|
* up-front; this cache helps user-installed dynamic skills, where
|
||||||
|
* the same row gets re-resolved on every refresh.
|
||||||
|
*/
|
||||||
|
private final ConcurrentMap<Long, CachedScanOutcome> securityScanCache = new ConcurrentHashMap<>();
|
||||||
|
|
||||||
|
private record CachedScanOutcome(String contentHash, SkillValidationResult result) {}
|
||||||
|
|
||||||
@Autowired
|
@Autowired
|
||||||
public SkillPackageResolver(SkillFrontmatterParser frontmatterParser,
|
public SkillPackageResolver(SkillFrontmatterParser frontmatterParser,
|
||||||
SkillManifestParser manifestParser,
|
SkillManifestParser manifestParser,
|
||||||
@ -365,8 +381,36 @@ public class SkillPackageResolver {
|
|||||||
// ==================== 阶段 2:安全扫描 ====================
|
// ==================== 阶段 2:安全扫描 ====================
|
||||||
|
|
||||||
private void applySecurity(ResolvedSkill resolved) {
|
private void applySecurity(ResolvedSkill resolved) {
|
||||||
|
// Builtin skills come from classpath/jar — they're version-controlled
|
||||||
|
// upstream and the resolver flow already maps blocked findings to
|
||||||
|
// `trustedBuiltin` (warn but don't block). Running the full scanner
|
||||||
|
// on every refresh is pure overhead; with 30+ shipped skills this
|
||||||
|
// dominates the refresh budget. Mark them trusted up-front and
|
||||||
|
// skip — the persisted scan_result on mate_skill (written by the
|
||||||
|
// initial resolve) keeps the UI's audit trail intact.
|
||||||
|
if (resolved.isBuiltin()) {
|
||||||
|
resolved.setSecurityBlocked(false);
|
||||||
|
resolved.setSecuritySummary("Trusted builtin (runtime scan skipped)");
|
||||||
|
resolved.setSecurityWarnings(List.of());
|
||||||
|
return;
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
SkillValidationResult result = securityService.validate(resolved);
|
// Content-hash cache: refreshes on unchanged SKILL.md content
|
||||||
|
// skip the (potentially expensive) scanner. Hash drifts → fall
|
||||||
|
// through to a fresh scan and replace the cached entry.
|
||||||
|
String contentHash = sha256(resolved.getContent());
|
||||||
|
CachedScanOutcome cached = resolved.getId() != null
|
||||||
|
? securityScanCache.get(resolved.getId())
|
||||||
|
: null;
|
||||||
|
SkillValidationResult result;
|
||||||
|
if (cached != null && cached.contentHash().equals(contentHash)) {
|
||||||
|
result = cached.result();
|
||||||
|
} else {
|
||||||
|
result = securityService.validate(resolved);
|
||||||
|
if (resolved.getId() != null) {
|
||||||
|
securityScanCache.put(resolved.getId(), new CachedScanOutcome(contentHash, result));
|
||||||
|
}
|
||||||
|
}
|
||||||
boolean trustedBuiltin = resolved.isBuiltin() && result.isBlocked();
|
boolean trustedBuiltin = resolved.isBuiltin() && result.isBlocked();
|
||||||
resolved.setSecurityBlocked(result.isBlocked() && !trustedBuiltin);
|
resolved.setSecurityBlocked(result.isBlocked() && !trustedBuiltin);
|
||||||
resolved.setSecuritySeverity(result.getMaxSeverity() != null ? result.getMaxSeverity().name() : null);
|
resolved.setSecuritySeverity(result.getMaxSeverity() != null ? result.getMaxSeverity().name() : null);
|
||||||
@ -658,6 +702,10 @@ public class SkillPackageResolver {
|
|||||||
*/
|
*/
|
||||||
public void deregisterSkillWrappers(Long skillId) {
|
public void deregisterSkillWrappers(Long skillId) {
|
||||||
if (skillId == null) return;
|
if (skillId == null) return;
|
||||||
|
// Evict the security-scan cache entry too — a re-installed skill
|
||||||
|
// with the same id but different content needs a fresh scan, and
|
||||||
|
// a permanently-deleted skill should free the slot.
|
||||||
|
securityScanCache.remove(skillId);
|
||||||
java.util.Set<String> previous = registeredWrappers.remove(skillId);
|
java.util.Set<String> previous = registeredWrappers.remove(skillId);
|
||||||
if (previous == null || previous.isEmpty()) return;
|
if (previous == null || previous.isEmpty()) return;
|
||||||
for (String name : previous) {
|
for (String name : previous) {
|
||||||
@ -670,6 +718,19 @@ public class SkillPackageResolver {
|
|||||||
log.info("Deregistered {} wrapper tool(s) for skill id={}", previous.size(), skillId);
|
log.info("Deregistered {} wrapper tool(s) for skill id={}", previous.size(), skillId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Hex-encoded SHA-256 of the input string (UTF-8). Empty/null → "". */
|
||||||
|
private static String sha256(String input) {
|
||||||
|
if (input == null || input.isEmpty()) return "";
|
||||||
|
try {
|
||||||
|
MessageDigest md = MessageDigest.getInstance("SHA-256");
|
||||||
|
return HexFormat.of().formatHex(md.digest(input.getBytes(StandardCharsets.UTF_8)));
|
||||||
|
} catch (Exception e) {
|
||||||
|
// SHA-256 is mandatory in the JRE — should never throw.
|
||||||
|
// Fall back to identity so the cache is just always-miss.
|
||||||
|
return input;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* RFC-090 Phase 7b — register / refresh / deregister wrapper tools
|
* RFC-090 Phase 7b — register / refresh / deregister wrapper tools
|
||||||
* for a single ACP skill. Mirrors {@link #applyKnowledgeWrappers}.
|
* for a single ACP skill. Mirrors {@link #applyKnowledgeWrappers}.
|
||||||
|
|||||||
@ -17,11 +17,17 @@ import vip.mate.skill.service.SkillService;
|
|||||||
import vip.mate.skill.workspace.SkillWorkspaceEvent;
|
import vip.mate.skill.workspace.SkillWorkspaceEvent;
|
||||||
|
|
||||||
import jakarta.annotation.PostConstruct;
|
import jakarta.annotation.PostConstruct;
|
||||||
|
import jakarta.annotation.PreDestroy;
|
||||||
import org.springframework.boot.context.event.ApplicationReadyEvent;
|
import org.springframework.boot.context.event.ApplicationReadyEvent;
|
||||||
import org.springframework.context.event.EventListener;
|
import org.springframework.context.event.EventListener;
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
|
import java.util.concurrent.Executors;
|
||||||
|
import java.util.concurrent.ScheduledExecutorService;
|
||||||
|
import java.util.concurrent.ScheduledFuture;
|
||||||
|
import java.util.concurrent.TimeUnit;
|
||||||
|
import java.util.concurrent.atomic.AtomicReference;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -75,6 +81,26 @@ public class SkillRuntimeService {
|
|||||||
|
|
||||||
private static final String CACHE_KEY = "active_skills";
|
private static final String CACHE_KEY = "active_skills";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Debounce window for {@link #onWorkspaceEvent}. Startup typically
|
||||||
|
* fires one event per bundled skill (30+ in a row), and there's
|
||||||
|
* nothing useful to do until the whole batch settles. 500 ms is
|
||||||
|
* long enough to swallow the burst without making admin re-syncs
|
||||||
|
* feel laggy.
|
||||||
|
*/
|
||||||
|
private static final long REFRESH_DEBOUNCE_MS = 500;
|
||||||
|
|
||||||
|
private final ScheduledExecutorService refreshScheduler =
|
||||||
|
Executors.newSingleThreadScheduledExecutor(r -> {
|
||||||
|
Thread t = new Thread(r, "skill-refresh-debouncer");
|
||||||
|
t.setDaemon(true);
|
||||||
|
return t;
|
||||||
|
});
|
||||||
|
|
||||||
|
/** Most recent pending refresh future — atomically swapped on each
|
||||||
|
* event so the previous one can be cancelled. */
|
||||||
|
private final AtomicReference<ScheduledFuture<?>> pendingRefresh = new AtomicReference<>();
|
||||||
|
|
||||||
@PostConstruct
|
@PostConstruct
|
||||||
public void init() {
|
public void init() {
|
||||||
log.info("SkillRuntimeService initialized");
|
log.info("SkillRuntimeService initialized");
|
||||||
@ -90,8 +116,42 @@ public class SkillRuntimeService {
|
|||||||
|
|
||||||
@EventListener(SkillWorkspaceEvent.class)
|
@EventListener(SkillWorkspaceEvent.class)
|
||||||
public void onWorkspaceEvent(SkillWorkspaceEvent event) {
|
public void onWorkspaceEvent(SkillWorkspaceEvent event) {
|
||||||
log.info("Workspace event: {} {} at {}", event.type(), event.skillName(), event.workspacePath());
|
// Coalesce bursts of workspace events (every bundled-skill sync at
|
||||||
refreshActiveSkills();
|
// startup fires one) into a single refresh. Without debounce, a
|
||||||
|
// 30-skill startup triggered 30 sequential refreshActiveSkills()
|
||||||
|
// calls — each running the whole resolve+scan loop. With 500 ms
|
||||||
|
// debounce it collapses to one.
|
||||||
|
log.debug("Workspace event: {} {} (refresh scheduled in {}ms)",
|
||||||
|
event.type(), event.skillName(), REFRESH_DEBOUNCE_MS);
|
||||||
|
ScheduledFuture<?> previous = pendingRefresh.get();
|
||||||
|
if (previous != null && !previous.isDone()) {
|
||||||
|
previous.cancel(false);
|
||||||
|
}
|
||||||
|
ScheduledFuture<?> task = refreshScheduler.schedule(() -> {
|
||||||
|
try {
|
||||||
|
refreshActiveSkills();
|
||||||
|
} catch (Exception e) {
|
||||||
|
log.warn("Debounced refresh failed: {}", e.getMessage());
|
||||||
|
}
|
||||||
|
}, REFRESH_DEBOUNCE_MS, TimeUnit.MILLISECONDS);
|
||||||
|
pendingRefresh.set(task);
|
||||||
|
}
|
||||||
|
|
||||||
|
@PreDestroy
|
||||||
|
public void shutdown() {
|
||||||
|
// Drain any pending refresh so JVM shutdown doesn't hang on the
|
||||||
|
// daemon thread, even though it's marked daemon and would die anyway.
|
||||||
|
ScheduledFuture<?> pending = pendingRefresh.getAndSet(null);
|
||||||
|
if (pending != null) pending.cancel(true);
|
||||||
|
refreshScheduler.shutdown();
|
||||||
|
try {
|
||||||
|
if (!refreshScheduler.awaitTermination(2, TimeUnit.SECONDS)) {
|
||||||
|
refreshScheduler.shutdownNow();
|
||||||
|
}
|
||||||
|
} catch (InterruptedException e) {
|
||||||
|
refreshScheduler.shutdownNow();
|
||||||
|
Thread.currentThread().interrupt();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -261,63 +321,44 @@ public class SkillRuntimeService {
|
|||||||
} else {
|
} else {
|
||||||
activeSkills = getActiveSkills();
|
activeSkills = getActiveSkills();
|
||||||
}
|
}
|
||||||
|
// Platform filter — drop skills whose `platforms:` frontmatter
|
||||||
|
// names a different OS than the runtime host. apple-notes /
|
||||||
|
// findmy etc. are macOS-only; surfacing them on Linux just
|
||||||
|
// burns prompt tokens for skills the user can never run.
|
||||||
|
// Empty / missing `platforms:` means "all platforms" (the default).
|
||||||
|
String currentOs = currentOsCanonical();
|
||||||
|
activeSkills = activeSkills.stream()
|
||||||
|
.filter(s -> matchesCurrentPlatform(s, currentOs))
|
||||||
|
.collect(java.util.stream.Collectors.toList());
|
||||||
if (activeSkills.isEmpty()) {
|
if (activeSkills.isEmpty()) {
|
||||||
return "";
|
return "";
|
||||||
}
|
}
|
||||||
|
|
||||||
// Issue #46 + #49 prompt rewrite. Stop-gap until RFC-090 lands the
|
// Compact preamble — was ~1 KB of warnings before. The two failure
|
||||||
// proper `type` enum and inlines `type: prompt` skill bodies into the
|
// modes that drove the older wording (#46: LLM treats skill name
|
||||||
// system prompt directly. Today every skill is announced via this
|
// as a tool; #49: LLM invokes runSkillScript on a docs-only skill)
|
||||||
// catalog and the LLM has to pull SKILL.md on demand. Two failure
|
// are now addressed in two cheaper spots:
|
||||||
// modes have been observed:
|
// - readSkillFile/runSkillScript tools have explicit "Tool not
|
||||||
// #46 — LLM treated the skill name as a tool ("tool_use{name=
|
// found" errors that nudge the model to retry the right way.
|
||||||
// RedisOps}"), so we lead with an explicit "NOT callable"
|
// - SKILL.md itself, once loaded via readSkillFile, tells the
|
||||||
// warning and surface the actual tool names readSkillFile /
|
// model whether to invoke a script or just follow the prose.
|
||||||
// runSkillScript.
|
// 49 skills × ~20 chars/row saved by dropping the Shape column +
|
||||||
// #49 — On a docs-only skill (SKILL.md, no scripts/ dir), the LLM
|
// ~600 chars saved by trimming the preamble = ~1.5 KB / ~400
|
||||||
// tried to invoke a non-existent scripts/ file and never
|
// tokens lighter on every chat request.
|
||||||
// followed SKILL.md's text guidance. The previous wording
|
|
||||||
// "usually that means calling runSkillScript(...)" actively
|
|
||||||
// pushed it that way. Fix: tell the model the two shapes
|
|
||||||
// exist, mark each row's shape, and forbid invoking scripts
|
|
||||||
// on docs-only skills.
|
|
||||||
StringBuilder sb = new StringBuilder();
|
StringBuilder sb = new StringBuilder();
|
||||||
sb.append("\n\n## Available Skills\n\n");
|
sb.append("\n\n## Skills\n");
|
||||||
sb.append("⚠️ **Skills are documentation packages, NOT directly callable tools.**\n");
|
sb.append("Before answering, scan the skills below. If a skill matches your task, ");
|
||||||
sb.append("Calling a skill name as a tool (e.g. tool_use{name=\"RedisOps\"}) will fail with \"Tool not found\". ");
|
sb.append("load it via `readSkillFile(skillName=<name>, filePath=\"SKILL.md\")` and follow its instructions. ");
|
||||||
sb.append("To use a skill:\n\n");
|
sb.append("Skills are documentation packages — calling a skill name as a tool will fail. ");
|
||||||
sb.append("1. ALWAYS first read its SKILL.md to learn how it works:\n");
|
sb.append("Skills with a `scripts/` directory expose `runSkillScript`; SKILL.md will name the script when needed.\n\n");
|
||||||
sb.append(" `readSkillFile(skillName=\"<name>\", filePath=\"SKILL.md\")`\n");
|
sb.append("| Skill | Description |\n");
|
||||||
sb.append("2. Then follow what SKILL.md says. Skills come in two shapes — check the **Shape** column below:\n");
|
sb.append("|-------|-------------|\n");
|
||||||
sb.append(" - **docs only** — no `scripts/` directory exists. SKILL.md is the entire instruction set; follow its text guidance directly. Do NOT call `runSkillScript` on these — the script does not exist and the call will fail.\n");
|
|
||||||
sb.append(" - **scripts + docs** — `scripts/` is present. SKILL.md will name the script to run; invoke it with `runSkillScript(skillName=\"<name>\", scriptPath=\"scripts/<file>\")`.\n");
|
|
||||||
sb.append(" Either shape may also expose supplementary docs via `readSkillFile(skillName=\"<name>\", filePath=\"references/<file>\")`.\n\n");
|
|
||||||
|
|
||||||
// Concrete example anchored to the first enabled skill so the LLM
|
|
||||||
// sees a real name it just read in the listing below.
|
|
||||||
String exampleName = activeSkills.get(0).getName();
|
|
||||||
sb.append("Concrete example — to use the `").append(exampleName).append("` skill, START with:\n");
|
|
||||||
sb.append(" `readSkillFile(skillName=\"").append(exampleName).append("\", filePath=\"SKILL.md\")`\n\n");
|
|
||||||
|
|
||||||
sb.append("### Enabled skills\n");
|
|
||||||
sb.append("Pass these names as the `skillName=` argument to `readSkillFile` / `runSkillScript`. ");
|
|
||||||
sb.append("Do **not** call them as tools.\n\n");
|
|
||||||
sb.append("| Skill name | Shape | Description |\n");
|
|
||||||
sb.append("|------------|-------|-------------|\n");
|
|
||||||
for (ResolvedSkill skill : activeSkills) {
|
for (ResolvedSkill skill : activeSkills) {
|
||||||
// `scripts` is populated by SkillDirectoryScanner — empty map both
|
|
||||||
// for "scripts/ directory absent" and "directory present but empty".
|
|
||||||
// Either way, runSkillScript has nothing to call, so we report the
|
|
||||||
// skill as docs-only. (Database-fallback skills also land here
|
|
||||||
// because SkillPackageResolver.resolveFromDatabase sets it to
|
|
||||||
// Map.of().) RFC-090's `type` field will replace this heuristic.
|
|
||||||
boolean hasScripts = skill.getScripts() != null && !skill.getScripts().isEmpty();
|
|
||||||
String shape = hasScripts ? "scripts + docs" : "docs only";
|
|
||||||
sb.append("| `").append(skill.getName()).append("`");
|
sb.append("| `").append(skill.getName()).append("`");
|
||||||
if (skill.getIcon() != null && !skill.getIcon().isBlank()) {
|
if (skill.getIcon() != null && !skill.getIcon().isBlank()) {
|
||||||
sb.append(" ").append(skill.getIcon());
|
sb.append(" ").append(skill.getIcon());
|
||||||
}
|
}
|
||||||
sb.append(" | ").append(shape).append(" | ");
|
sb.append(" | ");
|
||||||
if (skill.getDescription() != null && !skill.getDescription().isBlank()) {
|
if (skill.getDescription() != null && !skill.getDescription().isBlank()) {
|
||||||
String desc = skill.getDescription();
|
String desc = skill.getDescription();
|
||||||
if (desc.length() > 200) {
|
if (desc.length() > 200) {
|
||||||
@ -367,4 +408,36 @@ public class SkillRuntimeService {
|
|||||||
sb.append(lessons);
|
sb.append(lessons);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Map {@code System.getProperty("os.name")} to one of the canonical
|
||||||
|
* tokens used in SKILL.md {@code platforms:} ({@code macos / linux /
|
||||||
|
* windows}). Anything unrecognised → {@code "other"} which never
|
||||||
|
* matches a declared platform list, so the skill stays visible only
|
||||||
|
* if its platforms list is empty (the "all platforms" default).
|
||||||
|
*/
|
||||||
|
static String currentOsCanonical() {
|
||||||
|
String os = System.getProperty("os.name", "").toLowerCase(java.util.Locale.ROOT);
|
||||||
|
if (os.contains("mac") || os.contains("darwin")) return "macos";
|
||||||
|
if (os.contains("nux") || os.contains("nix")) return "linux";
|
||||||
|
if (os.contains("win")) return "windows";
|
||||||
|
return "other";
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* True when the skill is compatible with {@code currentOs}. A skill
|
||||||
|
* with empty / null {@code platforms:} matches every OS (legacy
|
||||||
|
* default). Otherwise the canonical OS token must appear in the list.
|
||||||
|
*/
|
||||||
|
static boolean matchesCurrentPlatform(ResolvedSkill skill, String currentOs) {
|
||||||
|
SkillManifest manifest = skill.getManifest();
|
||||||
|
if (manifest == null) return true;
|
||||||
|
List<String> platforms = manifest.getPlatforms();
|
||||||
|
if (platforms == null || platforms.isEmpty()) return true;
|
||||||
|
for (String p : platforms) {
|
||||||
|
if (p == null) continue;
|
||||||
|
if (currentOs.equalsIgnoreCase(p.trim())) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user