From 407ffdef0bd18c073ebacbb166170c85b787b595 Mon Sep 17 00:00:00 2001 From: Shakker Date: Thu, 28 May 2026 19:46:00 +0100 Subject: [PATCH] fix: preserve skill snapshot freshness --- src/skills/service.test.ts | 72 ++++++++++++++++++++++++++++++++++++++ src/skills/service.ts | 36 +++++++++++++++---- 2 files changed, 102 insertions(+), 6 deletions(-) diff --git a/src/skills/service.test.ts b/src/skills/service.test.ts index 37fe7d89347..0050b661c09 100644 --- a/src/skills/service.test.ts +++ b/src/skills/service.test.ts @@ -96,6 +96,78 @@ describe("SkillsService", () => { expect(after.entries.map((entry) => entry.name)).toContain("service-zero-beta"); }); + it("does not cache explicit indexes when skill watching is disabled", async () => { + const workspaceDir = await makeTempWorkspace(); + await writeWorkspaceSkills(workspaceDir, [ + { name: "service-watch-alpha", description: "Alpha workflow" }, + ]); + const service = new SkillsService(); + const roots = isolatedSkillRoots(workspaceDir); + const config = { skills: { load: { watch: false } } } satisfies OpenClawConfig; + + const before = service.getIndex({ workspaceDir, ...roots, config, snapshotVersion: 1 }); + await writeWorkspaceSkills(workspaceDir, [ + { name: "service-watch-beta", description: "Beta workflow" }, + ]); + const after = service.getIndex({ workspaceDir, ...roots, config, snapshotVersion: 1 }); + + expect(after).not.toBe(before); + expect(before.entries.map((entry) => entry.name)).not.toContain("service-watch-beta"); + expect(after.entries.map((entry) => entry.name)).toContain("service-watch-beta"); + }); + + it("keeps snapshots uncached even when callers pass a positive version", async () => { + const workspaceDir = await makeTempWorkspace(); + await writeWorkspaceSkills(workspaceDir, [ + { name: "service-snapshot-alpha", description: "Alpha workflow" }, + ]); + const service = new SkillsService(); + const roots = isolatedSkillRoots(workspaceDir); + + const before = service.buildSnapshot(workspaceDir, { ...roots, snapshotVersion: 1 }); + await writeWorkspaceSkills(workspaceDir, [ + { name: "service-snapshot-beta", description: "Beta workflow" }, + ]); + const after = service.buildSnapshot(workspaceDir, { ...roots, snapshotVersion: 1 }); + + expect(before.skills.map((skill) => skill.name)).not.toContain("service-snapshot-beta"); + expect(after.skills.map((skill) => skill.name)).toContain("service-snapshot-beta"); + }); + + it("bounds cached indexes across workspace scopes", async () => { + const service = new SkillsService(); + const workspaces = await Promise.all( + Array.from({ length: 17 }, async (_, index) => { + const workspaceDir = await makeTempWorkspace(); + await writeWorkspaceSkills(workspaceDir, [ + { name: `service-lru-${index}`, description: "Cached workflow" }, + ]); + return workspaceDir; + }), + ); + const firstWorkspace = workspaces[0]!; + const first = service.getIndex({ + workspaceDir: firstWorkspace, + ...isolatedSkillRoots(firstWorkspace), + snapshotVersion: 1, + }); + + for (const workspaceDir of workspaces.slice(1)) { + service.getIndex({ + workspaceDir, + ...isolatedSkillRoots(workspaceDir), + snapshotVersion: 1, + }); + } + const firstAgain = service.getIndex({ + workspaceDir: firstWorkspace, + ...isolatedSkillRoots(firstWorkspace), + snapshotVersion: 1, + }); + + expect(firstAgain).not.toBe(first); + }); + it("includes plugin config in the versioned cache key", async () => { const workspaceDir = await makeTempWorkspace(); const roots = isolatedSkillRoots(workspaceDir); diff --git a/src/skills/service.ts b/src/skills/service.ts index a4bd701ddc1..17fbc9c0845 100644 --- a/src/skills/service.ts +++ b/src/skills/service.ts @@ -11,6 +11,8 @@ import { loadWorkspaceSkillEntries, } from "./workspace.js"; +const MAX_SKILL_INDEX_CACHE_ENTRIES = 16; + export type SkillIndexRequest = { workspaceDir: string; config?: OpenClawConfig; @@ -38,19 +40,22 @@ export class SkillsService { getIndex(request: SkillIndexRequest): SkillIndex { const snapshotVersion = request.snapshotVersion ?? getSkillsSnapshotVersion(request.workspaceDir); - if (!shouldCacheSkillIndex(snapshotVersion)) { + if (!shouldCacheSkillIndex(request, snapshotVersion)) { return this.loadIndex(request, buildUncachedSkillIndexCacheKey(request, snapshotVersion)); } const cacheKeyParts = buildSkillIndexCacheKeyParts(request, snapshotVersion); const cacheKey = stringifyCacheKeyParts(cacheKeyParts); const cached = this.cache.get(cacheKey); if (cached) { + this.cache.delete(cacheKey); + this.cache.set(cacheKey, cached); return cached; } const index = this.loadIndex(request, cacheKey); this.pruneScope(cacheKeyParts.scope, cacheKey); this.cache.set(cacheKey, index); this.cacheScopes.set(cacheKey, cacheKeyParts.scope); + this.pruneCapacity(); return index; } @@ -74,9 +79,10 @@ export class SkillsService { snapshotVersion: opts?.snapshotVersion, }; const snapshotVersion = request.snapshotVersion ?? getSkillsSnapshotVersion(workspaceDir); - const index = shouldCacheSkillIndex(snapshotVersion) - ? this.getIndex(request) - : this.loadIndex(request, buildUncachedSkillIndexCacheKey(request, snapshotVersion)); + const index = this.loadIndex( + request, + buildUncachedSkillIndexCacheKey(request, snapshotVersion), + ); return buildSkillSnapshotFromIndex(workspaceDir, index, opts); } @@ -94,6 +100,17 @@ export class SkillsService { this.cacheScopes.delete(key); } } + + private pruneCapacity(): void { + while (this.cache.size > MAX_SKILL_INDEX_CACHE_ENTRIES) { + const oldestKey = this.cache.keys().next().value; + if (oldestKey === undefined) { + break; + } + this.cache.delete(oldestKey); + this.cacheScopes.delete(oldestKey); + } + } } export const skillsService = new SkillsService(); @@ -138,8 +155,15 @@ type SkillIndexCacheKeyParts = { snapshotVersion: number; }; -function shouldCacheSkillIndex(snapshotVersion: number | undefined): boolean { - return typeof snapshotVersion === "number" && snapshotVersion > 0; +function shouldCacheSkillIndex( + request: SkillIndexRequest, + snapshotVersion: number | undefined, +): boolean { + return ( + typeof snapshotVersion === "number" && + snapshotVersion > 0 && + request.config?.skills?.load?.watch !== false + ); } function buildUncachedSkillIndexCacheKey(