diff --git a/src/skills/runtime/refresh-state.test.ts b/src/skills/runtime/refresh-state.test.ts new file mode 100644 index 000000000000..ae96d1d895a2 --- /dev/null +++ b/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); + }); +}); diff --git a/src/skills/runtime/refresh-state.ts b/src/skills/runtime/refresh-state.ts index dc866d30ec35..ea93101ba099 100644 --- a/src/skills/runtime/refresh-state.ts +++ b/src/skills/runtime/refresh-state.ts @@ -7,7 +7,8 @@ export type SkillsChangeEvent = { const listeners = new Set<(event: SkillsChangeEvent) => void>(); const workspaceVersions = new Map(); -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; } diff --git a/src/skills/runtime/session-snapshot.test.ts b/src/skills/runtime/session-snapshot.test.ts index 91c03d7ef9cc..503700972bd2 100644 --- a/src/skills/runtime/session-snapshot.test.ts +++ b/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;