mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 05:16:23 -06:00
fix(ui): prevent configuration edits from disappearing during operations (#105555)
* fix(ui): lock configuration during operations * chore: defer release note to release workflow
This commit is contained in:
committed by
GitHub
parent
2ebf0422db
commit
d3fdd214c2
@@ -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<RequestFn>().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();
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -308,6 +308,51 @@ describe("loadConfig", () => {
|
||||
});
|
||||
|
||||
describe("createRuntimeConfigCapability", () => {
|
||||
it("publishes the pending save state before the request settles", async () => {
|
||||
const pendingSave = deferred<void>();
|
||||
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<ConfigSnapshot>();
|
||||
const secondConfig = deferred<ConfigSnapshot>();
|
||||
|
||||
@@ -839,7 +839,11 @@ export function createRuntimeConfigCapability(
|
||||
};
|
||||
const run = async <T>(task: () => Promise<T>): Promise<T> => {
|
||||
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();
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>) {
|
||||
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(),
|
||||
|
||||
@@ -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<HTMLButtonElement>(".qs-pending__actions button")].every(
|
||||
(button) => button.disabled,
|
||||
),
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("disables commit actions until the config is ready", () => {
|
||||
const container = document.createElement("div");
|
||||
|
||||
|
||||
@@ -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`
|
||||
<div class="qs-card qs-card--model">
|
||||
${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`
|
||||
<button
|
||||
class="qs-segmented__btn ${fastMode === value ? "qs-segmented__btn--active" : ""}"
|
||||
?disabled=${configBusy}
|
||||
@click=${() =>
|
||||
fastMode === value
|
||||
? undefined
|
||||
@@ -526,6 +540,7 @@ function renderSecurityCard(props: QuickSettingsProps) {
|
||||
const toolProfiles = TOOL_PROFILES.includes(normalizedToolProfile)
|
||||
? TOOL_PROFILES
|
||||
: [...TOOL_PROFILES, normalizedToolProfile];
|
||||
const configBusy = isConfigBusy(props);
|
||||
|
||||
return html`
|
||||
<div class="qs-card qs-card--security">
|
||||
@@ -555,6 +570,7 @@ function renderSecurityCard(props: QuickSettingsProps) {
|
||||
<input
|
||||
type="checkbox"
|
||||
.checked=${browserEnabled}
|
||||
?disabled=${configBusy}
|
||||
@change=${(event: Event) =>
|
||||
props.onBrowserEnabledToggle?.((event.currentTarget as HTMLInputElement).checked)}
|
||||
/>
|
||||
@@ -574,6 +590,7 @@ function renderSecurityCard(props: QuickSettingsProps) {
|
||||
normalizedToolProfile
|
||||
? "qs-segmented__btn--active"
|
||||
: ""}"
|
||||
?disabled=${configBusy}
|
||||
@click=${() => props.onToolProfileChange?.(profile)}
|
||||
>
|
||||
${profile}
|
||||
@@ -1103,11 +1120,8 @@ function renderPendingChangesBar(props: QuickSettingsProps) {
|
||||
if (props.configDirty !== true) {
|
||||
return nothing;
|
||||
}
|
||||
const canCommit =
|
||||
props.connected &&
|
||||
props.configReady === true &&
|
||||
props.configSaving !== true &&
|
||||
props.configApplying !== true;
|
||||
const configBusy = isConfigBusy(props);
|
||||
const canCommit = props.connected && props.configReady === true && !configBusy;
|
||||
|
||||
return html`
|
||||
<div class="qs-card qs-card--span-all qs-pending" aria-live="polite">
|
||||
@@ -1116,11 +1130,7 @@ function renderPendingChangesBar(props: QuickSettingsProps) {
|
||||
<span class="qs-pending__hint muted">${t("quickSettings.pending.hint")}</span>
|
||||
</div>
|
||||
<div class="qs-pending__actions">
|
||||
<button
|
||||
class="btn btn--sm"
|
||||
?disabled=${props.configSaving === true || props.configApplying === true}
|
||||
@click=${props.onResetConfig}
|
||||
>
|
||||
<button class="btn btn--sm" ?disabled=${configBusy} @click=${props.onResetConfig}>
|
||||
${t("quickSettings.pending.discard")}
|
||||
</button>
|
||||
<button class="btn btn--sm primary" ?disabled=${!canCommit} @click=${props.onSaveConfig}>
|
||||
|
||||
@@ -239,7 +239,7 @@ describe("config view", () => {
|
||||
expect(onReset).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("renders inline progress inside busy action buttons without locking adjacent controls", () => {
|
||||
it("locks config editors and adjacent actions while a config operation is pending", () => {
|
||||
const container = document.createElement("div");
|
||||
const renderCase = (overrides: Partial<ConfigProps>) =>
|
||||
render(
|
||||
@@ -266,8 +266,9 @@ describe("config view", () => {
|
||||
expect(busyButton.disabled).toBe(true);
|
||||
expect(busyButton.getAttribute("aria-busy")).toBe("true");
|
||||
expect(busyButton.querySelectorAll(".config-action-spinner")).toHaveLength(1);
|
||||
expect(clearButton.disabled).toBe(false);
|
||||
expect(applyButton.disabled).toBe(false);
|
||||
expect(clearButton.disabled).toBe(true);
|
||||
expect(applyButton.disabled).toBe(true);
|
||||
expect(container.querySelector(".config-content input")?.hasAttribute("disabled")).toBe(true);
|
||||
|
||||
renderCase({ applying: true });
|
||||
busyButton = findButtonContainingText(container, "Applying…");
|
||||
@@ -275,7 +276,7 @@ describe("config view", () => {
|
||||
clearButton = requireActionButton(actionButtons.clearButton, "Clear");
|
||||
expect(busyButton.disabled).toBe(true);
|
||||
expect(busyButton.querySelectorAll(".config-action-spinner")).toHaveLength(1);
|
||||
expect(clearButton.disabled).toBe(false);
|
||||
expect(clearButton.disabled).toBe(true);
|
||||
|
||||
renderCase({ updating: true });
|
||||
busyButton = findButtonContainingText(container, "Updating…");
|
||||
@@ -283,7 +284,17 @@ describe("config view", () => {
|
||||
clearButton = requireActionButton(actionButtons.clearButton, "Clear");
|
||||
expect(busyButton.disabled).toBe(true);
|
||||
expect(busyButton.querySelectorAll(".config-action-spinner")).toHaveLength(1);
|
||||
expect(clearButton.disabled).toBe(false);
|
||||
expect(clearButton.disabled).toBe(true);
|
||||
|
||||
renderCase({
|
||||
formMode: "raw",
|
||||
raw: '{\n gateway: { mode: "remote" }\n}\n',
|
||||
originalRaw: '{\n gateway: { mode: "local" }\n}\n',
|
||||
saving: true,
|
||||
});
|
||||
const rawEditor = container.querySelector(".config-raw-field textarea");
|
||||
expect(rawEditor).toBeInstanceOf(HTMLTextAreaElement);
|
||||
expect(rawEditor?.hasAttribute("disabled")).toBe(true);
|
||||
});
|
||||
|
||||
it("switches mode via the sidebar toggle", () => {
|
||||
|
||||
+12
-10
@@ -1456,19 +1456,16 @@ export function renderConfig(props: ConfigProps) {
|
||||
? computeRawDiff(viewState, props.originalRaw, props.raw)
|
||||
: [];
|
||||
const hasChanges = formMode === "form" ? diff.length > 0 : hasRawChanges;
|
||||
const configBusy = props.loading || props.saving || props.applying || props.updating;
|
||||
|
||||
// Save/apply buttons require actual changes to be enabled.
|
||||
// Note: formUnsafe warns about unsupported schema paths but shouldn't block saving.
|
||||
const canSaveForm = Boolean(props.formValue) && !props.loading && Boolean(analysis.schema);
|
||||
const canSave =
|
||||
props.connected && !props.saving && hasChanges && (formMode === "raw" ? true : canSaveForm);
|
||||
props.connected && !configBusy && hasChanges && (formMode === "raw" ? true : canSaveForm);
|
||||
const canApply =
|
||||
props.connected &&
|
||||
!props.applying &&
|
||||
!props.updating &&
|
||||
hasChanges &&
|
||||
(formMode === "raw" ? true : canSaveForm);
|
||||
const canUpdate = props.connected && !props.applying && !props.updating;
|
||||
props.connected && !configBusy && hasChanges && (formMode === "raw" ? true : canSaveForm);
|
||||
const canUpdate = props.connected && !configBusy;
|
||||
const renderActionButtonContent = (busy: boolean, label: string, busyLabel: string) =>
|
||||
busy
|
||||
? html`<span class="config-action-spinner" aria-hidden="true">${icons.loader}</span
|
||||
@@ -1541,10 +1538,14 @@ export function renderConfig(props: ConfigProps) {
|
||||
</button>
|
||||
`
|
||||
: nothing}
|
||||
<button class="btn btn--sm" ?disabled=${props.loading} @click=${props.onReload}>
|
||||
<button class="btn btn--sm" ?disabled=${configBusy} @click=${props.onReload}>
|
||||
${props.loading ? t("common.loading") : t("common.reload")}
|
||||
</button>
|
||||
<button class="btn btn--sm" ?disabled=${!hasChanges} @click=${props.onReset}>
|
||||
<button
|
||||
class="btn btn--sm"
|
||||
?disabled=${configBusy || !hasChanges}
|
||||
@click=${props.onReset}
|
||||
>
|
||||
${t("configView.clear")}
|
||||
</button>
|
||||
<button
|
||||
@@ -1869,7 +1870,7 @@ export function renderConfig(props: ConfigProps) {
|
||||
uiHints: props.uiHints,
|
||||
value: props.formValue,
|
||||
rawAvailable,
|
||||
disabled: props.loading || !props.formValue,
|
||||
disabled: configBusy || !props.formValue,
|
||||
unsupportedPaths: analysis.unsupportedPaths,
|
||||
onPatch: props.onFormPatch,
|
||||
searchQuery: props.searchQuery,
|
||||
@@ -1953,6 +1954,7 @@ export function renderConfig(props: ConfigProps) {
|
||||
<textarea
|
||||
placeholder=${t("configView.rawConfig")}
|
||||
.value=${props.raw}
|
||||
?disabled=${configBusy}
|
||||
@input=${(e: Event) => {
|
||||
props.onRawChange((e.target as HTMLTextAreaElement).value);
|
||||
}}
|
||||
|
||||
Reference in New Issue
Block a user