diff --git a/ui/src/lib/skills/config-mutations.ts b/ui/src/lib/skills/config-mutations.ts new file mode 100644 index 000000000000..92207b6c8065 --- /dev/null +++ b/ui/src/lib/skills/config-mutations.ts @@ -0,0 +1,47 @@ +import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import type { RuntimeConfigCapability } from "../config/index.ts"; + +export type SkillConfigMutationOwner = Pick; + +type SkillConfigPatch = { skillKey: string; enabled?: boolean; apiKey?: string }; + +export function normalizeSkillApiKeyReplacement(value: string | undefined): string | undefined { + const apiKey = value?.trim(); + // Blank skills.update API keys explicitly clear stored credentials; this UI only replaces them. + return apiKey || undefined; +} + +export async function runSkillConfigMutation( + owner: SkillConfigMutationOwner, + expectedClient: GatewayBrowserClient, + patch: SkillConfigPatch, +): Promise { + let requestError: Error | undefined; + // Settings autosave and skills.update persist the same config; one owner + // prevents a pending draft from restoring an older skill credential/toggle. + const mutation = await owner.runExternalMutation(async (client) => { + if (client !== expectedClient) { + throw new Error("Connection changed before the skill update started."); + } + try { + return await client.request("skills.update", patch); + } catch (error) { + requestError = error instanceof Error ? error : new Error(String(error)); + throw requestError; + } + }); + if (!mutation.ok) { + throw requestError ?? new Error(mutation.error); + } + return mutation.refresh.ok ? null : mutation.refresh.error; +} + +export function skillConfigMutationSuccess( + message: string, + refreshError: string | null, +): { kind: "success"; message: string } { + return { + kind: "success", + message: refreshError ? `${message}\n${refreshError}` : message, + }; +} diff --git a/ui/src/lib/skills/index.test.ts b/ui/src/lib/skills/index.test.ts index 3a766d4e4256..fc932f00b543 100644 --- a/ui/src/lib/skills/index.test.ts +++ b/ui/src/lib/skills/index.test.ts @@ -3,6 +3,7 @@ import { expectDefined } from "@openclaw/normalization-core"; import { describe, expect, it, vi } from "vitest"; import { waitForFast } from "../../test-helpers/wait-for.ts"; +import { createRuntimeConfigCapability } from "../config/index.ts"; import { installFromClawHub, installSkill, @@ -37,6 +38,23 @@ function createState(): { state: SkillsState; request: ReturnType { + try { + return { + ok: true, + value: await task(expectDefined(state.client, "connected skill mutation client")), + refresh: { ok: true }, + }; + } catch (error) { + return { + ok: false, + reason: "error", + error: error instanceof Error ? error.message : String(error), + }; + } + }, + }, skillsAgentId: null, skillsAgentRevision: 0, skillsLoading: false, @@ -706,7 +724,7 @@ describe("skill mutations", () => { { name: "saves API keys and reports success", run: async (state: SkillsState) => { - state.skillEdits.github = "sk-test"; + state.skillEdits.github = " sk-test "; await saveSkillApiKey(state, "github"); }, expectedRequest: ["skills.update", { skillKey: "github", apiKey: "sk-test" }], @@ -740,6 +758,88 @@ describe("skill mutations", () => { expect(state.skillsError).toBeNull(); }); + it.each([undefined, "", " ", "\t\n"])( + "does not clear an existing API key when its replacement is blank: %j", + async (editValue) => { + const { state, request } = createState(); + if (editValue !== undefined) { + state.skillEdits.github = editValue; + } + + await saveSkillApiKey(state, "github"); + + expect(request).not.toHaveBeenCalled(); + expect(state.skillMessages.github).toBeUndefined(); + expect(state.skillOperation).toBeNull(); + }, + ); + + it("serializes skill changes after pending settings drafts and refreshes both owners", async () => { + const { state, request } = createState(); + const methods: string[] = []; + let storedConfig: Record = { count: 1 }; + let hash = "hash-1"; + request.mockImplementation(async (method, params) => { + methods.push(method); + if (method === "config.get") { + return { + config: storedConfig, + raw: JSON.stringify(storedConfig), + hash, + valid: true, + issues: [], + }; + } + if (method === "config.set") { + storedConfig = JSON.parse((params as { raw: string }).raw) as Record; + hash = "hash-2"; + return { hash }; + } + if (method === "skills.update") { + storedConfig = { ...storedConfig, skillEnabled: true }; + hash = "hash-3"; + return {}; + } + return { workspaceDir: "/tmp/workspace", managedSkillsDir: "/tmp/skills", skills: [] }; + }); + const client = expectDefined(state.client, "connected skill mutation client"); + const runtimeConfig = createRuntimeConfigCapability({ + snapshot: { client, phase: "connected", sessionKey: "main" }, + subscribe: () => () => undefined, + }); + state.runtimeConfig = runtimeConfig; + await runtimeConfig.ensureLoaded(); + methods.length = 0; + + try { + runtimeConfig.patchForm(["count"], 2); + await updateSkillEnabled(state, "github", true); + + expect(methods).toEqual(["config.set", "skills.update", "config.get", "skills.status"]); + expect(runtimeConfig.state.configForm).toEqual({ count: 2, skillEnabled: true }); + expect(state.skillMessages.github).toEqual({ kind: "success", message: "Skill enabled" }); + } finally { + runtimeConfig.dispose(); + } + }); + + it("reports a committed skill update when its configuration refresh fails", async () => { + const { state, request } = createState(); + state.runtimeConfig.runExternalMutation = async (task) => ({ + ok: true, + value: await task(expectDefined(state.client, "connected skill mutation client")), + refresh: { ok: false, error: "Configuration refresh failed" }, + }); + mockSkillMutationRequests(request); + + await updateSkillEnabled(state, "github", true); + + expect(state.skillMessages.github).toEqual({ + kind: "success", + message: "Skill enabled\nConfiguration refresh failed", + }); + }); + it.each([ { name: "skill update blocks ClawHub install", diff --git a/ui/src/lib/skills/index.ts b/ui/src/lib/skills/index.ts index 5a7b02c17020..277d77116fd8 100644 --- a/ui/src/lib/skills/index.ts +++ b/ui/src/lib/skills/index.ts @@ -9,6 +9,12 @@ import type { SkillStatusEntry, SkillStatusReport, } from "../../api/types.ts"; +import { + normalizeSkillApiKeyReplacement, + runSkillConfigMutation, + skillConfigMutationSuccess, + type SkillConfigMutationOwner, +} from "./config-mutations.ts"; export type ClawHubSearchResult = { score: number; @@ -78,6 +84,7 @@ export type ClawHubSkillSecurityVerdict = { type SkillsState = { client: GatewayBrowserClient | null; connected: boolean; + runtimeConfig: SkillConfigMutationOwner; skillsAgentId: string | null; skillsAgentRevision: number; skillsLoading: boolean; @@ -561,22 +568,28 @@ async function runSkillMutation( export async function updateSkillEnabled(state: SkillsState, skillKey: string, enabled: boolean) { await runSkillMutation(state, skillKey, async (client) => { - await client.request("skills.update", { skillKey, enabled }); - return { - kind: "success", - message: enabled ? "Skill enabled" : "Skill disabled", - }; + const refreshError = await runSkillConfigMutation(state.runtimeConfig, client, { + skillKey, + enabled, + }); + return skillConfigMutationSuccess(enabled ? "Skill enabled" : "Skill disabled", refreshError); }); } export async function saveSkillApiKey(state: SkillsState, skillKey: string) { + const apiKey = normalizeSkillApiKeyReplacement(state.skillEdits[skillKey]); + if (!apiKey) { + return; + } await runSkillMutation(state, skillKey, async (client) => { - const editValue = state.skillEdits[skillKey] ?? ""; - await client.request("skills.update", { skillKey, apiKey: editValue }); - return { - kind: "success", - message: `API key saved — stored in openclaw.json (skills.entries.${skillKey})`, - }; + const refreshError = await runSkillConfigMutation(state.runtimeConfig, client, { + skillKey, + apiKey, + }); + return skillConfigMutationSuccess( + `API key saved — stored in openclaw.json (skills.entries.${skillKey})`, + refreshError, + ); }); } diff --git a/ui/src/pages/skills/skills-page.ts b/ui/src/pages/skills/skills-page.ts index 6acdb399271c..e48225e91261 100644 --- a/ui/src/pages/skills/skills-page.ts +++ b/ui/src/pages/skills/skills-page.ts @@ -96,6 +96,10 @@ class SkillsPage extends OpenClawLightDomElement { @state() skillCardLoadingKey: string | null = null; @state() skillCardErrors: Record = {}; + get runtimeConfig(): ApplicationContext["runtimeConfig"] { + return this.context.runtimeConfig; + } + private clawhubSearchTimer: ReturnType | null = null; private routeDataInitialized = false; private routeDataEnabled = true; diff --git a/ui/src/pages/skills/view.test.ts b/ui/src/pages/skills/view.test.ts index 28a63667176e..8c683b9c5012 100644 --- a/ui/src/pages/skills/view.test.ts +++ b/ui/src/pages/skills/view.test.ts @@ -194,6 +194,50 @@ describe("renderSkills", () => { ); }); + it.each([ + { editValue: "", disabled: true }, + { editValue: " ", disabled: true }, + { editValue: " sk-test ", disabled: false }, + ])( + "only enables credential replacement for nonblank input: $editValue", + async ({ editValue, disabled }) => { + const container = document.createElement("div"); + document.body.append(container); + dialogRestores.push(() => container.remove()); + installDialogMethod("showModal", function (this: HTMLDialogElement) { + this.setAttribute("open", ""); + }); + const onSaveKey = vi.fn(); + + render( + renderSkills( + createProps({ + detailKey: "repo-skill", + edits: { "repo-skill": editValue }, + onSaveKey, + }), + ), + container, + ); + await Promise.resolve(); + + const input = container.querySelector('input[type="password"]'); + const save = Array.from(container.querySelectorAll("button")).find( + (button) => normalizeText(button) === "Save key", + ); + expect(input?.required).toBe(true); + expect(save?.disabled).toBe(disabled); + + save?.click(); + + if (disabled) { + expect(onSaveKey).not.toHaveBeenCalled(); + } else { + expect(onSaveKey).toHaveBeenCalledWith("repo-skill"); + } + }, + ); + it("renders skill groups as open collapsible sections with heading summaries", async () => { const container = document.createElement("div"); document.body.append(container); diff --git a/ui/src/pages/skills/view.ts b/ui/src/pages/skills/view.ts index 967fd770e6b7..674dbcfbf598 100644 --- a/ui/src/pages/skills/view.ts +++ b/ui/src/pages/skills/view.ts @@ -737,6 +737,7 @@ function renderSkillDetail(skill: SkillStatusEntry, props: SkillsProps) { > @@ -756,7 +757,7 @@ function renderSkillDetail(skill: SkillStatusEntry, props: SkillsProps) { })()}