From d9bef3fe7cba0d7481a74605d2e59b0869fd6602 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 27 Apr 2026 11:10:38 -0700 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + ui/src/ui/app-channels.test.ts | 136 +++++++++++++++++++++++++++ ui/src/ui/app-channels.ts | 13 ++- ui/src/ui/app-render.ts | 6 +- ui/src/ui/controllers/config.test.ts | 106 +++++++++++++++++++++ ui/src/ui/controllers/config.ts | 45 ++++++--- 6 files changed, 288 insertions(+), 19 deletions(-) create mode 100644 ui/src/ui/app-channels.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 950bb1cffac..96b82c2dce0 100644 --- 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. diff --git a/ui/src/ui/app-channels.test.ts b/ui/src/ui/app-channels.test.ts new file mode 100644 index 00000000000..53e58f15d8e --- /dev/null +++ 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 = 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); + }); +}); diff --git a/ui/src/ui/app-channels.ts b/ui/src/ui/app-channels.ts index 3ec7f7c9e8c..c22fc06cbdc 100644 --- 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); } diff --git a/ui/src/ui/app-render.ts b/ui/src/ui/app-render.ts index fce3d90c700..5fb83e7a74b 100644 --- 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, 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 diff --git a/ui/src/ui/controllers/config.test.ts b/ui/src/ui/controllers/config.test.ts index 1ccd11b5984..2923586b8e5 100644 --- 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(); diff --git a/ui/src/ui/controllers/config.ts b/ui/src/ui/controllers/config.ts index eeb2d37cafb..5434e5833de 100644 --- 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("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 = {}, -) { +): Promise { 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) state.configFormDirty = nextRaw !== originalRaw; } -export async function saveConfig(state: ConfigState) { - await submitConfigChange(state, "config.set", "configSaving"); +export async function saveConfig(state: ConfigState): Promise { + 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 { + 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) {