mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 20:30:45 +00:00
fix(sessions): hydrate skillsSnapshot.resolvedSkills on resume
Codex review on PR #75960 flagged that prepareClaudeCliSkillsPlugin and the claude-live-session fingerprint read skillsSnapshot.resolvedSkills directly without a disk fallback. After the persistence-layer strip, those consumers would see an empty Skill[] on cold session resume, breaking the documented Claude Code skills integration. Add a hydration helper in ensureSkillSnapshot that rebuilds resolvedSkills from a fresh workspace scan when the loaded snapshot lacks it, while keeping the persisted prompt/skills/skillFilter/version fields untouched so the model's prompt-cache key stays stable across resume. The embedded runner's existing consumer-level fallback in src/agents/pi-embedded-runner/skills-runtime.ts is now a redundant safety net rather than the only fallback path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
Peter Steinberger
parent
643ff4d778
commit
479ed596bd
@@ -104,6 +104,22 @@ function resolvePositiveTokenCount(value: number | undefined): number | undefine
|
||||
: undefined;
|
||||
}
|
||||
|
||||
// resolvedSkills is stripped from the persisted snapshot (see store-load.ts).
|
||||
// On cold session resume, the snapshot loaded from disk reaches this code path
|
||||
// without resolvedSkills. Consumers like prepareClaudeCliSkillsPlugin and the
|
||||
// claude-live-session fingerprint read resolvedSkills directly, so re-fill it
|
||||
// here from a fresh workspace scan while preserving the persisted prompt /
|
||||
// skills / version fields for prompt-cache stability.
|
||||
export function hydrateResolvedSkills(
|
||||
snapshot: NonNullable<SessionEntry["skillsSnapshot"]>,
|
||||
rebuild: () => NonNullable<SessionEntry["skillsSnapshot"]>,
|
||||
): NonNullable<SessionEntry["skillsSnapshot"]> {
|
||||
if (snapshot.resolvedSkills) {
|
||||
return snapshot;
|
||||
}
|
||||
return { ...snapshot, resolvedSkills: rebuild().resolvedSkills };
|
||||
}
|
||||
|
||||
export async function ensureSkillSnapshot(params: {
|
||||
sessionEntry?: SessionEntry;
|
||||
sessionStore?: Record<string, SessionEntry>;
|
||||
@@ -175,7 +191,9 @@ export async function ensureSkillSnapshot(params: {
|
||||
updatedAt: Date.now(),
|
||||
};
|
||||
const skillSnapshot =
|
||||
!current.skillsSnapshot || shouldRefreshSnapshot ? buildSnapshot() : current.skillsSnapshot;
|
||||
!current.skillsSnapshot || shouldRefreshSnapshot
|
||||
? buildSnapshot()
|
||||
: hydrateResolvedSkills(current.skillsSnapshot, buildSnapshot);
|
||||
nextEntry = {
|
||||
...current,
|
||||
sessionId: sessionId ?? current.sessionId ?? crypto.randomUUID(),
|
||||
@@ -190,11 +208,12 @@ export async function ensureSkillSnapshot(params: {
|
||||
const hasFreshSnapshotInEntry =
|
||||
Boolean(nextEntry?.skillsSnapshot) &&
|
||||
(nextEntry?.skillsSnapshot !== existingSnapshot || !shouldRefreshSnapshot);
|
||||
const skillsSnapshot = hasFreshSnapshotInEntry
|
||||
? nextEntry?.skillsSnapshot
|
||||
: shouldRefreshSnapshot || !nextEntry?.skillsSnapshot
|
||||
? buildSnapshot()
|
||||
: nextEntry.skillsSnapshot;
|
||||
const skillsSnapshot =
|
||||
hasFreshSnapshotInEntry && nextEntry?.skillsSnapshot
|
||||
? hydrateResolvedSkills(nextEntry.skillsSnapshot, buildSnapshot)
|
||||
: shouldRefreshSnapshot || !nextEntry?.skillsSnapshot
|
||||
? buildSnapshot()
|
||||
: hydrateResolvedSkills(nextEntry.skillsSnapshot, buildSnapshot);
|
||||
if (
|
||||
skillsSnapshot &&
|
||||
sessionStore &&
|
||||
|
||||
@@ -4,6 +4,7 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi }
|
||||
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 { hydrateResolvedSkills } from "../../auto-reply/reply/session-updates.js";
|
||||
import { createSuiteTempRootTracker } from "../../test-helpers/temp-dir.js";
|
||||
import type { SessionEntry, SessionSkillSnapshot } from "./types.js";
|
||||
|
||||
@@ -203,3 +204,69 @@ describe("embedded runner falls back to disk when resolvedSkills is absent", ()
|
||||
expect(result.skillEntries).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("hydrateResolvedSkills", () => {
|
||||
it("returns the same snapshot when resolvedSkills is already populated", () => {
|
||||
const snapshot: SessionSkillSnapshot = {
|
||||
prompt: "p",
|
||||
skills: [{ name: "x" }],
|
||||
resolvedSkills: [makeFixtureSkill("x", 100)],
|
||||
version: 1,
|
||||
};
|
||||
let buildCalls = 0;
|
||||
const result = hydrateResolvedSkills(snapshot, () => {
|
||||
buildCalls += 1;
|
||||
return { prompt: "rebuilt", skills: [], resolvedSkills: [], version: 99 };
|
||||
});
|
||||
expect(result).toBe(snapshot);
|
||||
expect(buildCalls).toBe(0);
|
||||
});
|
||||
|
||||
it("rebuilds resolvedSkills only when missing and preserves persisted fields", () => {
|
||||
// Simulates a cold session resume: the on-disk snapshot has no
|
||||
// resolvedSkills, but consumers like prepareClaudeCliSkillsPlugin still
|
||||
// need them. Hydration must not change prompt/skills/version, so the
|
||||
// model's prompt-cache key stays stable across resume.
|
||||
const stripped: SessionSkillSnapshot = {
|
||||
prompt: "original-prompt",
|
||||
skills: [{ name: "x" }],
|
||||
skillFilter: ["x"],
|
||||
version: 7,
|
||||
};
|
||||
const rebuiltSkills = [makeFixtureSkill("x", 200)];
|
||||
let buildCalls = 0;
|
||||
const result = hydrateResolvedSkills(stripped, () => {
|
||||
buildCalls += 1;
|
||||
return {
|
||||
prompt: "DIFFERENT-PROMPT",
|
||||
skills: [{ name: "y" }],
|
||||
resolvedSkills: rebuiltSkills,
|
||||
version: 99,
|
||||
};
|
||||
});
|
||||
expect(buildCalls).toBe(1);
|
||||
expect(result.prompt).toBe("original-prompt");
|
||||
expect(result.skills).toEqual([{ name: "x" }]);
|
||||
expect(result.skillFilter).toEqual(["x"]);
|
||||
expect(result.version).toBe(7);
|
||||
expect(result.resolvedSkills).toBe(rebuiltSkills);
|
||||
});
|
||||
|
||||
it("hydrates an empty resolvedSkills array as if it were absent is NOT done — empty is treated as populated", () => {
|
||||
// A resolvedSkills set explicitly to [] means the workspace genuinely had
|
||||
// no skills, not that the field was stripped. Don't trigger a rebuild.
|
||||
const snapshot: SessionSkillSnapshot = {
|
||||
prompt: "",
|
||||
skills: [],
|
||||
resolvedSkills: [],
|
||||
version: 1,
|
||||
};
|
||||
let buildCalls = 0;
|
||||
const result = hydrateResolvedSkills(snapshot, () => {
|
||||
buildCalls += 1;
|
||||
return { prompt: "", skills: [], resolvedSkills: [makeFixtureSkill("x")], version: 1 };
|
||||
});
|
||||
expect(result).toBe(snapshot);
|
||||
expect(buildCalls).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user