diff --git a/src/agents/skills-clawhub.ts b/src/agents/skills-clawhub.ts index 244c5c4b4ff..7952cb7f758 100644 --- a/src/agents/skills-clawhub.ts +++ b/src/agents/skills-clawhub.ts @@ -74,7 +74,7 @@ function normalizeTrackedSlug(raw: string): string { return slug; } -function normalizeSlug(raw: string): string { +function validateRequestedSlug(raw: string): string { const slug = normalizeTrackedSlug(raw); if (NON_ASCII_PATTERN.test(slug) || !VALID_SLUG_PATTERN.test(slug)) { throw new Error(`Invalid skill slug: ${raw}`); @@ -93,9 +93,31 @@ async function resolveRequestedUpdateSlug(params: { if (trackedOrigin || params.lock.skills[trackedSlug]) { return trackedSlug; } - return normalizeSlug(params.requestedSlug); + return validateRequestedSlug(params.requestedSlug); } +type ClawHubInstallParams = { + workspaceDir: string; + slug: string; + version?: string; + baseUrl?: string; + force?: boolean; + logger?: Logger; +}; + +type TrackedUpdateTarget = + | { + ok: true; + slug: string; + baseUrl?: string; + previousVersion: string | null; + } + | { + ok: false; + slug: string; + error: string; + }; + function resolveSkillInstallDir(workspaceDir: string, slug: string): string { const skillsDir = path.join(path.resolve(workspaceDir), "skills"); const target = resolveSafeInstallDir({ @@ -244,27 +266,16 @@ async function installExtractedSkill(params: { return { ok: true, targetDir }; } -async function installSkillFromClawHubInternal( - params: { - workspaceDir: string; - slug: string; - version?: string; - baseUrl?: string; - force?: boolean; - logger?: Logger; - }, - options?: { allowLegacyTrackedSlug?: boolean }, +async function performClawHubSkillInstall( + params: ClawHubInstallParams, ): Promise { try { - const slug = options?.allowLegacyTrackedSlug - ? normalizeTrackedSlug(params.slug) - : normalizeSlug(params.slug); const { detail, version } = await resolveInstallVersion({ - slug, + slug: params.slug, version: params.version, baseUrl: params.baseUrl, }); - const targetDir = resolveSkillInstallDir(params.workspaceDir, slug); + const targetDir = resolveSkillInstallDir(params.workspaceDir, params.slug); if (!params.force && (await fileExists(targetDir))) { return { ok: false, @@ -272,9 +283,9 @@ async function installSkillFromClawHubInternal( }; } - params.logger?.info?.(`Downloading ${slug}@${version} from ClawHub…`); + params.logger?.info?.(`Downloading ${params.slug}@${version} from ClawHub…`); const archive = await downloadClawHubSkillArchive({ - slug, + slug: params.slug, version, baseUrl: params.baseUrl, }); @@ -287,7 +298,7 @@ async function installSkillFromClawHubInternal( onExtracted: async (rootDir) => await installExtractedSkill({ workspaceDir: params.workspaceDir, - slug, + slug: params.slug, extractedRoot: rootDir, mode: params.force ? "update" : "install", logger: params.logger, @@ -301,12 +312,12 @@ async function installSkillFromClawHubInternal( await writeClawHubSkillOrigin(install.targetDir, { version: 1, registry: resolveClawHubBaseUrl(params.baseUrl), - slug, + slug: params.slug, installedVersion: version, installedAt, }); const lock = await readClawHubSkillsLockfile(params.workspaceDir); - lock.skills[slug] = { + lock.skills[params.slug] = { version, installedAt, }; @@ -314,7 +325,7 @@ async function installSkillFromClawHubInternal( return { ok: true, - slug, + slug: params.slug, version, targetDir: install.targetDir, detail, @@ -333,6 +344,61 @@ async function installSkillFromClawHubInternal( } } +async function installRequestedSkillFromClawHub( + params: ClawHubInstallParams, +): Promise { + try { + return await performClawHubSkillInstall({ + ...params, + slug: validateRequestedSlug(params.slug), + }); + } catch (err) { + return { + ok: false, + error: err instanceof Error ? err.message : String(err), + }; + } +} + +async function installTrackedSkillFromClawHub( + params: ClawHubInstallParams, +): Promise { + try { + return await performClawHubSkillInstall({ + ...params, + slug: normalizeTrackedSlug(params.slug), + }); + } catch (err) { + return { + ok: false, + error: err instanceof Error ? err.message : String(err), + }; + } +} + +async function resolveTrackedUpdateTarget(params: { + workspaceDir: string; + slug: string; + lock: ClawHubSkillsLockfile; + baseUrl?: string; +}): Promise { + const targetDir = resolveSkillInstallDir(params.workspaceDir, params.slug); + const origin = (await readClawHubSkillOrigin(targetDir)) ?? null; + if (!origin && !params.lock.skills[params.slug]) { + return { + ok: false, + slug: params.slug, + error: `Skill "${params.slug}" is not tracked as a ClawHub install.`, + }; + } + return { + ok: true, + slug: params.slug, + baseUrl: origin?.registry ?? params.baseUrl, + previousVersion: origin?.installedVersion ?? params.lock.skills[params.slug]?.version ?? null, + }; +} + export async function installSkillFromClawHub(params: { workspaceDir: string; slug: string; @@ -341,7 +407,7 @@ export async function installSkillFromClawHub(params: { force?: boolean; logger?: Logger; }): Promise { - return await installSkillFromClawHubInternal(params); + return await installRequestedSkillFromClawHub(params); } export async function updateSkillsFromClawHub(params: { @@ -362,37 +428,36 @@ export async function updateSkillsFromClawHub(params: { : Object.keys(lock.skills).map((slug) => normalizeTrackedSlug(slug)); const results: UpdateClawHubSkillResult[] = []; for (const slug of slugs) { - const targetDir = resolveSkillInstallDir(params.workspaceDir, slug); - const origin = (await readClawHubSkillOrigin(targetDir)) ?? null; - const baseUrl = origin?.registry ?? params.baseUrl; - if (!origin && !lock.skills[slug]) { + const tracked = await resolveTrackedUpdateTarget({ + workspaceDir: params.workspaceDir, + slug, + lock, + baseUrl: params.baseUrl, + }); + if (!tracked.ok) { results.push({ ok: false, - error: `Skill "${slug}" is not tracked as a ClawHub install.`, + error: tracked.error, }); continue; } - const previousVersion = origin?.installedVersion ?? lock.skills[slug]?.version ?? null; - const install = await installSkillFromClawHubInternal( - { - workspaceDir: params.workspaceDir, - slug, - baseUrl, - force: true, - logger: params.logger, - }, - { allowLegacyTrackedSlug: true }, - ); + const install = await installTrackedSkillFromClawHub({ + workspaceDir: params.workspaceDir, + slug: tracked.slug, + baseUrl: tracked.baseUrl, + force: true, + logger: params.logger, + }); if (!install.ok) { results.push(install); continue; } results.push({ ok: true, - slug, - previousVersion, + slug: tracked.slug, + previousVersion: tracked.previousVersion, version: install.version, - changed: previousVersion !== install.version, + changed: tracked.previousVersion !== install.version, targetDir: install.targetDir, }); } diff --git a/src/channels/plugins/plugins-core.test.ts b/src/channels/plugins/plugins-core.test.ts index d06d03caf5b..cc9c1b8875e 100644 --- a/src/channels/plugins/plugins-core.test.ts +++ b/src/channels/plugins/plugins-core.test.ts @@ -234,6 +234,7 @@ describe("channel plugin catalog", () => { env: { ...process.env, OPENCLAW_PLUGIN_CATALOG_PATHS: "~/catalog.json", + OPENCLAW_HOME: home, HOME: home, }, }).map((entry) => entry.id); diff --git a/src/config/version.test.ts b/src/config/version.test.ts index 2b967d11ec4..7e157710235 100644 --- a/src/config/version.test.ts +++ b/src/config/version.test.ts @@ -70,6 +70,7 @@ describe("isSameOpenClawStableFamily", () => { describe("shouldWarnOnTouchedVersion", () => { it("skips same-base stable families", () => { expect(shouldWarnOnTouchedVersion("2026.3.23", "2026.3.23-1")).toBe(false); + expect(shouldWarnOnTouchedVersion("2026.3.23-1", "2026.3.23-2")).toBe(false); }); it("skips same-base prerelease configs when current is newer", () => { @@ -79,5 +80,6 @@ describe("shouldWarnOnTouchedVersion", () => { it("warns when the touched config is newer", () => { expect(shouldWarnOnTouchedVersion("2026.3.23-beta.1", "2026.3.23")).toBe(true); expect(shouldWarnOnTouchedVersion("2026.3.23", "2026.3.24")).toBe(true); + expect(shouldWarnOnTouchedVersion("2026.3.23", "2027.1.1")).toBe(true); }); }); diff --git a/src/config/version.ts b/src/config/version.ts index 26e3a18a5d3..2febecc0835 100644 --- a/src/config/version.ts +++ b/src/config/version.ts @@ -105,7 +105,6 @@ export function shouldWarnOnTouchedVersion( const cmp = compareOpenClawVersions(current, touched); return cmp !== null && cmp < 0; } - function normalizeLegacyDotBetaVersion(version: string): string { const dotBetaMatch = /^([vV]?[0-9]+\.[0-9]+\.[0-9]+)\.beta(?:\.([0-9A-Za-z.-]+))?$/.exec(version); if (!dotBetaMatch) { diff --git a/src/cron/service.session-reaper-in-finally.test.ts b/src/cron/service.session-reaper-in-finally.test.ts index 3f43ad52d41..f76b51219f5 100644 --- a/src/cron/service.session-reaper-in-finally.test.ts +++ b/src/cron/service.session-reaper-in-finally.test.ts @@ -1,7 +1,11 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createNoopLogger, createCronStoreHarness } from "./service.test-harness.js"; +import { + createNoopLogger, + createCronStoreHarness, + withCronServiceStateForTest, +} from "./service.test-harness.js"; import { createCronServiceState } from "./service/state.js"; import { onTimer } from "./service/timer.js"; import { resetReaperThrottle } from "./session-reaper.js"; @@ -29,13 +33,6 @@ function createDueIsolatedJob(params: { id: string; nowMs: number }): CronJob { }; } -function clearCronTimer(state: { timer: NodeJS.Timeout | null }): void { - if (state.timer) { - clearTimeout(state.timer); - state.timer = null; - } -} - describe("CronService - session reaper runs in finally block (#31946)", () => { beforeEach(() => { noopLogger.debug.mockClear(); @@ -79,7 +76,7 @@ describe("CronService - session reaper runs in finally block (#31946)", () => { sessionStorePath, }); - try { + await withCronServiceStateForTest(state, async () => { await onTimer(state); // After onTimer finishes (even with a job error), state.running must be @@ -88,9 +85,7 @@ describe("CronService - session reaper runs in finally block (#31946)", () => { // The timer must be re-armed. expect(state.timer).not.toBeNull(); - } finally { - clearCronTimer(state); - } + }); }); it("session reaper runs when resolveSessionStorePath is provided", async () => { @@ -123,16 +118,14 @@ describe("CronService - session reaper runs in finally block (#31946)", () => { }, }); - try { + await withCronServiceStateForTest(state, async () => { await onTimer(state); // The resolveSessionStorePath callback should have been invoked to build // the set of store paths for the session reaper. expect(resolvedPaths.length).toBeGreaterThan(0); expect(state.running).toBe(false); - } finally { - clearCronTimer(state); - } + }); }); it("prunes expired cron-run sessions even when cron store load throws", async () => { @@ -168,7 +161,7 @@ describe("CronService - session reaper runs in finally block (#31946)", () => { sessionStorePath, }); - try { + await withCronServiceStateForTest(state, async () => { await expect(onTimer(state)).rejects.toThrow("Failed to parse cron store"); const updatedSessionStore = JSON.parse( @@ -176,8 +169,6 @@ describe("CronService - session reaper runs in finally block (#31946)", () => { ) as Record; expect(updatedSessionStore).toEqual({}); expect(state.running).toBe(false); - } finally { - clearCronTimer(state); - } + }); }); }); diff --git a/src/cron/service.test-harness.ts b/src/cron/service.test-harness.ts index d6bd04fb37f..2c288592375 100644 --- a/src/cron/service.test-harness.ts +++ b/src/cron/service.test-harness.ts @@ -204,6 +204,24 @@ export function createRunningCronServiceState(params: { return state; } +export function disposeCronServiceState(state: { timer: NodeJS.Timeout | null }): void { + if (state.timer) { + clearTimeout(state.timer); + state.timer = null; + } +} + +export async function withCronServiceStateForTest( + state: { timer: NodeJS.Timeout | null }, + run: () => Promise, +): Promise { + try { + return await run(); + } finally { + disposeCronServiceState(state); + } +} + export function createDeferred() { let resolve!: (value: T) => void; let reject!: (reason?: unknown) => void;