fix(ui): preserve Skills credentials and serialize configuration updates (#116749)

* fix(ui): preserve skill credentials and serialize updates

* refactor(ui): extract skill config mutation ownership

---------

Co-authored-by: Peter Steinberger <steipete@macos.shared>

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<RuntimeConfigCapability, "runExternalMutation">;
+
+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<string | null> {
+  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,
+  };
+}

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<typeof vi.fn<T
       request,
     } as unknown as SkillsState["client"],
     connected: true,
+    runtimeConfig: {
+      runExternalMutation: async (task) => {
+        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<string, unknown> = { 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<string, unknown>;
+        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",

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,
+    );
   });
 }
 

ui/src/pages/skills/skills-page.ts

@@ -96,6 +96,10 @@ class SkillsPage extends OpenClawLightDomElement {
   @state() skillCardLoadingKey: string | null = null;
   @state() skillCardErrors: Record<string, string> = {};
 
+  get runtimeConfig(): ApplicationContext["runtimeConfig"] {
+    return this.context.runtimeConfig;
+  }
+
   private clawhubSearchTimer: ReturnType<typeof setTimeout> | null = null;
   private routeDataInitialized = false;
   private routeDataEnabled = true;

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<HTMLInputElement>('input[type="password"]');
+      const save = Array.from(container.querySelectorAll<HTMLButtonElement>("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);

ui/src/pages/skills/view.ts

@@ -737,6 +737,7 @@ function renderSkillDetail(skill: SkillStatusEntry, props: SkillsProps) {
                     >
                     <input
                       type="password"
+                      required
                       ?disabled=${locked}
                       .value=${editValue}
                       @input=${(e: Event) =>
@@ -756,7 +757,7 @@ function renderSkillDetail(skill: SkillStatusEntry, props: SkillsProps) {
                   })()}
                   <button
                     class="btn primary"
-                    ?disabled=${locked}
+                    ?disabled=${locked || !editValue.trim()}
                     @click=${() => props.onSaveKey(skill.skillKey)}
                   >
                     ${t("skillsPage.saveKey")}