From d3fdd214c28fcd26208e70fc9536c2409aeac0a7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 12 Jul 2026 18:50:19 +0100 Subject: [PATCH] fix(ui): prevent configuration edits from disappearing during operations (#105555) * fix(ui): lock configuration during operations * chore: defer release note to release workflow --- ui/src/app/overlays.test.ts | 22 ++++++++++++ ui/src/app/overlays.ts | 5 +++ ui/src/lib/config/index.test.ts | 45 ++++++++++++++++++++++++ ui/src/lib/config/index.ts | 6 +++- ui/src/pages/config/config-page.ts | 9 ++++- ui/src/pages/config/quick.test.ts | 33 +++++++++++++++++ ui/src/pages/config/quick.ts | 30 ++++++++++------ ui/src/pages/config/view.browser.test.ts | 21 ++++++++--- ui/src/pages/config/view.ts | 22 ++++++------ 9 files changed, 166 insertions(+), 27 deletions(-) diff --git a/ui/src/app/overlays.test.ts b/ui/src/app/overlays.test.ts index 0ef361eae092..185460c2dc3b 100644 --- a/ui/src/app/overlays.test.ts +++ b/ui/src/app/overlays.test.ts @@ -259,6 +259,27 @@ describe("application update overlays", () => { text: "Update installed. A gateway restart is already in progress; status will refresh after it reconnects.", }); expect(overlays.snapshot.updateRunning).toBe(false); + expect(overlays.snapshot.updateReconciliationPending).toBe(true); + overlays.dispose(); + }); + + it("keeps reconciliation pending after a managed-service handoff starts", async () => { + const request = vi.fn().mockResolvedValue({ + ok: true, + handoff: { status: "started" }, + result: { + status: "skipped", + reason: "managed-service-handoff-started", + after: { version: "2.0.0" }, + }, + }); + const harness = createGatewayHarness(client(request)); + const overlays = createApplicationOverlays(harness.gateway); + + await overlays.runUpdate(); + + expect(overlays.snapshot.updateRunning).toBe(false); + expect(overlays.snapshot.updateReconciliationPending).toBe(true); overlays.dispose(); }); @@ -314,6 +335,7 @@ describe("application update overlays", () => { expect(statusRequests).toBe(2); expect(overlays.snapshot.updateStatusBanner).toBeNull(); + expect(overlays.snapshot.updateReconciliationPending).toBe(false); } finally { overlays.dispose(); vi.useRealTimers(); diff --git a/ui/src/app/overlays.ts b/ui/src/app/overlays.ts index 793aa6453b28..d2e77b7e3d3d 100644 --- a/ui/src/app/overlays.ts +++ b/ui/src/app/overlays.ts @@ -34,6 +34,7 @@ type ApplicationStatusBanner = { export type ApplicationOverlaySnapshot = { updateAvailable: UpdateAvailable | null; updateRunning: boolean; + updateReconciliationPending: boolean; updateStatusBanner: ApplicationStatusBanner | null; approvalQueue: readonly ExecApprovalRequest[]; approvalBusy: boolean; @@ -202,6 +203,7 @@ export function createApplicationOverlays(gateway: ApplicationGateway): Applicat let snapshot: ApplicationOverlaySnapshot = { updateAvailable: null, updateRunning: false, + updateReconciliationPending: false, updateStatusBanner: null, approvalQueue: [], approvalBusy: false, @@ -251,6 +253,9 @@ export function createApplicationOverlays(gateway: ApplicationGateway): Applicat snapshot = { updateAvailable: snapshot.updateAvailable, updateRunning: snapshot.updateRunning, + // The update RPC can finish before its restart handoff. Keep consumers + // locked until the replacement Gateway reports the authoritative result. + updateReconciliationPending: pendingUpdateHandoff || pendingUpdateExpectedVersion !== null, updateStatusBanner: snapshot.updateStatusBanner, approvalQueue: promptState.execApprovalQueue, approvalBusy: promptState.execApprovalBusy, diff --git a/ui/src/lib/config/index.test.ts b/ui/src/lib/config/index.test.ts index c2672d9f5dce..58880c2ef0a7 100644 --- a/ui/src/lib/config/index.test.ts +++ b/ui/src/lib/config/index.test.ts @@ -308,6 +308,51 @@ describe("loadConfig", () => { }); describe("createRuntimeConfigCapability", () => { + it("publishes the pending save state before the request settles", async () => { + const pendingSave = deferred(); + const request = vi.fn(async (method: string) => { + if (method === "config.set") { + await pendingSave.promise; + return {}; + } + if (method === "config.get") { + return { + hash: "saved-hash", + config: { source: "saved" }, + valid: true, + issues: [], + raw: '{"source":"saved"}', + }; + } + throw new Error(`unexpected request: ${method}`); + }); + const client = { request } as unknown as GatewayBrowserClient; + const { gateway } = createGatewayHarness(client); + const runtimeConfig = createRuntimeConfigCapability(gateway); + applyConfigSnapshot(runtimeConfig.state, { + hash: "base-hash", + config: { source: "base" }, + valid: true, + issues: [], + raw: '{"source":"base"}', + }); + updateConfigFormValue(runtimeConfig.state, ["source"], "draft"); + const savingStates: boolean[] = []; + const unsubscribe = runtimeConfig.subscribe((state) => savingStates.push(state.configSaving)); + + const operation = runtimeConfig.save(); + + expect(runtimeConfig.state.configSaving).toBe(true); + expect(savingStates).toEqual([true]); + + pendingSave.resolve(); + await expect(operation).resolves.toBe(true); + expect(runtimeConfig.state.configSaving).toBe(false); + expect(savingStates.at(-1)).toBe(false); + unsubscribe(); + runtimeConfig.dispose(); + }); + it("rejects stale config and schema work after reconnecting the same client", async () => { const firstConfig = deferred(); const secondConfig = deferred(); diff --git a/ui/src/lib/config/index.ts b/ui/src/lib/config/index.ts index 979fa4b91071..50eede3dc012 100644 --- a/ui/src/lib/config/index.ts +++ b/ui/src/lib/config/index.ts @@ -839,7 +839,11 @@ export function createRuntimeConfigCapability( }; const run = async (task: () => Promise): Promise => { try { - return await task(); + const result = task(); + // Async config owners mutate their busy flag before the first await. + // Publish that transition so editors can lock before accepting more input. + publish(); + return await result; } finally { publish(); } diff --git a/ui/src/pages/config/config-page.ts b/ui/src/pages/config/config-page.ts index 6591122e3169..98eed45fbaff 100644 --- a/ui/src/pages/config/config-page.ts +++ b/ui/src/pages/config/config-page.ts @@ -690,6 +690,11 @@ export class ConfigPage extends OpenClawLightDomElement { : undefined; } + private isUpdateBusy(): boolean { + const update = this.context.overlays.snapshot; + return update.updateRunning || update.updateReconciliationPending; + } + private renderAdvancedConfig(configObject: Record) { const runtimeConfig = this.context.runtimeConfig; const configState = runtimeConfig.state; @@ -720,7 +725,7 @@ export class ConfigPage extends OpenClawLightDomElement { loading: configState.configLoading, saving: configState.configSaving, applying: configState.configApplying, - updating: this.context.overlays.snapshot.updateRunning, + updating: this.isUpdateBusy(), connected: configState.connected, schema: configState.configSchema, schemaLoading: configState.configSchemaLoading, @@ -874,8 +879,10 @@ export class ConfigPage extends OpenClawLightDomElement { version: appConfig.serverVersion ?? this.context.gateway.snapshot.hello?.server?.version ?? "", configDirty: runtimeConfig.state.configFormDirty, + configLoading: runtimeConfig.state.configLoading, configSaving: runtimeConfig.state.configSaving, configApplying: runtimeConfig.state.configApplying, + configUpdating: this.isUpdateBusy(), configReady: Boolean(runtimeConfig.state.configSnapshot?.hash), onResetConfig: () => runtimeConfig.resetDraft(), onSaveConfig: () => void runtimeConfig.save(), diff --git a/ui/src/pages/config/quick.test.ts b/ui/src/pages/config/quick.test.ts index 846a5aa3a3be..1481a669f90c 100644 --- a/ui/src/pages/config/quick.test.ts +++ b/ui/src/pages/config/quick.test.ts @@ -341,6 +341,39 @@ describe("renderQuickSettings", () => { expect(onApplyConfig).toHaveBeenCalledTimes(1); }); + it("locks config-backed quick controls while a config operation is pending", () => { + const container = document.createElement("div"); + + for (const pending of [ + { configLoading: true }, + { configSaving: true }, + { configApplying: true }, + { configUpdating: true }, + ]) { + render(renderQuickSettings(createProps({ configDirty: true, ...pending })), container); + + const thinkingRow = expectRowByLabel(container, "Thinking"); + const fastModeRow = expectRowByLabel(container, "Fast mode"); + const browserRow = expectRowByLabel(container, "Browser enabled"); + const toolProfileRow = expectRowByLabel(container, "Tool profile"); + expect([...thinkingRow.querySelectorAll("button")].every((button) => button.disabled)).toBe( + true, + ); + expect([...fastModeRow.querySelectorAll("button")].every((button) => button.disabled)).toBe( + true, + ); + expect(browserRow.querySelector("input")?.hasAttribute("disabled")).toBe(true); + expect( + [...toolProfileRow.querySelectorAll("button")].every((button) => button.disabled), + ).toBe(true); + expect( + [...container.querySelectorAll(".qs-pending__actions button")].every( + (button) => button.disabled, + ), + ).toBe(true); + } + }); + it("disables commit actions until the config is ready", () => { const container = document.createElement("div"); diff --git a/ui/src/pages/config/quick.ts b/ui/src/pages/config/quick.ts index db13f1c3f62a..5f7464f9293d 100644 --- a/ui/src/pages/config/quick.ts +++ b/ui/src/pages/config/quick.ts @@ -109,8 +109,10 @@ export type QuickSettingsProps = { // Pending config changes configDirty?: boolean; + configLoading?: boolean; configSaving?: boolean; configApplying?: boolean; + configUpdating?: boolean; configReady?: boolean; onResetConfig?: () => void; onSaveConfig?: () => void; @@ -370,8 +372,18 @@ function renderGeneralCard(props: QuickSettingsProps) { `; } +function isConfigBusy(props: QuickSettingsProps): boolean { + return ( + props.configLoading === true || + props.configSaving === true || + props.configApplying === true || + props.configUpdating === true + ); +} + function renderModelCard(props: QuickSettingsProps) { const fastMode = formatFastModeValue(props.fastMode); + const configBusy = isConfigBusy(props); return html`
${renderCardHeader(icons.brain, t("quickSettings.model.title"))} @@ -392,6 +404,7 @@ function renderModelCard(props: QuickSettingsProps) { class="qs-segmented__btn ${level === props.thinkingLevel ? "qs-segmented__btn--active" : ""}" + ?disabled=${configBusy} @click=${() => props.onThinkingChange?.(level)} > ${t(`quickSettings.model.thinkingLevels.${level}`)} @@ -413,6 +426,7 @@ function renderModelCard(props: QuickSettingsProps) { ([value, labelKey]) => html` -