fix(ui): discard stale config state on explicit reload (#72624)

* fix(ui): discard stale config state on explicit reload * fix(clownfish): address review for ghcrawl-156594-autonomous-smoke (1) * fix(clownfish): address review for ghcrawl-156594-autonomous-smoke (1) * test(ui): align channel config host state
2026-05-06 17:40:44 +00:00 · 2026-04-27 11:10:38 -07:00
parent be6263da4f
commit d9bef3fe7c
6 changed files with 288 additions and 19 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -139,6 +139,7 @@ Docs: https://docs.openclaw.ai
 - Agents/tools: normalize `null` or missing tool-call arguments to `{}` for parameterless object schemas before Pi validation, so empty-argument tools run instead of failing argument validation. Fixes #72587. Thanks @amknight.
 - Agents/subagents: clear active embedded-run state before terminal lifecycle events so post-completion cleanup no longer treats finished child runs as still active and skips archive or announcement bookkeeping. (#70187) Thanks @amknight.
 - CLI/update: keep the automatic post-update completion refresh on the core-command tree so it no longer stages bundled plugin runtime deps before the Gateway restart path, avoiding `.24` update hangs and 1006 disconnect cascades. Fixes #72665. Thanks @sakalaboator and @He-Pin.
+- Control UI: make explicit Reload Config actions discard stale local config edits while passive refreshes and failed-save recovery keep pending drafts intact. Fixes #40352; carries forward #40443. Thanks @realmikechong-dotcom.
 - Agents/Bedrock: stop heartbeat runs from persisting blank user transcript turns and repair existing blank user text messages before replay, preventing AWS Bedrock `ContentBlock` blank-text validation failures. Fixes #72640 and #72622. Thanks @goldzulu.
 - Agents/LM Studio: promote standalone bracketed local-model tool requests into registered tool calls and hide unsupported bracket blocks from visible replies, so MemPalace MCP lookups do not print raw `[tool]` JSON scaffolding in chat. Fixes #66178. Thanks @detroit357.
 - Local models: warn when an assistant reply looks like a tool call but the provider emitted plain text instead of a structured tool invocation, making fake/non-executed tool calls visible in logs. Fixes #51332. Thanks @emilclaw.
--- a/ui/src/ui/app-channels.test.ts
+++ b/ui/src/ui/app-channels.test.ts
@@ -0,0 +1,136 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { handleChannelConfigReload, handleChannelConfigSave } from "./app-channels.ts";
+import type { ChannelsState } from "./controllers/channels.ts";
+import type { ConfigState } from "./controllers/config.ts";
+import type { ChannelsStatusSnapshot } from "./types.ts";
+
+type ChannelsActionHostForTest = ConfigState &
+  ChannelsState & {
+    hello?: { auth?: { deviceToken?: string | null } | null } | null;
+    password?: string;
+    settings: { token?: string };
+    nostrProfileFormState: null;
+    nostrProfileAccountId: string | null;
+  };
+
+function createChannelsSnapshot(name = "saved"): ChannelsStatusSnapshot {
+  const nostrAccount = { accountId: "default", configured: true, profile: { name } } as
+    ChannelsStatusSnapshot["channelAccounts"][string][number];
+  return {
+    ts: Date.now(),
+    channelOrder: ["nostr"],
+    channelLabels: { nostr: "Nostr" },
+    channels: { nostr: { configured: true } },
+    channelAccounts: {
+      nostr: [nostrAccount],
+    },
+    channelDefaultAccountId: { nostr: "default" },
+  };
+}
+
+function createHost(request: ReturnType<typeof vi.fn> = vi.fn()): ChannelsActionHostForTest {
+  return {
+    applySessionKey: "main",
+    channelsError: null,
+    channelsLastSuccess: null,
+    channelsLoading: false,
+    channelsSnapshot: createChannelsSnapshot("old"),
+    client: { request } as unknown as ConfigState["client"],
+    configActiveSection: null,
+    configActiveSubsection: null,
+    configApplying: false,
+    configForm: null,
+    configFormDirty: false,
+    configFormMode: "form",
+    configFormOriginal: null,
+    configIssues: [],
+    configLoading: false,
+    configRaw: "",
+    configRawOriginal: "",
+    configSaving: false,
+    configSchema: null,
+    configSchemaLoading: false,
+    configSchemaVersion: null,
+    configSearchQuery: "",
+    configSnapshot: null,
+    configUiHints: {},
+    configValid: null,
+    connected: true,
+    lastError: null,
+    nostrProfileAccountId: null,
+    nostrProfileFormState: null,
+    pendingUpdateExpectedVersion: null,
+    settings: {},
+    updateStatusBanner: null,
+    updateRunning: false,
+    whatsappBusy: false,
+    whatsappLoginConnected: null,
+    whatsappLoginMessage: null,
+    whatsappLoginQrDataUrl: null,
+  };
+}
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe("channel config actions", () => {
+  it("discards stale dirty config state on explicit reload", async () => {
+    const request = vi.fn().mockImplementation(async (method: string) => {
+      if (method === "config.get") {
+        return {
+          config: { gateway: { mode: "remote" } },
+          valid: true,
+          issues: [],
+          raw: '{\n  "gateway": { "mode": "remote" }\n}\n',
+        };
+      }
+      if (method === "channels.status") {
+        return createChannelsSnapshot();
+      }
+      return {};
+    });
+    const host = createHost(request);
+    host.configFormDirty = true;
+    host.configForm = { gateway: { mode: "local" } };
+
+    await handleChannelConfigReload(host);
+
+    expect(host.configFormDirty).toBe(false);
+    expect(host.configForm).toEqual({ gateway: { mode: "remote" } });
+    expect(host.configFormOriginal).toEqual({ gateway: { mode: "remote" } });
+    expect(request).toHaveBeenCalledWith("channels.status", { probe: true, timeoutMs: 8000 });
+  });
+
+  it("keeps failed channel saves from discarding pending edits during recovery reload", async () => {
+    const request = vi.fn().mockImplementation(async (method: string) => {
+      if (method === "config.set") {
+        throw new Error("Config hash mismatch");
+      }
+      if (method === "config.get") {
+        return {
+          config: { gateway: { mode: "remote" } },
+          valid: true,
+          issues: [],
+          raw: '{\n  "gateway": { "mode": "remote" }\n}\n',
+        };
+      }
+      if (method === "channels.status") {
+        return createChannelsSnapshot();
+      }
+      return {};
+    });
+    const host = createHost(request);
+    host.configSnapshot = { hash: "old-hash" };
+    host.configFormDirty = true;
+    host.configForm = { gateway: { mode: "local" } };
+
+    await handleChannelConfigSave(host);
+
+    expect(host.lastError).toContain("Config hash mismatch");
+    expect(host.configFormDirty).toBe(true);
+    expect(host.configForm).toEqual({ gateway: { mode: "local" } });
+    expect(host.configSnapshot?.config).toEqual({ gateway: { mode: "remote" } });
+    expect(request.mock.calls.some(([method]) => method === "channels.status")).toBe(false);
+  });
+});
--- a/ui/src/ui/app-channels.ts
+++ b/ui/src/ui/app-channels.ts
@@ -37,13 +37,20 @@ export async function handleWhatsAppLogout(host: ChannelsActionHost) {
 }

 export async function handleChannelConfigSave(host: ChannelsActionHost) {
-  await saveConfig(host as ConfigState);
-  await loadConfig(host as ConfigState);
+  const saved = await saveConfig(host as ConfigState);
+  const saveError = host.lastError;
+  if (!saved) {
+    await loadConfig(host as ConfigState);
+    if (saveError && !host.lastError) {
+      host.lastError = saveError;
+    }
+    return;
+  }
  await loadChannels(host as ChannelsState, true);
 }

 export async function handleChannelConfigReload(host: ChannelsActionHost) {
-  await loadConfig(host as ConfigState);
+  await loadConfig(host as ConfigState, { discardPendingChanges: true });
  await loadChannels(host as ChannelsState, true);
 }

--- a/ui/src/ui/app-render.ts
+++ b/ui/src/ui/app-render.ts
@@ -869,7 +869,7 @@ export function renderApp(state: AppViewState) {
    onRequestUpdate: requestHostUpdate,
    onFormPatch: (path: Array<string | number>, value: unknown) =>
      updateConfigFormValue(state, path, value),
-    onReload: () => loadConfig(state),
+    onReload: () => loadConfig(state, { discardPendingChanges: true }),
    onReset: () => resetConfigPendingChanges(state),
    onSave: () => saveConfig(state),
    onApply: () => applyConfig(state),
@@ -2027,7 +2027,7 @@ export function renderApp(state: AppViewState) {
                    removeConfigFormValue(state, [...basePath, "deny"]);
                  }
                },
-                onConfigReload: () => loadConfig(state),
+                onConfigReload: () => loadConfig(state, { discardPendingChanges: true }),
                onConfigSave: () => saveAgentsConfig(state),
                onChannelsRefresh: () => loadChannels(state, false),
                onCronRefresh: () => state.loadCron(),
@@ -2245,7 +2245,7 @@ export function renderApp(state: AppViewState) {
                onDeviceRotate: (deviceId, role, scopes) =>
                  rotateDeviceToken(state, { deviceId, role, scopes }),
                onDeviceRevoke: (deviceId, role) => revokeDeviceToken(state, { deviceId, role }),
-                onLoadConfig: () => loadConfig(state),
+                onLoadConfig: () => loadConfig(state, { discardPendingChanges: true }),
                onLoadExecApprovals: () => {
                  const target =
                    state.execApprovalsTarget === "node" && state.execApprovalsTargetNodeId
--- a/ui/src/ui/controllers/config.test.ts
+++ b/ui/src/ui/controllers/config.test.ts
@@ -4,6 +4,7 @@ import {
  applyConfig,
  ensureAgentConfigEntry,
  findAgentConfigEntryIndex,
+  loadConfig,
  resetConfigPendingChanges,
  runUpdate,
  saveConfig,
@@ -33,6 +34,7 @@ function createState(): ConfigState {
    configSchemaVersion: null,
    configSearchQuery: "",
    configSnapshot: null,
+    configDraftBaseHash: null,
    configUiHints: {},
    configValid: null,
    connected: false,
@@ -115,6 +117,59 @@ describe("applyConfigSnapshot", () => {
    expect(state.configFormOriginal).toEqual({ original: true });
  });

+  it("keeps the draft base hash when preserving dirty edits across refreshes", () => {
+    const state = createState();
+    applyConfigSnapshot(state, {
+      hash: "hash-original",
+      config: { gateway: { mode: "local" } },
+      valid: true,
+      issues: [],
+      raw: '{ "gateway": { "mode": "local" } }',
+    });
+
+    updateConfigFormValue(state, ["gateway", "mode"], "remote");
+    applyConfigSnapshot(state, {
+      hash: "hash-refreshed",
+      config: { gateway: { mode: "external" } },
+      valid: true,
+      issues: [],
+      raw: '{ "gateway": { "mode": "external" } }',
+    });
+
+    expect(state.configSnapshot?.hash).toBe("hash-refreshed");
+    expect(state.configDraftBaseHash).toBe("hash-original");
+    expect(state.configForm).toEqual({ gateway: { mode: "remote" } });
+  });
+
+  it("discards dirty form edits when explicitly requested", () => {
+    const state = createState();
+    state.configFormMode = "form";
+    state.configFormDirty = true;
+    state.configForm = { gateway: { mode: "local", port: 18789 } };
+    state.configFormOriginal = { gateway: { mode: "local", port: 18789 } };
+    state.configRawOriginal =
+      '{\n  "gateway": {\n    "mode": "local",\n    "port": 18789\n  }\n}\n';
+
+    applyConfigSnapshot(
+      state,
+      {
+        hash: "hash-remote",
+        config: { gateway: { mode: "remote", port: 9999 } },
+        valid: true,
+        issues: [],
+        raw: '{\n  "gateway": { "mode": "remote", "port": 9999 }\n}\n',
+      },
+      { discardPendingChanges: true },
+    );
+
+    expect(state.configFormDirty).toBe(false);
+    expect(state.configForm).toEqual({ gateway: { mode: "remote", port: 9999 } });
+    expect(state.configFormOriginal).toEqual({ gateway: { mode: "remote", port: 9999 } });
+    expect(state.configRaw).toBe('{\n  "gateway": { "mode": "remote", "port": 9999 }\n}\n');
+    expect(state.configRawOriginal).toBe('{\n  "gateway": { "mode": "remote", "port": 9999 }\n}\n');
+    expect(state.configDraftBaseHash).toBe("hash-remote");
+  });
+
  it("forces form mode when the snapshot does not include raw text", () => {
    const state = createState();
    state.configFormMode = "raw";
@@ -131,6 +186,28 @@ describe("applyConfigSnapshot", () => {
  });
 });

+describe("loadConfig", () => {
+  it("passes explicit reload mode through to snapshot application", async () => {
+    const request = vi.fn().mockResolvedValue({
+      config: { gateway: { mode: "remote" } },
+      valid: true,
+      issues: [],
+      raw: '{\n  "gateway": { "mode": "remote" }\n}\n',
+    });
+    const state = createState();
+    state.connected = true;
+    state.client = { request } as unknown as ConfigState["client"];
+    state.configFormDirty = true;
+    state.configForm = { gateway: { mode: "local" } };
+
+    await loadConfig(state, { discardPendingChanges: true });
+
+    expect(state.configFormDirty).toBe(false);
+    expect(state.configForm).toEqual({ gateway: { mode: "remote" } });
+    expect(state.configRawOriginal).toBe('{\n  "gateway": { "mode": "remote" }\n}\n');
+  });
+});
+
 describe("updateConfigFormValue", () => {
  it("seeds from snapshot when form is null", () => {
    const state = createState();
@@ -469,6 +546,35 @@ describe("applyConfig", () => {
 });

 describe("saveConfig", () => {
+  it("submits the original draft base hash after a dirty config refresh", async () => {
+    const request = createRequestWithConfigGet();
+    const state = createState();
+    state.connected = true;
+    state.client = { request } as unknown as ConfigState["client"];
+    state.configFormMode = "form";
+    applyConfigSnapshot(state, {
+      hash: "hash-original",
+      config: { gateway: { mode: "local" } },
+      valid: true,
+      issues: [],
+      raw: '{\n  "gateway": { "mode": "local" }\n}\n',
+    });
+    updateConfigFormValue(state, ["gateway", "mode"], "remote");
+    applyConfigSnapshot(state, {
+      hash: "hash-refreshed",
+      config: { gateway: { mode: "external" } },
+      valid: true,
+      issues: [],
+      raw: '{\n  "gateway": { "mode": "external" }\n}\n',
+    });
+
+    await saveConfig(state);
+
+    expect(request.mock.calls[0]?.[0]).toBe("config.set");
+    const params = request.mock.calls[0]?.[1] as { raw: string; baseHash: string };
+    expect(params.baseHash).toBe("hash-original");
+  });
+
  it("coerces schema-typed values before config.set in form mode", async () => {
    const request = createRequestWithConfigGet();
    const state = createState();
--- a/ui/src/ui/controllers/config.ts
+++ b/ui/src/ui/controllers/config.ts
@@ -23,6 +23,7 @@ export type ConfigState = {
  configApplying: boolean;
  updateRunning: boolean;
  configSnapshot: ConfigSnapshot | null;
+  configDraftBaseHash?: string | null;
  configSchema: unknown;
  configSchemaVersion: string | null;
  configSchemaLoading: boolean;
@@ -39,7 +40,11 @@ export type ConfigState = {
  lastError: string | null;
 };

-export async function loadConfig(state: ConfigState) {
+export type LoadConfigOptions = {
+  discardPendingChanges?: boolean;
+};
+
+export async function loadConfig(state: ConfigState, options: LoadConfigOptions = {}) {
  if (!state.client || !state.connected) {
    return;
  }
@@ -47,7 +52,7 @@ export async function loadConfig(state: ConfigState) {
  state.lastError = null;
  try {
    const res = await state.client.request<ConfigSnapshot>("config.get", {});
-    applyConfigSnapshot(state, res);
+    applyConfigSnapshot(state, res, options);
  } catch (err) {
    state.lastError = String(err);
  } finally {
@@ -79,7 +84,13 @@ export function applyConfigSchema(state: ConfigState, res: ConfigSchemaResponse)
  state.configSchemaVersion = res.version ?? null;
 }

-export function applyConfigSnapshot(state: ConfigState, snapshot: ConfigSnapshot) {
+export function applyConfigSnapshot(
+  state: ConfigState,
+  snapshot: ConfigSnapshot,
+  options: LoadConfigOptions = {},
+) {
+  const preservePendingChanges = state.configFormDirty && options.discardPendingChanges !== true;
+  const draftBaseHash = state.configDraftBaseHash ?? state.configSnapshot?.hash ?? null;
  state.configSnapshot = snapshot;
  const rawAvailable = typeof snapshot.raw === "string";
  if (!rawAvailable && state.configFormMode === "raw") {
@@ -91,7 +102,7 @@ export function applyConfigSnapshot(state: ConfigState, snapshot: ConfigSnapshot
      : snapshot.config && typeof snapshot.config === "object"
        ? serializeConfigForm(snapshot.config)
        : state.configRaw;
-  if (!state.configFormDirty || state.configFormMode === "raw") {
+  if (!preservePendingChanges || state.configFormMode === "raw") {
    state.configRaw = rawFromSnapshot;
  } else if (state.configForm) {
    state.configRaw = serializeConfigForm(state.configForm);
@@ -101,10 +112,14 @@ export function applyConfigSnapshot(state: ConfigState, snapshot: ConfigSnapshot
  state.configValid = typeof snapshot.valid === "boolean" ? snapshot.valid : null;
  state.configIssues = Array.isArray(snapshot.issues) ? snapshot.issues : [];

-  if (!state.configFormDirty) {
+  if (!preservePendingChanges) {
    state.configForm = cloneConfigObject(snapshot.config ?? {});
    state.configFormOriginal = cloneConfigObject(snapshot.config ?? {});
    state.configRawOriginal = rawFromSnapshot;
+    state.configFormDirty = false;
+    state.configDraftBaseHash = snapshot.hash ?? null;
+  } else {
+    state.configDraftBaseHash = draftBaseHash;
  }
 }

@@ -179,24 +194,27 @@ async function submitConfigChange(
  method: ConfigSubmitMethod,
  busyKey: ConfigSubmitBusyKey,
  extraParams: Record<string, unknown> = {},
-) {
+): Promise<boolean> {
  if (!state.client || !state.connected) {
-    return;
+    return false;
  }
  state[busyKey] = true;
  state.lastError = null;
  try {
    const raw = serializeFormForSubmit(state);
-    const baseHash = state.configSnapshot?.hash;
+    const baseHash = state.configDraftBaseHash ?? state.configSnapshot?.hash;
    if (!baseHash) {
      state.lastError = "Config hash missing; reload and retry.";
-      return;
+      return false;
    }
    await state.client.request(method, { raw, baseHash, ...extraParams });
    state.configFormDirty = false;
+    state.configDraftBaseHash = null;
    await loadConfig(state);
+    return true;
  } catch (err) {
    state.lastError = String(err);
+    return false;
  } finally {
    state[busyKey] = false;
  }
@@ -213,12 +231,12 @@ function syncConfigDraft(state: ConfigState, nextForm: Record<string, unknown>)
  state.configFormDirty = nextRaw !== originalRaw;
 }

-export async function saveConfig(state: ConfigState) {
-  await submitConfigChange(state, "config.set", "configSaving");
+export async function saveConfig(state: ConfigState): Promise<boolean> {
+  return submitConfigChange(state, "config.set", "configSaving");
 }

-export async function applyConfig(state: ConfigState) {
-  await submitConfigChange(state, "config.apply", "configApplying", {
+export async function applyConfig(state: ConfigState): Promise<boolean> {
+  return submitConfigChange(state, "config.apply", "configApplying", {
    sessionKey: state.applySessionKey,
  });
 }
@@ -296,6 +314,7 @@ export function resetConfigPendingChanges(state: ConfigState) {
    state.configRawOriginal ??
    serializeConfigForm(state.configFormOriginal ?? state.configSnapshot?.config ?? {});
  state.configFormDirty = false;
+  state.configDraftBaseHash = state.configSnapshot?.hash ?? null;
 }

 export function removeConfigFormValue(state: ConfigState, path: Array<string | number>) {