diff --git a/CHANGELOG.md b/CHANGELOG.md index bc07b3025b95..ea370c808c48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,7 @@ Docs: https://docs.openclaw.ai - **Android settings:** keep form fields and actions reachable above the keyboard, and respect bottom system insets without duplicating navigation padding. - **Nextcloud Talk diagnostics:** redact reflected credentials before displaying send, reaction, and bot-preflight errors, and suppress incomplete error bodies. (#119976) Thanks @xialonglee. +- **Control UI config drafts:** preserve external changes and newer Form or Raw edits across reconnects, including saves whose acknowledgments were lost, by keeping the draft's original document and write revision together. - Codex image attachments: decode mixed-case `file://` URLs as local image paths while preserving existing file URL validation and platform behavior. (#121611) Thanks @sunlit-deng. - **Android gateway discovery:** resolve nearby gateways one at a time on Android 12 and 13 so simultaneously advertised gateways are not silently omitted. diff --git a/ui/src/e2e/config-safe-write.e2e.test.ts b/ui/src/e2e/config-safe-write.e2e.test.ts index aefbbb73ea5a..250304e42c51 100644 --- a/ui/src/e2e/config-safe-write.e2e.test.ts +++ b/ui/src/e2e/config-safe-write.e2e.test.ts @@ -139,6 +139,149 @@ async function capture(page: Page, name: string): Promise { } suite.define(() => { + it("retains a Raw revert when an autosave commits after its connection closes", async () => { + await suite.withPage( + { + colorScheme: "dark", + locale: "en-US", + recordVideo: captureUiProofEnabled + ? { dir: uiProofArtifactDir, size: { height: 1000, width: 1440 } } + : undefined, + serviceWorkers: "block", + viewport: { height: 1000, width: 1440 }, + }, + async ({ page }) => { + const initialConfig = { + laboratory: { endpoint: "original-api", retryBudget: 2 }, + tools: {}, + }; + const gateway = await installMockGateway(page, { + methodResponses: { + "config.get": configResponse(initialConfig, "revision-original"), + "config.schema": configSchemaResponse(), + }, + }); + await page.goto(`${suite.server.baseUrl}settings/advanced?section=laboratory`); + const endpoint = page.getByRole("textbox", { name: "Endpoint", exact: true }); + await expect.poll(() => endpoint.inputValue()).toBe("original-api"); + await page.getByRole("button", { name: "Raw", exact: true }).click(); + const raw = page.locator(".config-raw-field textarea"); + const originalRaw = await raw.inputValue(); + await page.getByRole("button", { name: "Form", exact: true }).click(); + + await gateway.deferNext("config.set"); + await endpoint.fill("committed-api"); + const submitted = mutationParams(await gateway.waitForRequest("config.set")); + expect(JSON.parse(String(submitted.raw))).toMatchObject({ + laboratory: { endpoint: "committed-api" }, + }); + await page.getByRole("button", { name: "Raw", exact: true }).click(); + await raw.fill(originalRaw); + await capture(page, "14-lost-ack-raw-revert.png"); + + const getsBeforeReconnect = (await gateway.getRequests("config.get")).length; + await gateway.setOnline(false); + // Commit through the stateful mock after closing the socket: its late + // acknowledgment is dropped, but reconnect must observe the saved revision. + await gateway.resolveDeferred("config.set"); + await gateway.setOnline(true); + await expect + .poll(async () => (await gateway.getRequests("config.get")).length) + .toBe(getsBeforeReconnect + 1); + await expect.poll(() => raw.isEnabled()).toBe(true); + await capture(page, "15-lost-ack-retained-draft.png"); + expect(await raw.inputValue()).toBe(originalRaw); + expect(await gateway.getRequests("config.set")).toHaveLength(1); + + const saveButton = page.getByRole("button", { name: "Save", exact: true }); + await expect.poll(() => saveButton.isEnabled()).toBe(true); + await gateway.deferNext("config.set"); + await saveButton.click(); + const saved = mutationParams(await gateway.waitForRequest("config.set", { after: 1 })); + expect(saved.baseHash).toBe("mock-config-hash-1"); + expect(saved.raw).toBe(originalRaw); + await gateway.resolveDeferred("config.set"); + await expect.poll(() => saveButton.isEnabled()).toBe(false); + await expect + .poll(() => page.getByRole("button", { name: "Apply changes", exact: true }).count()) + .toBe(1); + await page.reload(); + await expect.poll(() => endpoint.inputValue()).toBe("original-api"); + await capture(page, "16-lost-ack-explicit-save-reload.png"); + }, + ); + }); + + it("preserves a refreshed external edit after reverting a retained Raw draft", async () => { + await suite.withPage( + { + colorScheme: "dark", + locale: "en-US", + recordVideo: captureUiProofEnabled + ? { dir: uiProofArtifactDir, size: { height: 1000, width: 1440 } } + : undefined, + serviceWorkers: "block", + viewport: { height: 1000, width: 1440 }, + }, + async ({ page }) => { + const initialConfig = { + laboratory: { endpoint: "original-api", retryBudget: 2 }, + tools: {}, + }; + const gateway = await installMockGateway(page, { + methodResponses: { + "config.get": configResponse(initialConfig, "revision-original"), + "config.schema": configSchemaResponse(), + }, + }); + await page.goto(`${suite.server.baseUrl}settings/advanced?section=laboratory`); + const endpoint = page.getByRole("textbox", { name: "Endpoint", exact: true }); + await expect.poll(() => endpoint.inputValue()).toBe("original-api"); + await page.getByRole("button", { name: "Raw", exact: true }).click(); + const raw = page.locator(".config-raw-field textarea"); + const originalRaw = await raw.inputValue(); + await raw.fill(originalRaw.replace("original-api", "unsaved-api")); + + const getsBeforeReconnect = (await gateway.getRequests("config.get")).length; + await gateway.setMethodResponse( + "config.get", + configResponse( + { ...initialConfig, laboratory: { endpoint: "external-api", retryBudget: 2 } }, + "revision-external", + ), + ); + await gateway.setOnline(false); + await gateway.setOnline(true); + await expect + .poll(async () => (await gateway.getRequests("config.get")).length) + .toBe(getsBeforeReconnect + 1); + await expect.poll(() => raw.isEnabled()).toBe(true); + await raw.fill(originalRaw); + await page.getByRole("button", { name: "Form", exact: true }).click(); + await endpoint.waitFor(); + await capture(page, "12-reconnect-raw-revert.png"); + expect.soft(await endpoint.inputValue()).toBe("external-api"); + + await gateway.deferNext("config.set"); + await page.getByRole("spinbutton", { name: "Retry budget", exact: true }).fill("3"); + const save = mutationParams(await gateway.waitForRequest("config.set")); + expect(save.baseHash).toBe("revision-external"); + expect.soft(JSON.parse(String(save.raw))).toEqual({ + ...initialConfig, + laboratory: { endpoint: "external-api", retryBudget: 3 }, + }); + await gateway.resolveDeferred("config.set"); + await expect + .poll(() => page.locator("openclaw-settings-save-indicator").textContent()) + .toContain("Saved"); + await page.reload(); + await endpoint.waitFor(); + await capture(page, "13-reconnect-save-reload.png"); + expect(await endpoint.inputValue()).toBe("external-api"); + }, + ); + }); + it("restores form values when a failed edit is reverted in Raw mode", async () => { await suite.withPage( { diff --git a/ui/src/lib/config/config-draft-model.ts b/ui/src/lib/config/config-draft-model.ts index bc042871bd52..96de16995a76 100644 --- a/ui/src/lib/config/config-draft-model.ts +++ b/ui/src/lib/config/config-draft-model.ts @@ -141,31 +141,15 @@ export function applyConfigSnapshot( if (!rawAvailable && state.configFormMode === "raw") { state.configFormMode = "form"; } - const rawFromSnapshot: string = - typeof snapshot.raw === "string" - ? snapshot.raw - : editableConfig - ? serializeConfigForm(editableConfig) - : state.configRaw; - if (!preservePendingChanges) { - state.configRaw = rawFromSnapshot; - } else if (state.configFormMode !== "raw" && state.configForm) { - state.configRaw = serializeConfigForm(state.configForm); - } else if (state.configFormMode !== "raw") { - state.configRaw = rawFromSnapshot; - } state.configValid = typeof snapshot.valid === "boolean" ? snapshot.valid : null; state.configIssues = Array.isArray(snapshot.issues) ? snapshot.issues : []; if (!preservePendingChanges) { - state.configForm = cloneConfigObject(editableConfig ?? {}); - state.configFormOriginal = cloneConfigObject(editableConfig ?? {}); - setConfigRawOriginal(state, rawFromSnapshot); - state.configFormDirty = false; - state.configFormMode = "form"; - state.configDraftBaseHash = snapshot.hash ?? null; - autoAllowlistedPluginIdsByState.delete(state); + resetConfigPendingChanges(state); } else { + if (state.configFormMode !== "raw" && state.configForm) { + state.configRaw = serializeConfigForm(state.configForm); + } state.configDraftBaseHash = draftBaseHash; } } @@ -599,15 +583,24 @@ export function updateConfigRawValue(state: RuntimeConfigState, value: string) { resetStaleAutoSaveStatus(state); } -export function resetConfigPendingChanges(state: RuntimeConfigState) { +export function rebaseConfigDraft(state: RuntimeConfigState) { const editableConfig = resolveEditableSnapshotConfig(state.configSnapshot); - state.configForm = cloneConfigObject(state.configFormOriginal ?? editableConfig ?? {}); - state.configRaw = - state.configRawOriginal ?? - serializeConfigForm(state.configFormOriginal ?? editableConfig ?? {}); + // A retained draft can predate a reconnect snapshot. Adopt its document and + // revision together; pairing old originals with the new hash bypasses CAS. + state.configFormOriginal = cloneConfigObject(editableConfig ?? {}); + const raw = + state.configSnapshot?.raw ?? + (editableConfig ? serializeConfigForm(editableConfig) : state.configRawOriginal); + setConfigRawOriginal(state, raw); + state.configDraftBaseHash = state.configSnapshot?.hash ?? null; +} + +export function resetConfigPendingChanges(state: RuntimeConfigState) { + rebaseConfigDraft(state); + state.configForm = cloneConfigObject(state.configFormOriginal ?? {}); + state.configRaw = state.configRawOriginal; state.configFormDirty = false; state.configFormMode = "form"; - state.configDraftBaseHash = state.configSnapshot?.hash ?? null; autoAllowlistedPluginIdsByState.delete(state); } diff --git a/ui/src/lib/config/config-draft-revision.test.ts b/ui/src/lib/config/config-draft-revision.test.ts new file mode 100644 index 000000000000..6f237eb98896 --- /dev/null +++ b/ui/src/lib/config/config-draft-revision.test.ts @@ -0,0 +1,128 @@ +// @vitest-environment node +import { describe, expect, it, vi } from "vitest"; +import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import { + createConfigCapabilityHarness, + createConfigServerMock, + CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS, +} from "./config-test-harness.ts"; + +describe("config draft revision ownership", () => { + it.each([ + { start: "raw", revert: "raw", next: "form", saved: true }, + { start: "raw", revert: "raw", next: "raw", saved: true }, + { start: "form", revert: "raw", next: "form", saved: true }, + { start: "raw", revert: "reset", next: "form", saved: true }, + { start: "raw", revert: "discard", next: "form", saved: true }, + { start: "raw", revert: "raw", next: "none", saved: true }, + { start: "raw", revert: "none", next: "form", saved: false }, + { start: "form", revert: "form", next: "form", saved: false }, + ] as const)( + "preserves external changes after $start editing, $revert revert and $next editing", + async ({ start, revert, next, saved }) => { + vi.useFakeTimers(); + let storedRaw = '{"count":1,"note":"original"}\n'; + let hash = "revision-original"; + const request = vi.fn(async (method: string, params?: unknown) => { + if (method === "config.get") { + return { config: JSON.parse(storedRaw), raw: storedRaw, hash, valid: true, issues: [] }; + } + if (method === "config.set") { + const submission = params as { raw: string; baseHash: string }; + if (submission.baseHash !== hash) { + throw new Error("config changed since last load; re-run config.get and retry"); + } + storedRaw = submission.raw; + hash = "revision-saved"; + return { hash }; + } + return {}; + }); + const { runtimeConfig, publish } = createConfigCapabilityHarness( + request as GatewayBrowserClient["request"], + ); + try { + await runtimeConfig.ensureLoaded(); + const originalRaw = runtimeConfig.state.configRaw; + if (start === "raw") { + runtimeConfig.setRaw(originalRaw.replace('"count":1', '"count":2')); + } else { + runtimeConfig.patchForm(["count"], 2); + } + storedRaw = '{"count":1,"note":"external"}\n'; + hash = "revision-external"; + publish(false); + publish(true); + await vi.advanceTimersByTimeAsync(0); + expect(runtimeConfig.state.configSnapshot?.hash).toBe("revision-external"); + expect(runtimeConfig.state.configDraftBaseHash).toBe("revision-original"); + + if (revert === "raw") { + runtimeConfig.setRaw(originalRaw); + } else if (revert === "form") { + runtimeConfig.patchForm(["count"], 1); + } else if (revert === "reset") { + runtimeConfig.resetDraft(); + } else if (revert === "discard") { + await runtimeConfig.discardDraft(); + } + if (next === "form") { + runtimeConfig.patchForm(["count"], 3); + } else if (next === "raw") { + runtimeConfig.setRaw(runtimeConfig.state.configRaw.replace('"count":1', '"count":3')); + } + + await expect(runtimeConfig.save()).resolves.toBe(saved); + expect(JSON.parse(storedRaw)).toEqual({ + count: saved && next !== "none" ? 3 : 1, + note: "external", + }); + expect(runtimeConfig.state.configAutoSaveStatus).toBe(saved ? "saved" : "conflict"); + expect(request.mock.calls.filter(([method]) => method === "config.set")).toHaveLength(1); + } finally { + runtimeConfig.setWritesSuspended(true); + runtimeConfig.dispose(); + } + }, + ); + + it("keeps the restart-needed banner when reverting an acknowledged autosave draft", async () => { + vi.useFakeTimers(); + const server = createConfigServerMock(); + const { runtimeConfig } = createConfigCapabilityHarness( + server.request as GatewayBrowserClient["request"], + ); + try { + await runtimeConfig.ensureLoaded(); + runtimeConfig.patchForm(["count"], 2); + await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS); + expect(runtimeConfig.state.configNeedsApply).toBe(true); + runtimeConfig.setRaw('{"count":3}'); + runtimeConfig.setRaw(runtimeConfig.state.configRawOriginal); + expect(runtimeConfig.state.configNeedsApply).toBe(true); + expect(runtimeConfig.state.configForm).toEqual({ count: 2 }); + expect(runtimeConfig.state.configFormDirty).toBe(false); + } finally { + runtimeConfig.dispose(); + } + }); + + it("keeps a failed first load retryable after a raw draft is reverted", async () => { + const server = createConfigServerMock(); + server.request.mockRejectedValueOnce(new Error("unavailable")); + const { runtimeConfig } = createConfigCapabilityHarness( + server.request as GatewayBrowserClient["request"], + ); + try { + await runtimeConfig.ensureLoaded(); + runtimeConfig.setRaw('{"count":9}'); + runtimeConfig.setRaw(runtimeConfig.state.configRawOriginal); + expect(runtimeConfig.state.configSnapshot).toBeNull(); + expect(runtimeConfig.state.configFormDirty).toBe(false); + await runtimeConfig.ensureLoaded(); + expect(runtimeConfig.state.configForm).toEqual({ count: 1 }); + } finally { + runtimeConfig.dispose(); + } + }); +}); diff --git a/ui/src/lib/config/config-gateway-operations.ts b/ui/src/lib/config/config-gateway-operations.ts index cf2ff109fa9a..615a25773f76 100644 --- a/ui/src/lib/config/config-gateway-operations.ts +++ b/ui/src/lib/config/config-gateway-operations.ts @@ -224,7 +224,7 @@ export async function executeConfigExternalMutation( export async function loadConfig( state: RuntimeConfigState, - options: LoadConfigOptions & { background?: boolean } = {}, + options: LoadConfigOptions & { background?: boolean; beforeApplySnapshot?: () => void } = {}, isCurrentLoad: () => boolean = () => true, ): Promise { const client = state.client; @@ -243,6 +243,8 @@ export async function loadConfig( if (!isCurrentRequest(state, "config", version, client, connectionEpoch) || !isCurrentLoad()) { return false; } + // Recovery captures the latest intent before a clean draft is replaced. + options.beforeApplySnapshot?.(); applyConfigSnapshot(state, res, options); return true; } catch (err) { diff --git a/ui/src/lib/config/config-write-coordinator.ts b/ui/src/lib/config/config-write-coordinator.ts index 201cae891464..b4b841a69ce3 100644 --- a/ui/src/lib/config/config-write-coordinator.ts +++ b/ui/src/lib/config/config-write-coordinator.ts @@ -1,9 +1,9 @@ import { asNullableRecord as asConfigRecord } from "@openclaw/normalization-core/record-coerce"; import type { GatewayBrowserClient } from "../../api/gateway.ts"; import { t } from "../../i18n/index.ts"; -import { cloneConfigObject, serializeConfigForm } from "../config-form-utils.ts"; +import { cloneConfigObject } from "../config-form-utils.ts"; import { - applyConfigSnapshot, + rebaseConfigDraft, removeConfigFormValue, resetConfigPendingChanges, serializeFormForSubmit, @@ -49,7 +49,7 @@ type ConfigWriteCoordinatorContext = { trackLoad: (key: "config" | "schema", promise: Promise) => Promise; resetLoads: () => void; resetConfigLoad: () => void; - refreshConnectionState: () => Promise; + refreshConnectionState: (beforeApplySnapshot?: () => void) => Promise; canCallConfigMethod: ( method: ConfigMethod, options?: { requireAdvertisement?: boolean }, @@ -127,9 +127,19 @@ export function createConfigWriteCoordinator({ state.configAutoSaveStatus = "idle"; } }; + const pauseAutoSaveDraftConnection = () => { + autoSaveRequiresExplicitSubmit = true; + // Conflict outranks the reconnect latch: the snapshot is still stale. + if (state.configFormMode === "form" && state.configAutoSaveStatus !== "conflict") { + state.configAutoSaveStatus = "paused"; + } + }; const captureAutoSaveDraftConnection = () => { + if (autoSaveRequiresExplicitSubmit) { + pauseAutoSaveDraftConnection(); + return; + } if ( - autoSaveRequiresExplicitSubmit || autoSaveDraftConnection || !state.client || !state.connected || @@ -162,7 +172,7 @@ export function createConfigWriteCoordinator({ autoSaveDraftConnection.client === state.client && autoSaveDraftConnection.epoch === currentConfigConnectionEpoch(state); const reconcileAutoSaveDraftConnection = () => { - if (state.configFormDirty && state.configFormMode === "form") { + if (state.configFormDirty) { captureAutoSaveDraftConnection(); } else if (autoSaveInFlight === null && manualSubmitInFlight === null) { clearAutoSaveDraftConnection(); @@ -398,8 +408,7 @@ export function createConfigWriteCoordinator({ state.applySessionKey = snapshot.sessionKey; if (clientChanged || connectionChanged) { const draftBelongsToPreviousConnection = - state.configFormMode === "form" && - (state.configFormDirty || autoSaveInFlight !== null || manualSubmitInFlight !== null); + state.configFormDirty || autoSaveInFlight !== null || manualSubmitInFlight !== null; resetLoads(); // A dead prior-connection flight must not keep the reconnected owner's // explicit-operation FIFO waiting forever. @@ -415,11 +424,7 @@ export function createConfigWriteCoordinator({ // Save/Apply or reload before the new Gateway may receive it. The // latch must be visible: without a rendered state the form looks // normal while every subsequent edit silently never saves. - autoSaveRequiresExplicitSubmit = true; - // Conflict outranks the latch: that snapshot is stale regardless. - if (state.configAutoSaveStatus !== "conflict") { - state.configAutoSaveStatus = "paused"; - } + pauseAutoSaveDraftConnection(); } if (autoSaveInFlight !== null || manualSubmitInFlight !== null) { // The epoch guard already blocks these flights from mutating state; @@ -457,13 +462,16 @@ export function createConfigWriteCoordinator({ // A revert made while the write was in flight reads clean (the ack // never rebased the originals), so the reload below would replace // it with the committed bytes. Capture it for restoration. - const draftFormBefore = - state.configFormMode === "form" && !state.configFormDirty && state.configForm - ? cloneConfigObject(state.configForm) - : null; - const draftRawBefore = draftFormBefore ? serializeConfigForm(draftFormBefore) : null; - const reconcile = refreshConnectionState(); - void reconcile.then((loaded) => { + const captureDraft = () => ({ + form: cloneConfigObject(state.configForm ?? {}), + raw: state.configRaw, + mode: state.configFormMode, + submittedRaw: state.configFormDirty ? serializeFormForSubmit(state) : state.configRaw, + }); + let draftBefore: ReturnType | null = null; + void refreshConnectionState(() => { + draftBefore = state.configFormDirty ? null : captureDraft(); + }).then((loaded) => { if (isDisposed()) { return; } @@ -482,36 +490,23 @@ export function createConfigWriteCoordinator({ // other server content keeps the old base and conflicts instead // of clobbering a foreign writer. if (interruptedRaw !== null && state.configSnapshot?.raw === interruptedRaw) { - const freshHash = state.configSnapshot.hash ?? null; if (state.configSnapshot.appliedConfigHash === undefined) { state.configNeedsApply = true; } - if (state.configFormDirty) { - if (serializeFormForSubmit(state) === interruptedRaw) { - // The lost acknowledgement was the only missing event: the - // retained bytes are already authoritative, so no draft - // remains to submit on the replacement connection. - applyConfigSnapshot(state, state.configSnapshot, { - discardPendingChanges: true, - }); - clearAutoSaveDraftConnection(); - } else { - state.configDraftBaseHash = freshHash ?? state.configDraftBaseHash; - } - } else if ( - draftFormBefore && - draftRawBefore !== null && - draftRawBefore !== interruptedRaw - ) { - // Reverted-while-in-flight: the clean-looking pre-reload - // draft differs from the committed bytes, so it was a real - // revert. Restore it as a dirty draft on the fresh base so - // the rescheduled autosave writes it back. - state.configForm = draftFormBefore; - state.configRaw = draftRawBefore; - state.configFormMode = "form"; + const pendingDraft = state.configFormDirty ? captureDraft() : draftBefore; + // Rebase originals and hash together, then retain newer edits or + // a pre-ack revert. Raw bytes and mode stay manual-save-only. + if (pendingDraft && pendingDraft.submittedRaw !== interruptedRaw) { + rebaseConfigDraft(state); + state.configForm = pendingDraft.form; + state.configRaw = pendingDraft.raw; + state.configFormMode = pendingDraft.mode; state.configFormDirty = true; - state.configDraftBaseHash = freshHash ?? state.configDraftBaseHash; + pauseAutoSaveDraftConnection(); + } else { + resetConfigPendingChanges(state); + state.configAutoSaveStatus = "idle"; + clearAutoSaveDraftConnection(); } } publish(); @@ -573,19 +568,16 @@ export function createConfigWriteCoordinator({ scheduleAutoSave(); }); }; + const mutateDraft = (mutation: () => void) => { + mutate(mutation); + reconcileAutoSaveDraftConnection(); + scheduleAutoSave(); + }; return { prepareDiscard: drainWritesForDiscard, - patchForm: (path, value) => { - mutate(() => updateConfigFormValue(state, path, value)); - reconcileAutoSaveDraftConnection(); - scheduleAutoSave(); - }, - removeFormValue: (path) => { - mutate(() => removeConfigFormValue(state, path)); - reconcileAutoSaveDraftConnection(); - scheduleAutoSave(); - }, - setRaw: (value) => mutate(() => updateConfigRawValue(state, value)), + patchForm: (path, value) => mutateDraft(() => updateConfigFormValue(state, path, value)), + removeFormValue: (path) => mutateDraft(() => removeConfigFormValue(state, path)), + setRaw: (value) => mutateDraft(() => updateConfigRawValue(state, value)), resetDraft: () => { cancelScheduledAutoSave(); mutate(() => resetConfigPendingChanges(state)); diff --git a/ui/src/lib/config/config-write-recovery.test.ts b/ui/src/lib/config/config-write-recovery.test.ts new file mode 100644 index 000000000000..b5c0007b897e --- /dev/null +++ b/ui/src/lib/config/config-write-recovery.test.ts @@ -0,0 +1,328 @@ +// @vitest-environment node +import { describe, expect, it, vi } from "vitest"; +import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import { + CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS, + createConfigCapabilityHarness, + createConfigServerMock, + deferred, +} from "./config-test-harness.ts"; + +const originalRaw = '{ "tools": { "exec": { "node": "original" } } }\n'; +const nodePath = ["tools", "exec", "node"]; +const nodeConfig = (node: string) => ({ tools: { exec: { node } } }); +const rawForNode = (node: string) => `${JSON.stringify(nodeConfig(node), null, 2)}\n`; + +function createRecoveryHarness( + outcome: "own" | "uncommitted" | "foreign" = "own", + initialRaw = originalRaw, +) { + let storedRaw = initialRaw; + let hash = "before"; + let getCount = 0; + const firstAck = deferred(); + const recoveryRead = deferred(); + const submissions: Array<{ raw: string; baseHash: string }> = []; + const request = vi.fn(async (method: string, params?: unknown) => { + if (method === "config.get") { + const config = JSON.parse(storedRaw) as Record; + const snapshot = { + config, + sourceConfig: config, + raw: storedRaw, + hash, + configRevisionHash: hash, + appliedConfigHash: "before", + valid: true, + issues: [], + }; + if (++getCount === 2) { + await recoveryRead.promise; + } + return snapshot; + } + if (method !== "config.set") { + return {}; + } + const submission = params as { raw: string; baseHash: string }; + submissions.push(submission); + if (submission.baseHash !== hash) { + throw new Error("config changed since last load; re-run config.get and retry"); + } + if (submissions.length === 1) { + if (outcome !== "uncommitted") { + storedRaw = submission.raw; + hash = "own-commit"; + } + return firstAck.promise; + } + storedRaw = submission.raw; + hash = "explicit-save"; + return { hash }; + }); + const { runtimeConfig, publish } = createConfigCapabilityHarness( + request as GatewayBrowserClient["request"], + ); + return { + runtimeConfig, + submissions, + get storedRaw() { + return storedRaw; + }, + async start(edit = () => runtimeConfig.patchForm(nodePath, "submitted")) { + await runtimeConfig.ensureLoaded(); + edit(); + await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS); + expect(submissions).toHaveLength(1); + expect(submissions[0]?.baseHash).toBe("before"); + }, + async reconnect(duringLoad?: () => void) { + if (outcome === "foreign") { + storedRaw = rawForNode("foreign"); + hash = "foreign"; + } + // The transport rejects pending requests before publishing socket close. + firstAck.reject(new Error("socket closed")); + publish(false); + publish(true); + expect(getCount).toBe(2); + expect(runtimeConfig.state.configLoading).toBe(true); + duringLoad?.(); + recoveryRead.resolve(); + await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS * 2); + expect(runtimeConfig.state.configLoading).toBe(false); + expect(submissions).toHaveLength(1); + }, + dispose() { + runtimeConfig.setWritesSuspended(true); + runtimeConfig.dispose(); + recoveryRead.resolve(); + firstAck.resolve({}); + }, + }; +} + +describe("config write recovery", () => { + it("retains pending plugin allowlist ownership without removing authored entries", async () => { + vi.useFakeTimers(); + const harness = createRecoveryHarness( + "own", + JSON.stringify({ plugins: { allow: ["authored"], entries: {} } }), + ); + const { runtimeConfig, submissions } = harness; + try { + await harness.start(() => + runtimeConfig.patchForm(["plugins", "entries", "first", "enabled"], true), + ); + runtimeConfig.patchForm(["plugins", "entries", "pending", "enabled"], true); + await harness.reconnect(); + runtimeConfig.patchForm(["plugins", "entries", "pending", "enabled"], false); + runtimeConfig.patchForm(["plugins", "entries", "authored", "enabled"], false); + expect(runtimeConfig.state.configForm?.plugins).toEqual({ + allow: ["authored", "first"], + entries: { + first: { enabled: true }, + pending: { enabled: false }, + authored: { enabled: false }, + }, + }); + await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS * 2); + expect(submissions).toHaveLength(1); + await expect(runtimeConfig.save()).resolves.toBe(true); + expect(JSON.parse(harness.storedRaw).plugins.allow).toEqual(["authored", "first"]); + expect(submissions[1]?.baseHash).toBe("own-commit"); + } finally { + harness.dispose(); + } + }); + + it.each([ + { mode: "raw", edit: "revert", next: "raw" }, + { mode: "form", edit: "revert", next: "form" }, + { mode: "raw", edit: "newer", next: "raw" }, + { mode: "form", edit: "newer", next: "form" }, + { mode: "raw", edit: "revert", next: "form" }, + { mode: "raw", edit: "newer", next: "form" }, + ] as const)( + "retains a $mode $edit before a subsequent $next edit", + async ({ mode, edit, next }) => { + vi.useFakeTimers(); + const harness = createRecoveryHarness(); + const { runtimeConfig, submissions } = harness; + try { + await harness.start(); + const node = edit === "revert" ? "original" : "newer"; + const pendingRaw = edit === "revert" ? originalRaw : `${rawForNode(node)}\n`; + if (mode === "raw") { + runtimeConfig.setRaw(pendingRaw); + } else { + runtimeConfig.patchForm(nodePath, node); + } + expect(runtimeConfig.state.configFormDirty).toBe(edit !== "revert"); + + await harness.reconnect(); + expect(runtimeConfig.state.configFormMode).toBe(mode); + expect(JSON.parse(runtimeConfig.state.configRaw)).toEqual(nodeConfig(node)); + if (mode === "raw") { + expect(runtimeConfig.state.configRaw).toBe(pendingRaw); + expect(runtimeConfig.state.configAutoSaveStatus).toBe("idle"); + } else { + expect(runtimeConfig.state.configAutoSaveStatus).toBe("paused"); + } + expect(runtimeConfig.state.configFormDirty).toBe(true); + expect(runtimeConfig.state.configRawOriginal).toBe(rawForNode("submitted")); + expect(runtimeConfig.state.configFormOriginal).toEqual(nodeConfig("submitted")); + expect(runtimeConfig.state.configDraftBaseHash).toBe("own-commit"); + expect(runtimeConfig.state.configNeedsApply).toBe(true); + + // The pre-write document is now a real edit, not a clean revert to stale originals. + if (next === "raw") { + runtimeConfig.setRaw(originalRaw); + } else { + if (mode === "raw") { + runtimeConfig.setRaw(`${pendingRaw}\n`); + } + runtimeConfig.patchForm(nodePath, "original"); + } + expect(runtimeConfig.state.configFormDirty).toBe(true); + expect(runtimeConfig.state.configAutoSaveStatus).toBe(next === "form" ? "paused" : "idle"); + await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS * 2); + expect(submissions).toHaveLength(1); + await expect(runtimeConfig.save()).resolves.toBe(true); + expect(submissions[1]).toEqual({ + raw: next === "raw" ? originalRaw : rawForNode("original"), + baseHash: "own-commit", + }); + expect(JSON.parse(harness.storedRaw)).toEqual(nodeConfig("original")); + expect(runtimeConfig.state.configFormDirty).toBe(false); + expect(runtimeConfig.state.configAutoSaveStatus).toBe("saved"); + } finally { + harness.dispose(); + } + }, + ); + + it.each([false, true])( + "keeps a raw-first draft paused after reconnect (rejected form edit: %s)", + async (rejectFormEdit) => { + vi.useFakeTimers(); + const server = createConfigServerMock(); + const { runtimeConfig, publish } = createConfigCapabilityHarness( + server.request as GatewayBrowserClient["request"], + ); + try { + await runtimeConfig.ensureLoaded(); + runtimeConfig.setRaw('{"count":2}'); + publish(false); + publish(true); + await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS * 2); + expect(runtimeConfig.state.configAutoSaveStatus).toBe("idle"); + expect(server.submissions).toHaveLength(0); + if (rejectFormEdit) { + runtimeConfig.setRaw("{"); + runtimeConfig.patchForm(["count"], 3); + expect(runtimeConfig.state.configFormMode).toBe("raw"); + expect(runtimeConfig.state.configAutoSaveStatus).toBe("error"); + expect(runtimeConfig.state.configRaw).toBe("{"); + } + runtimeConfig.setRaw('{"count":2}\n'); + runtimeConfig.patchForm(["count"], 3); + expect.soft(runtimeConfig.state.configAutoSaveStatus).toBe("paused"); + await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS * 2); + expect(server.submissions).toHaveLength(0); + await expect(runtimeConfig.save()).resolves.toBe(true); + expect(server.submissions).toEqual([ + { method: "config.set", raw: '{\n "count": 3\n}\n', baseHash: "hash-1" }, + ]); + runtimeConfig.patchForm(["count"], 4); + await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS); + expect(server.submissions[1]).toEqual({ + method: "config.set", + raw: '{\n "count": 4\n}\n', + baseHash: "hash-2", + }); + } finally { + runtimeConfig.setWritesSuspended(true); + runtimeConfig.dispose(); + } + }, + ); + + it("retains a Devices binding reverted while the recovery read is pending", async () => { + vi.useFakeTimers(); + const harness = createRecoveryHarness(); + const { runtimeConfig, submissions } = harness; + try { + await harness.start(); + runtimeConfig.patchForm(nodePath, "newer"); + await harness.reconnect(() => { + // Devices binding controls remain enabled while config.get is loading. + expect(runtimeConfig.canSet).toBe(true); + expect(runtimeConfig.state.configSaving).toBe(false); + runtimeConfig.patchForm(nodePath, "original"); + expect(runtimeConfig.state.configFormDirty).toBe(false); + }); + expect(runtimeConfig.state.configForm).toEqual(nodeConfig("original")); + expect(runtimeConfig.state.configFormDirty).toBe(true); + expect(runtimeConfig.state.configAutoSaveStatus).toBe("paused"); + expect(runtimeConfig.state.configRawOriginal).toBe(rawForNode("submitted")); + expect(runtimeConfig.state.configDraftBaseHash).toBe("own-commit"); + await expect(runtimeConfig.save()).resolves.toBe(true); + expect(submissions[1]).toEqual({ raw: rawForNode("original"), baseHash: "own-commit" }); + expect(JSON.parse(harness.storedRaw)).toEqual(nodeConfig("original")); + } finally { + harness.dispose(); + } + }); + + it.each(["raw", "form"] as const)( + "never rebases a %s draft onto a foreign write", + async (mode) => { + vi.useFakeTimers(); + const harness = createRecoveryHarness("foreign"); + const { runtimeConfig } = harness; + try { + await harness.start(); + if (mode === "raw") { + runtimeConfig.setRaw(`${rawForNode("newer")}\n`); + } else { + runtimeConfig.patchForm(nodePath, "newer"); + } + await harness.reconnect(); + expect(runtimeConfig.state.configDraftBaseHash).toBe("before"); + expect(runtimeConfig.state.configRawOriginal).toBe(originalRaw); + expect(JSON.parse(runtimeConfig.state.configRaw)).toEqual(nodeConfig("newer")); + await expect(runtimeConfig.save()).resolves.toBe(false); + expect(harness.storedRaw).toBe(rawForNode("foreign")); + expect(runtimeConfig.state.configAutoSaveStatus).toBe("conflict"); + } finally { + harness.dispose(); + } + }, + ); + + it.each(["own", "uncommitted"] as const)( + "leaves a matching %s document clean", + async (outcome) => { + vi.useFakeTimers(); + const harness = createRecoveryHarness(outcome); + const { runtimeConfig } = harness; + try { + await harness.start(); + if (outcome === "uncommitted") { + runtimeConfig.setRaw(originalRaw); + } + await harness.reconnect(); + expect(runtimeConfig.state.configFormDirty).toBe(false); + expect(runtimeConfig.state.configRaw).toBe(harness.storedRaw); + expect(runtimeConfig.state.configRawOriginal).toBe(harness.storedRaw); + expect(runtimeConfig.state.configDraftBaseHash).toBe( + outcome === "own" ? "own-commit" : "before", + ); + expect(runtimeConfig.state.configAutoSaveStatus).toBe("idle"); + } finally { + harness.dispose(); + } + }, + ); +}); diff --git a/ui/src/lib/config/runtime-config-capability.test.ts b/ui/src/lib/config/runtime-config-capability.test.ts index 627655d029aa..7a398f49876b 100644 --- a/ui/src/lib/config/runtime-config-capability.test.ts +++ b/ui/src/lib/config/runtime-config-capability.test.ts @@ -1015,66 +1015,6 @@ describe("runtime config capability", () => { runtimeConfig.dispose(); }); - it("restores a revert made while the interrupted write was in flight", async () => { - vi.useFakeTimers(); - let committedRaw = '{\n "count": 1\n}\n'; - let hash = "hash-1"; - const sets: Array<{ raw: string; baseHash: string }> = []; - const request = vi.fn((method: string, params?: unknown) => { - if (method === "config.get") { - return Promise.resolve({ - config: JSON.parse(committedRaw) as Record, - raw: committedRaw, - hash, - valid: true, - issues: [], - }); - } - if (method === "config.set") { - sets.push(params as { raw: string; baseHash: string }); - if (sets.length === 1) { - // Commits server-side; the ack is lost to the disconnect. - committedRaw = (params as { raw: string }).raw; - hash = "hash-2"; - return new Promise(() => {}); - } - committedRaw = (params as { raw: string }).raw; - hash = "hash-3"; - return Promise.resolve({ hash }); - } - return Promise.resolve({}); - }); - const { runtimeConfig, publish } = createConfigCapabilityHarness( - request as GatewayBrowserClient["request"], - ); - await runtimeConfig.ensureLoaded(); - - runtimeConfig.patchForm(["count"], 2); - await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS); - expect(sets).toHaveLength(1); - - // Revert to the original while the save is in flight: the draft reads - // clean, so a plain reconnect reload would silently replace it with the - // committed bytes and drop the revert forever. - runtimeConfig.patchForm(["count"], 1); - expect(runtimeConfig.state.configFormDirty).toBe(false); - publish(false); - publish(true); - await vi.advanceTimersByTimeAsync(CONFIG_FORM_AUTO_SAVE_DEBOUNCE_MS); - - expect(sets).toHaveLength(1); - expect(runtimeConfig.state.configFormDirty).toBe(true); - expect(runtimeConfig.state.configDraftBaseHash).toBe("hash-2"); - expect(runtimeConfig.state.configForm).toEqual({ count: 1 }); - - await expect(runtimeConfig.save()).resolves.toBe(true); - expect(sets).toHaveLength(2); - expect(sets[1]).toEqual({ raw: '{\n "count": 1\n}\n', baseHash: "hash-2" }); - expect(runtimeConfig.state.configForm).toEqual({ count: 1 }); - expect(runtimeConfig.state.configAutoSaveStatus).toBe("saved"); - runtimeConfig.dispose(); - }); - it("keeps the conflict status through an offline discard", async () => { vi.useFakeTimers(); const request = vi.fn(async (method: string) => { diff --git a/ui/src/lib/config/runtime-config-capability.ts b/ui/src/lib/config/runtime-config-capability.ts index 54a824990b6e..9bf1bba242d9 100644 --- a/ui/src/lib/config/runtime-config-capability.ts +++ b/ui/src/lib/config/runtime-config-capability.ts @@ -145,8 +145,8 @@ export function createRuntimeConfigCapability( refresh: (isCurrent) => loadOnce("config", () => loadConfig(state, { background: true }, isCurrent)), }); - const refreshConnectionState = () => { - const config = run(() => loadConfig(state)); + const refreshConnectionState = (beforeApplySnapshot?: () => void) => { + const config = run(() => loadConfig(state, { beforeApplySnapshot })); void trackLoad("config", config); if (state.configSchemaVersion !== null && canLoadConfigSchema()) { void trackLoad(