From 239def78384b4d09e953bb787d104adaa9606423 Mon Sep 17 00:00:00 2001 From: solodmd <51304754+solodmd@users.noreply.github.com> Date: Fri, 15 May 2026 18:48:22 +0800 Subject: [PATCH] perf(skills): cache hydrated resolved skills (#81451) Merged via squash. Prepared head SHA: e202d16e50c6585c1401ad3cdd2d49db1b92762e Co-authored-by: solodmd <51304754+solodmd@users.noreply.github.com> Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com> Reviewed-by: @altaywtf --- CHANGELOG.md | 2 + src/auto-reply/reply/session-updates.test.ts | 169 ++++++++++++++++++- src/auto-reply/reply/session-updates.ts | 123 +++++++++++++- 3 files changed, 282 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c06953ba3a..4b31304497c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Docs: https://docs.openclaw.ai ### Changes +- Agents/skills: cache hydrated `resolvedSkills` across warm gateway turns while keying reuse by the redacted effective config, reducing redundant skill snapshot rebuilds without crossing config-gated skill boundaries. (#81451) Thanks @solodmd. + ### Fixes - Memory search: stop using chokidar write-stability polling for memory and QMD watchers so large Markdown extraPath trees no longer build up regular file descriptors; changed files now settle through the existing debounced sync queue. Fixes #77327 and #78224. (#81802) Thanks @frankekn, @loyur, and @JanPlessow. diff --git a/src/auto-reply/reply/session-updates.test.ts b/src/auto-reply/reply/session-updates.test.ts index 87b6f2e7c0e..a99ecb7de0c 100644 --- a/src/auto-reply/reply/session-updates.test.ts +++ b/src/auto-reply/reply/session-updates.test.ts @@ -1,4 +1,24 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { SessionEntry } from "../../config/sessions.js"; + +const TEST_WORKSPACE_DIR = "/tmp/workspace"; +type TestSkillSnapshot = NonNullable; + +function strippedSnapshot(skillName = "test"): TestSkillSnapshot { + return { + prompt: "skills prompt", + skills: [{ name: skillName }], + version: 0, + }; +} + +function testSessionEntry(sessionId: string, skillsSnapshot: TestSkillSnapshot): SessionEntry { + return { + sessionId, + updatedAt: Date.now(), + skillsSnapshot, + }; +} const { buildWorkspaceSkillSnapshotMock, @@ -10,7 +30,11 @@ const { resolveSessionAgentIdMock, resolveAgentIdFromSessionKeyMock, } = vi.hoisted(() => ({ - buildWorkspaceSkillSnapshotMock: vi.fn(() => ({ prompt: "", skills: [], resolvedSkills: [] })), + buildWorkspaceSkillSnapshotMock: vi.fn((..._args: unknown[]) => ({ + prompt: "", + skills: [] as unknown[], + resolvedSkills: [] as unknown[], + })), ensureSkillsWatcherMock: vi.fn(), getSkillsSnapshotVersionMock: vi.fn(() => 0), shouldRefreshSnapshotForVersionMock: vi.fn(() => false), @@ -55,11 +79,13 @@ vi.mock("../../routing/session-key.js", () => ({ resolveAgentIdFromSessionKey: resolveAgentIdFromSessionKeyMock, })); -const { ensureSkillSnapshot } = await import("./session-updates.js"); +const { ensureSkillSnapshot, __testing_resetResolvedSkillsCache } = + await import("./session-updates.js"); describe("ensureSkillSnapshot", () => { beforeEach(() => { vi.clearAllMocks(); + __testing_resetResolvedSkillsCache(); buildWorkspaceSkillSnapshotMock.mockReturnValue({ prompt: "", skills: [], resolvedSkills: [] }); getSkillsSnapshotVersionMock.mockReturnValue(0); shouldRefreshSnapshotForVersionMock.mockReturnValue(false); @@ -83,7 +109,7 @@ describe("ensureSkillSnapshot", () => { await ensureSkillSnapshot({ sessionKey: "main", isFirstTurnInSession: false, - workspaceDir: "/tmp/workspace", + workspaceDir: TEST_WORKSPACE_DIR, cfg: { agents: { list: [{ id: "writer", default: true }], @@ -102,8 +128,143 @@ describe("ensureSkillSnapshot", () => { expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1); const [[workspaceDir, snapshotParams]] = buildWorkspaceSkillSnapshotMock.mock .calls as unknown as Array<[string, { agentId?: string }]>; - expect(workspaceDir).toBe("/tmp/workspace"); + expect(workspaceDir).toBe(TEST_WORKSPACE_DIR); expect(snapshotParams.agentId).toBe("writer"); expect(resolveAgentIdFromSessionKeyMock).not.toHaveBeenCalled(); }); + + it("reuses cached resolvedSkills across calls with same workspaceDir/version/filter", async () => { + vi.stubEnv("OPENCLAW_TEST_FAST", "0"); + + const sessionStore: Record = {}; + const sessionKey = "main"; + const snapshot = strippedSnapshot(); + const sessionEntry = testSessionEntry("sess-1", snapshot); + + await ensureSkillSnapshot({ + sessionEntry, + sessionStore, + sessionKey, + isFirstTurnInSession: true, + workspaceDir: TEST_WORKSPACE_DIR, + cfg: {}, + }); + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1); + + const sessionEntry2 = testSessionEntry("sess-2", { ...snapshot }); + await ensureSkillSnapshot({ + sessionEntry: sessionEntry2, + sessionStore: {}, + sessionKey: "other", + isFirstTurnInSession: false, + workspaceDir: TEST_WORKSPACE_DIR, + cfg: {}, + }); + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1); + }); + + it("invalidates cache when skillFilter changes", async () => { + vi.stubEnv("OPENCLAW_TEST_FAST", "0"); + + const sessionStore: Record = {}; + const sessionKey = "main"; + const snapshot = strippedSnapshot(); + const sessionEntry = testSessionEntry("sess-1", snapshot); + + await ensureSkillSnapshot({ + sessionEntry, + sessionStore, + sessionKey, + isFirstTurnInSession: true, + workspaceDir: TEST_WORKSPACE_DIR, + cfg: {}, + }); + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1); + + const sessionEntry2 = testSessionEntry("sess-2", { + ...snapshot, + skillFilter: ["old-filter"], + }); + await ensureSkillSnapshot({ + sessionEntry: sessionEntry2, + sessionStore: {}, + sessionKey: "other", + isFirstTurnInSession: false, + workspaceDir: TEST_WORKSPACE_DIR, + skillFilter: ["new-filter"], + cfg: {}, + }); + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(2); + }); + + it("invalidates cache when non-skills config gates change", async () => { + vi.stubEnv("OPENCLAW_TEST_FAST", "0"); + + buildWorkspaceSkillSnapshotMock.mockImplementation((_workspaceDir, opts) => { + const config = (opts as { config?: { channels?: { discord?: { token?: string } } } }).config; + return { + prompt: "", + skills: [], + resolvedSkills: config?.channels?.discord?.token ? [{ name: "discord" }] : [], + }; + }); + + const snapshot = strippedSnapshot("discord"); + + const first = await ensureSkillSnapshot({ + sessionEntry: testSessionEntry("sess-1", snapshot), + sessionStore: {}, + sessionKey: "main", + isFirstTurnInSession: true, + workspaceDir: TEST_WORKSPACE_DIR, + cfg: { channels: { discord: { token: "enabled" } } }, + }); + + expect(first.skillsSnapshot?.resolvedSkills).toEqual([{ name: "discord" }]); + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1); + + const second = await ensureSkillSnapshot({ + sessionEntry: testSessionEntry("sess-2", { ...snapshot }), + sessionStore: {}, + sessionKey: "other", + isFirstTurnInSession: false, + workspaceDir: TEST_WORKSPACE_DIR, + cfg: { channels: { discord: {} } }, + }); + + expect(second.skillsSnapshot?.resolvedSkills).toEqual([]); + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(2); + }); + + it("redacts secret values in the cache key while preserving eligibility presence", async () => { + vi.stubEnv("OPENCLAW_TEST_FAST", "0"); + + buildWorkspaceSkillSnapshotMock.mockReturnValue({ + prompt: "", + skills: [], + resolvedSkills: [{ name: "discord" }], + }); + + const snapshot = strippedSnapshot("discord"); + + await ensureSkillSnapshot({ + sessionEntry: testSessionEntry("sess-1", snapshot), + sessionStore: {}, + sessionKey: "main", + isFirstTurnInSession: true, + workspaceDir: TEST_WORKSPACE_DIR, + cfg: { channels: { discord: { token: "first-secret" } } }, + }); + + await ensureSkillSnapshot({ + sessionEntry: testSessionEntry("sess-2", { ...snapshot }), + sessionStore: {}, + sessionKey: "other", + isFirstTurnInSession: false, + workspaceDir: TEST_WORKSPACE_DIR, + cfg: { channels: { discord: { token: "rotated-secret" } } }, + }); + + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/auto-reply/reply/session-updates.ts b/src/auto-reply/reply/session-updates.ts index 15160a17a56..c8115f7b716 100644 --- a/src/auto-reply/reply/session-updates.ts +++ b/src/auto-reply/reply/session-updates.ts @@ -3,7 +3,7 @@ import fs from "node:fs"; import path from "node:path"; import { resolveSessionAgentId } from "../../agents/agent-scope.js"; import { canExecRequestNode } from "../../agents/exec-defaults.js"; -import { buildWorkspaceSkillSnapshot } from "../../agents/skills.js"; +import { buildWorkspaceSkillSnapshot, type SkillSnapshot } from "../../agents/skills.js"; import { matchesSkillFilter } from "../../agents/skills/filter.js"; import { getSkillsSnapshotVersion, @@ -11,6 +11,7 @@ import { } from "../../agents/skills/refresh-state.js"; import { ensureSkillsWatcher } from "../../agents/skills/refresh.js"; import { hydrateResolvedSkills } from "../../agents/skills/snapshot-hydration.js"; +import { stableStringify } from "../../agents/stable-stringify.js"; import { resolveSessionFilePath, resolveSessionFilePathOptions, @@ -31,6 +32,92 @@ import { normalizeOptionalString } from "../../shared/string-coerce.js"; import { buildSessionEndHookPayload, buildSessionStartHookPayload } from "./session-hooks.js"; export { drainFormattedSystemEvents } from "./session-system-events.js"; +// Warm-start resolvedSkills cache: avoids redundant buildSnapshot calls when +// stripPersistedSkillsCache has removed resolvedSkills between turns. +// Bounded to 10 entries to prevent unbounded growth in long-lived gateways. +const resolvedSkillsCache = new Map(); +const RESOLVED_SKILLS_CACHE_MAX = 10; + +export function __testing_resetResolvedSkillsCache(): void { + resolvedSkillsCache.clear(); +} + +function isSensitiveConfigKey(key: string): boolean { + const normalized = key.toLowerCase().replaceAll(/[^a-z0-9]/g, ""); + return ( + normalized.endsWith("apikey") || + normalized.endsWith("token") || + normalized.endsWith("secret") || + normalized.endsWith("password") || + normalized.endsWith("privatekey") || + normalized.endsWith("clientsecret") + ); +} + +function redactSensitiveConfigValue(value: unknown): unknown { + if (value === undefined || value === null || value === false || value === "") { + return value; + } + if (typeof value === "string") { + return value.trim() ? "[redacted:string]" : ""; + } + if (typeof value === "number") { + return Number.isFinite(value) && value !== 0 ? "[redacted:number]" : value; + } + if (typeof value === "boolean") { + return value; + } + if (Array.isArray(value)) { + return value.length === 0 ? [] : "[redacted:array]"; + } + return "[redacted:object]"; +} + +function redactConfigForSkillSnapshotCache(value: unknown, stack = new WeakSet()): unknown { + if (!value || typeof value !== "object") { + return value; + } + if (stack.has(value)) { + return "[Circular]"; + } + stack.add(value); + try { + if (Array.isArray(value)) { + return value.map((entry) => redactConfigForSkillSnapshotCache(entry, stack)); + } + const redacted: Record = {}; + for (const key of Object.keys(value as Record).toSorted()) { + const field = (value as Record)[key]; + redacted[key] = isSensitiveConfigKey(key) + ? redactSensitiveConfigValue(field) + : redactConfigForSkillSnapshotCache(field, stack); + } + return redacted; + } finally { + stack.delete(value); + } +} + +// Skill frontmatter `requires.config` reads the full OpenClaw config, so cache +// reuse must follow the same boundary without putting raw secrets in Map keys. +function fingerprintSkillSnapshotConfig(config: OpenClawConfig): string { + return crypto + .createHash("sha256") + .update(stableStringify(redactConfigForSkillSnapshotCache(config))) + .digest("hex"); +} + +function cacheResolvedSkills(cacheKey: string, snapshot: SkillSnapshot): SkillSnapshot { + resolvedSkillsCache.set(cacheKey, snapshot.resolvedSkills); + if (resolvedSkillsCache.size > RESOLVED_SKILLS_CACHE_MAX) { + const oldest = resolvedSkillsCache.keys().next().value; + if (oldest !== undefined) { + resolvedSkillsCache.delete(oldest); + } + } + return snapshot; +} + // nextEntry.skillsSnapshot may carry resolvedSkills (full Skill[] with // SKILL.md bodies) for in-turn use. The persistence layer in // src/config/sessions/store-load.ts strips resolvedSkills before serializing, @@ -177,14 +264,34 @@ export async function ensureSkillSnapshot(params: { const shouldRefreshSnapshot = shouldRefreshSnapshotForVersion(existingSnapshot?.version, snapshotVersion) || !matchesSkillFilter(existingSnapshot?.skillFilter, skillFilter); - const buildSnapshot = () => - buildWorkspaceSkillSnapshot(workspaceDir, { + const buildSnapshot = () => { + return buildWorkspaceSkillSnapshot(workspaceDir, { config: cfg, agentId: sessionAgentId, skillFilter, eligibility: { remote: remoteEligibility }, snapshotVersion, }); + }; + + const configFingerprint = fingerprintSkillSnapshotConfig(cfg); + const snapshotCacheKey = JSON.stringify([ + workspaceDir, + snapshotVersion, + skillFilter, + sessionAgentId, + remoteEligibility, + configFingerprint, + ]); + + const cachedRebuild = (): SkillSnapshot => { + if (resolvedSkillsCache.has(snapshotCacheKey)) { + return { resolvedSkills: resolvedSkillsCache.get(snapshotCacheKey) } as SkillSnapshot; + } + return cacheResolvedSkills(snapshotCacheKey, buildSnapshot()); + }; + + const buildAndCache = (): SkillSnapshot => cacheResolvedSkills(snapshotCacheKey, buildSnapshot()); if (isFirstTurnInSession && sessionStore && sessionKey) { const current = nextEntry ?? @@ -194,8 +301,8 @@ export async function ensureSkillSnapshot(params: { }; const skillSnapshot = !current.skillsSnapshot || shouldRefreshSnapshot - ? buildSnapshot() - : hydrateResolvedSkills(current.skillsSnapshot, buildSnapshot); + ? buildAndCache() + : hydrateResolvedSkills(current.skillsSnapshot, cachedRebuild); nextEntry = { ...current, sessionId: sessionId ?? current.sessionId ?? crypto.randomUUID(), @@ -212,10 +319,10 @@ export async function ensureSkillSnapshot(params: { (nextEntry?.skillsSnapshot !== existingSnapshot || !shouldRefreshSnapshot); const skillsSnapshot = hasFreshSnapshotInEntry && nextEntry?.skillsSnapshot - ? hydrateResolvedSkills(nextEntry.skillsSnapshot, buildSnapshot) + ? hydrateResolvedSkills(nextEntry.skillsSnapshot, cachedRebuild) : shouldRefreshSnapshot || !nextEntry?.skillsSnapshot - ? buildSnapshot() - : hydrateResolvedSkills(nextEntry.skillsSnapshot, buildSnapshot); + ? buildAndCache() + : hydrateResolvedSkills(nextEntry.skillsSnapshot, cachedRebuild); if ( skillsSnapshot && sessionStore &&