fix(skills): refresh persisted snapshots after restart (#93513)

* fix(skills): refresh persisted snapshots after restart

Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>

Co-authored-by: Oleksandr Zakotyanskyi <28755978+fif911@users.noreply.github.com>

Co-authored-by: Stephan Kadauke <10904538+skadauke@users.noreply.github.com>

* fix(clownfish): address review for ghcrawl-156600-autonomous-smoke (1)

Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>

Co-authored-by: Oleksandr Zakotyanskyi <28755978+fif911@users.noreply.github.com>

Co-authored-by: Stephan Kadauke <10904538+skadauke@users.noreply.github.com>

---------

Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com>
Co-authored-by: Stephan Kadauke <10904538+skadauke@users.noreply.github.com>

src/skills/runtime/refresh-state.test.ts

@@ -0,0 +1,44 @@
+// Skill refresh state tests cover snapshot version invalidation contracts.
+import { beforeEach, describe, expect, it } from "vitest";
+import {
+  bumpSkillsSnapshotVersion,
+  getSkillsSnapshotVersion,
+  resetSkillsRefreshStateForTest,
+  shouldRefreshSnapshotForVersion,
+} from "./refresh-state.js";
+
+describe("skills refresh state", () => {
+  beforeEach(() => {
+    resetSkillsRefreshStateForTest();
+  });
+
+  it("starts above persisted version 0 so restarted sessions refresh once", () => {
+    const currentVersion = getSkillsSnapshotVersion("/tmp/workspace");
+
+    expect(currentVersion).toBeGreaterThan(0);
+    expect(shouldRefreshSnapshotForVersion(0, currentVersion)).toBe(true);
+  });
+
+  it("starts above persisted timestamp versions from earlier processes", () => {
+    const currentVersion = getSkillsSnapshotVersion("/tmp/workspace");
+    const previousProcessVersion = currentVersion - 1;
+
+    expect(shouldRefreshSnapshotForVersion(previousProcessVersion, currentVersion)).toBe(true);
+  });
+
+  it("reuses snapshots already built for the current startup version", () => {
+    const currentVersion = getSkillsSnapshotVersion("/tmp/workspace");
+
+    expect(shouldRefreshSnapshotForVersion(currentVersion, currentVersion)).toBe(false);
+  });
+
+  it("keeps workspace and global bumps above the startup version", () => {
+    const startupVersion = getSkillsSnapshotVersion("/tmp/workspace");
+    const workspaceVersion = bumpSkillsSnapshotVersion({ workspaceDir: "/tmp/workspace" });
+    const globalVersion = bumpSkillsSnapshotVersion();
+
+    expect(workspaceVersion).toBeGreaterThan(startupVersion);
+    expect(globalVersion).toBeGreaterThanOrEqual(workspaceVersion);
+    expect(getSkillsSnapshotVersion("/tmp/workspace")).toBe(globalVersion);
+  });
+});

src/skills/runtime/refresh-state.ts

@@ -7,7 +7,8 @@ export type SkillsChangeEvent = {
 
 const listeners = new Set<(event: SkillsChangeEvent) => void>();
 const workspaceVersions = new Map<string, number>();
-let globalVersion = 0;
+const INITIAL_SKILLS_SNAPSHOT_VERSION = Date.now();
+let globalVersion = INITIAL_SKILLS_SNAPSHOT_VERSION;
 let listenerErrorHandler: ((err: unknown) => void) | undefined;
 
 function bumpVersion(current: number): number {
@@ -85,6 +86,6 @@ export function shouldRefreshSnapshotForVersion(
 export function resetSkillsRefreshStateForTest(): void {
   listeners.clear();
   workspaceVersions.clear();
-  globalVersion = 0;
+  globalVersion = INITIAL_SKILLS_SNAPSHOT_VERSION;
   listenerErrorHandler = undefined;
 }

src/skills/runtime/session-snapshot.test.ts

@@ -6,11 +6,11 @@ import type { SkillSnapshot } from "../types.js";
 
 const TEST_WORKSPACE_DIR = "/tmp/workspace";
 
-function strippedSnapshot(skillName = "test"): SkillSnapshot {
+function strippedSnapshot(skillName = "test", version = 1): SkillSnapshot {
   return {
     prompt: "skills prompt",
     skills: [{ name: skillName }],
-    version: 0,
+    version,
     promptFormatVersion: WORKSPACE_SKILLS_PROMPT_FORMAT_VERSION,
   };
 }
@@ -27,8 +27,10 @@ const {
     resolvedSkills: [] as unknown[],
   })),
   ensureSkillsWatcherMock: vi.fn(),
-  getSkillsSnapshotVersionMock: vi.fn(() => 0),
-  shouldRefreshSnapshotForVersionMock: vi.fn((_cached?: number, _next?: number) => false),
+  getSkillsSnapshotVersionMock: vi.fn(() => 1),
+  shouldRefreshSnapshotForVersionMock: vi.fn((cached = 0, next = 0) =>
+    next === 0 ? cached > 0 : cached < next,
+  ),
 }));
 
 vi.mock("../loading/workspace.js", () => ({
@@ -52,8 +54,10 @@ describe("resolveReusableWorkspaceSkillSnapshot", () => {
     vi.clearAllMocks();
     resetResolvedSkillsCacheForTests();
     buildWorkspaceSkillSnapshotMock.mockReturnValue({ prompt: "", skills: [], resolvedSkills: [] });
-    getSkillsSnapshotVersionMock.mockReturnValue(0);
-    shouldRefreshSnapshotForVersionMock.mockReturnValue(false);
+    getSkillsSnapshotVersionMock.mockReturnValue(1);
+    shouldRefreshSnapshotForVersionMock.mockImplementation((cached = 0, next = 0) =>
+      next === 0 ? cached > 0 : cached < next,
+    );
   });
 
   it("reuses cached resolvedSkills across calls with the same workspace, version, and filter", () => {
@@ -97,19 +101,18 @@ describe("resolveReusableWorkspaceSkillSnapshot", () => {
   });
 
   it("reads the skills snapshot version after watcher-side invalidation", () => {
-    getSkillsSnapshotVersionMock.mockReturnValue(0);
+    getSkillsSnapshotVersionMock.mockReturnValue(1);
     ensureSkillsWatcherMock.mockImplementation(() => {
       getSkillsSnapshotVersionMock.mockReturnValue(5);
     });
-    shouldRefreshSnapshotForVersionMock.mockImplementation((cached = 0, next = 0) => cached < next);
 
     resolveReusableWorkspaceSkillSnapshot({
       workspaceDir: TEST_WORKSPACE_DIR,
       config: { skills: { load: { extraDirs: ["/tmp/shared-skills"] } } },
-      existingSnapshot: strippedSnapshot(),
+      existingSnapshot: strippedSnapshot("test", 1),
     });
 
-    expect(shouldRefreshSnapshotForVersionMock).toHaveBeenCalledWith(0, 5);
+    expect(shouldRefreshSnapshotForVersionMock).toHaveBeenCalledWith(1, 5);
     expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1);
     const [[, snapshotParams]] = buildWorkspaceSkillSnapshotMock.mock.calls as unknown as Array<
       [string, { snapshotVersion?: number }]
@@ -117,6 +120,40 @@ describe("resolveReusableWorkspaceSkillSnapshot", () => {
     expect(snapshotParams.snapshotVersion).toBe(5);
   });
 
+  it("refreshes persisted version-0 snapshots after process restart", () => {
+    const result = resolveReusableWorkspaceSkillSnapshot({
+      workspaceDir: TEST_WORKSPACE_DIR,
+      config: {},
+      existingSnapshot: strippedSnapshot("test", 0),
+    });
+
+    expect(result.shouldRefresh).toBe(true);
+    expect(shouldRefreshSnapshotForVersionMock).toHaveBeenCalledWith(0, 1);
+    expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1);
+    const [[, snapshotParams]] = buildWorkspaceSkillSnapshotMock.mock.calls as unknown as Array<
+      [string, { snapshotVersion?: number }]
+    >;
+    expect(snapshotParams.snapshotVersion).toBe(1);
+  });
+
+  it("refreshes persisted timestamp-version snapshots from earlier processes", () => {
+    getSkillsSnapshotVersionMock.mockReturnValue(10_000);
+
+    const result = resolveReusableWorkspaceSkillSnapshot({
+      workspaceDir: TEST_WORKSPACE_DIR,
+      config: {},
+      existingSnapshot: strippedSnapshot("test", 9_999),
+    });
+
+    expect(result.shouldRefresh).toBe(true);
+    expect(shouldRefreshSnapshotForVersionMock).toHaveBeenCalledWith(9_999, 10_000);
+    expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledTimes(1);
+    const [[, snapshotParams]] = buildWorkspaceSkillSnapshotMock.mock.calls as unknown as Array<
+      [string, { snapshotVersion?: number }]
+    >;
+    expect(snapshotParams.snapshotVersion).toBe(10_000);
+  });
+
   it("invalidates cached resolvedSkills when non-skills config gates change", () => {
     buildWorkspaceSkillSnapshotMock.mockImplementation((_workspaceDir, opts) => {
       const config = (opts as { config?: { channels?: { discord?: { token?: string } } } }).config;