From a3cacf3b6770576bb9aeac40723a6351b1254f16 Mon Sep 17 00:00:00 2001 From: Amogh Asgekar Date: Fri, 1 May 2026 17:03:07 -0700 Subject: [PATCH] perf(sessions): stop persisting skillsSnapshot.resolvedSkills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each SessionEntry carried the fully parsed Skill[] (including each SKILL.md body) inside skillsSnapshot.resolvedSkills, multiplied across every active session. Strip the field at the persistence chokepoint — normalizeSessionStore in store-load.ts — so every load and every save naturally drops it. The runtime already falls back to a workspace skill scan when resolvedSkills is absent (see src/agents/pi-embedded-runner/skills-runtime.ts:14), so prompts and session resume behavior are unchanged. Legacy sessions.json files self-heal on first load: normalize strips the in-memory store, the next write rewrites the file in stripped form. Test fixture (100 sessions × 50 skills × ~3KB body) goes from ~32MB to under 2MB on disk. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/auto-reply/reply/session-updates.ts | 5 + src/config/sessions/store-load.ts | 19 +- .../sessions/store.skills-stripping.test.ts | 205 ++++++++++++++++++ src/config/sessions/types.ts | 8 + 4 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 src/config/sessions/store.skills-stripping.test.ts diff --git a/src/auto-reply/reply/session-updates.ts b/src/auto-reply/reply/session-updates.ts index e774f73546a..dc876bf38e0 100644 --- a/src/auto-reply/reply/session-updates.ts +++ b/src/auto-reply/reply/session-updates.ts @@ -26,6 +26,11 @@ import { normalizeOptionalString } from "../../shared/string-coerce.js"; import { buildSessionEndHookPayload, buildSessionStartHookPayload } from "./session-hooks.js"; export { drainFormattedSystemEvents } from "./session-system-events.js"; +// 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, +// so the on-disk sessions.json stays small. The in-memory params.sessionStore +// reference still carries the runtime cache for the rest of this turn. async function persistSessionEntryUpdate(params: { sessionStore?: Record; sessionKey?: string; diff --git a/src/config/sessions/store-load.ts b/src/config/sessions/store-load.ts index 85a3e168bf5..c9141ec649c 100644 --- a/src/config/sessions/store-load.ts +++ b/src/config/sessions/store-load.ts @@ -65,13 +65,30 @@ function normalizeSessionEntryDelivery(entry: SessionEntry): SessionEntry { }; } +// resolvedSkills carries the full parsed Skill[] (including each SKILL.md body) +// and is only used as an in-turn cache by the runtime — see +// src/agents/pi-embedded-runner/skills-runtime.ts. Persisting it bloats +// sessions.json by orders of magnitude when many sessions are active. Strip +// it from every entry that flows through normalize, so neither the in-memory +// store reloaded from disk nor the JSON serialized back to disk carries it. +function stripPersistedSkillsCache(entry: SessionEntry): SessionEntry { + const snapshot = entry.skillsSnapshot; + if (!snapshot || snapshot.resolvedSkills === undefined) { + return entry; + } + const { resolvedSkills: _drop, ...rest } = snapshot; + return { ...entry, skillsSnapshot: rest }; +} + export function normalizeSessionStore(store: Record): boolean { let changed = false; for (const [key, entry] of Object.entries(store)) { if (!entry) { continue; } - const normalized = normalizeSessionEntryDelivery(normalizeSessionRuntimeModelFields(entry)); + const normalized = stripPersistedSkillsCache( + normalizeSessionEntryDelivery(normalizeSessionRuntimeModelFields(entry)), + ); if (normalized !== entry) { store[key] = normalized; changed = true; diff --git a/src/config/sessions/store.skills-stripping.test.ts b/src/config/sessions/store.skills-stripping.test.ts new file mode 100644 index 00000000000..70e386d3fc2 --- /dev/null +++ b/src/config/sessions/store.skills-stripping.test.ts @@ -0,0 +1,205 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { resolveEmbeddedRunSkillEntries } from "../../agents/pi-embedded-runner/skills-runtime.js"; +import { createCanonicalFixtureSkill } from "../../agents/skills.test-helpers.js"; +import type { Skill } from "../../agents/skills/skill-contract.js"; +import { createSuiteTempRootTracker } from "../../test-helpers/temp-dir.js"; +import type { SessionEntry, SessionSkillSnapshot } from "./types.js"; + +vi.mock("../config.js", async () => ({ + ...(await vi.importActual("../config.js")), + getRuntimeConfig: vi.fn().mockReturnValue({}), +})); + +import { + clearSessionStoreCacheForTest, + loadSessionStore, + saveSessionStore, + updateSessionStore, +} from "./store.js"; + +const suiteRootTracker = createSuiteTempRootTracker({ prefix: "openclaw-skills-strip-" }); + +function makeFixtureSkill(name: string, bodySize = 3000): Skill { + // 3KB body simulates a realistic SKILL.md. + const source = `# ${name}\n\n${"x".repeat(bodySize)}`; + return createCanonicalFixtureSkill({ + name, + description: `${name} skill description`, + filePath: `/skills/${name}/SKILL.md`, + baseDir: `/skills/${name}`, + source, + }); +} + +function makeSnapshot(skillCount: number): SessionSkillSnapshot { + const resolved = Array.from({ length: skillCount }, (_, i) => makeFixtureSkill(`skill-${i}`)); + return { + prompt: "...", + skills: resolved.map((s) => ({ name: s.name })), + skillFilter: undefined, + resolvedSkills: resolved, + version: 1, + }; +} + +function makeEntry(sessionId: string, snapshot?: SessionSkillSnapshot): SessionEntry { + return { + sessionId, + updatedAt: Date.now(), + skillsSnapshot: snapshot, + }; +} + +describe("session store strips resolvedSkills from persistence", () => { + let testDir: string; + let storePath: string; + let savedCacheTtl: string | undefined; + + beforeAll(async () => { + await suiteRootTracker.setup(); + }); + + afterAll(async () => { + await suiteRootTracker.cleanup(); + }); + + beforeEach(async () => { + testDir = await suiteRootTracker.make("case"); + storePath = path.join(testDir, "sessions.json"); + savedCacheTtl = process.env.OPENCLAW_SESSION_CACHE_TTL_MS; + process.env.OPENCLAW_SESSION_CACHE_TTL_MS = "0"; + clearSessionStoreCacheForTest(); + }); + + afterEach(() => { + clearSessionStoreCacheForTest(); + if (savedCacheTtl === undefined) { + delete process.env.OPENCLAW_SESSION_CACHE_TTL_MS; + } else { + process.env.OPENCLAW_SESSION_CACHE_TTL_MS = savedCacheTtl; + } + }); + + it("does not write resolvedSkills to disk", async () => { + const store = { + "agent:main:test:1": makeEntry("session-1", makeSnapshot(5)), + }; + + await saveSessionStore(storePath, store, { skipMaintenance: true }); + + const raw = await fs.readFile(storePath, "utf-8"); + expect(raw).not.toContain("resolvedSkills"); + expect(raw).not.toContain("xxxxx"); // none of the skill source bodies leaked + const parsed = JSON.parse(raw) as Record; + expect(parsed["agent:main:test:1"]?.skillsSnapshot?.resolvedSkills).toBeUndefined(); + }); + + it("preserves prompt, skills, skillFilter, and version on roundtrip", async () => { + const snapshot = makeSnapshot(3); + snapshot.skillFilter = ["skill-0"]; + const store = { + "agent:main:test:1": makeEntry("session-1", snapshot), + }; + + await saveSessionStore(storePath, store, { skipMaintenance: true }); + const loaded = loadSessionStore(storePath, { skipCache: true }); + + const persistedSnapshot = loaded["agent:main:test:1"]?.skillsSnapshot; + expect(persistedSnapshot).toBeDefined(); + expect(persistedSnapshot?.prompt).toBe(snapshot.prompt); + expect(persistedSnapshot?.skills).toEqual(snapshot.skills); + expect(persistedSnapshot?.skillFilter).toEqual(["skill-0"]); + expect(persistedSnapshot?.version).toBe(1); + expect(persistedSnapshot?.resolvedSkills).toBeUndefined(); + }); + + it("strips resolvedSkills from a legacy sessions.json on load", async () => { + // Hand-craft a pre-fix file with embedded resolvedSkills. + const legacy = { + "agent:main:test:1": makeEntry("session-1", makeSnapshot(4)), + }; + await fs.mkdir(path.dirname(storePath), { recursive: true }); + const rawLegacy = JSON.stringify(legacy, null, 2); + expect(rawLegacy).toContain("resolvedSkills"); + await fs.writeFile(storePath, rawLegacy, "utf-8"); + + const loaded = loadSessionStore(storePath, { skipCache: true }); + expect(loaded["agent:main:test:1"]?.skillsSnapshot?.resolvedSkills).toBeUndefined(); + expect(loaded["agent:main:test:1"]?.skillsSnapshot?.prompt).toBe( + legacy["agent:main:test:1"].skillsSnapshot?.prompt, + ); + + // Saving the loaded record should rewrite the file in stripped form. + await saveSessionStore(storePath, loaded, { skipMaintenance: true }); + const rawAfter = await fs.readFile(storePath, "utf-8"); + expect(rawAfter).not.toContain("resolvedSkills"); + }); + + it("strips resolvedSkills written via updateSessionStore mutator", async () => { + // Simulate the production hot path where ensureSkillSnapshot puts a + // freshly-built snapshot (with resolvedSkills) into the store via mutator. + await updateSessionStore( + storePath, + (store) => { + store["agent:main:test:1"] = makeEntry("session-1", makeSnapshot(6)); + }, + { skipMaintenance: true }, + ); + + const raw = await fs.readFile(storePath, "utf-8"); + expect(raw).not.toContain("resolvedSkills"); + const reloaded = loadSessionStore(storePath, { skipCache: true }); + expect(reloaded["agent:main:test:1"]?.skillsSnapshot?.resolvedSkills).toBeUndefined(); + expect(reloaded["agent:main:test:1"]?.skillsSnapshot?.skills).toHaveLength(6); + }); + + it("keeps the on-disk file small with many sessions and skills", async () => { + const SESSION_COUNT = 100; + const SKILLS_PER_SESSION = 50; + const store: Record = {}; + for (let i = 0; i < SESSION_COUNT; i += 1) { + store[`agent:main:scale:${i}`] = makeEntry(`session-${i}`, makeSnapshot(SKILLS_PER_SESSION)); + } + + await saveSessionStore(storePath, store, { skipMaintenance: true }); + + const stat = await fs.stat(storePath); + // Pre-fix: ~SESSION_COUNT * SKILLS_PER_SESSION * ~3KB ≈ 15MB. + // Post-fix: only the lightweight `skills` array + prompt per entry. + // Conservative budget that comfortably covers metadata growth. + expect(stat.size).toBeLessThan(2 * 1024 * 1024); + }); +}); + +describe("embedded runner falls back to disk when resolvedSkills is absent", () => { + it("signals shouldLoadSkillEntries when the persisted snapshot has no resolvedSkills", () => { + const result = resolveEmbeddedRunSkillEntries({ + workspaceDir: "/nonexistent-workspace-for-test", + skillsSnapshot: { + prompt: "", + skills: [{ name: "x" }], + version: 1, + // resolvedSkills intentionally omitted — this is the post-fix shape. + }, + }); + + expect(result.shouldLoadSkillEntries).toBe(true); + }); + + it("skips loading when resolvedSkills is present (in-turn cache hot path)", () => { + const result = resolveEmbeddedRunSkillEntries({ + workspaceDir: "/nonexistent-workspace-for-test", + skillsSnapshot: { + prompt: "", + skills: [{ name: "x" }], + resolvedSkills: [makeFixtureSkill("x", 100)], + version: 1, + }, + }); + + expect(result.shouldLoadSkillEntries).toBe(false); + expect(result.skillEntries).toEqual([]); + }); +}); diff --git a/src/config/sessions/types.ts b/src/config/sessions/types.ts index 3ddb0d66f71..ab042ca5812 100644 --- a/src/config/sessions/types.ts +++ b/src/config/sessions/types.ts @@ -530,6 +530,14 @@ export type SessionSkillSnapshot = { skills: Array<{ name: string; primaryEnv?: string; requiredEnv?: string[] }>; /** Normalized agent-level filter used to build this snapshot; undefined means unrestricted. */ skillFilter?: string[]; + /** + * Runtime-only, never persisted. Carries the full parsed Skill[] (including + * each SKILL.md body) so the embedded runner can skip a workspace skill + * scan within a turn. Stripped from sessions.json on every read and write + * via normalizeSessionStore — see store-load.ts. On a cold session resume + * this is undefined and src/agents/pi-embedded-runner/skills-runtime.ts + * rebuilds it by reloading skill entries from disk. + */ resolvedSkills?: Skill[]; version?: number; };