diff --git a/apps/vscode-e2e/src/visual/__screenshots__/electron-chat-dark-sidebar.png b/apps/vscode-e2e/src/visual/__screenshots__/electron-chat-dark-sidebar.png index 3791a71175..dfc6fee680 100644 Binary files a/apps/vscode-e2e/src/visual/__screenshots__/electron-chat-dark-sidebar.png and b/apps/vscode-e2e/src/visual/__screenshots__/electron-chat-dark-sidebar.png differ diff --git a/apps/vscode-e2e/src/visual/electron.visual.ts b/apps/vscode-e2e/src/visual/electron.visual.ts index 155f496980..c10ee7cb0c 100644 --- a/apps/vscode-e2e/src/visual/electron.visual.ts +++ b/apps/vscode-e2e/src/visual/electron.visual.ts @@ -256,8 +256,21 @@ for (const scenario of scenarios) { const tokenCountMask = webviewFrame .frameLocator("iframe") .locator('[data-testid="context-tokens-count"],[data-testid="context-window-size"]') + // The time under each message is the real send time, so it changes on every run, and so does the length + // of its text ("Today 9:05 AM" and "Yesterday 12:36 PM" are not equally long). A mask covers the + // element's box, so that box needs a fixed size as well: without it the masked area differs between + // runs, in width and, when the text wraps onto a second line, in height. + await contentFrame.evaluate(() => { + document.querySelectorAll('[data-testid="message-timestamp"]').forEach((time) => { + time.style.setProperty("width", "160px", "important") + time.style.setProperty("white-space", "nowrap", "important") + time.style.setProperty("overflow", "hidden", "important") + }) + }) + // Mask it, as the token counters above, and keep the rest of the scene byte-stable. + const messageTimeMask = webviewFrame.frameLocator("iframe").locator('[data-testid="message-timestamp"]') await expect(sidebar).toHaveScreenshot(`electron-${scenario.name}-sidebar.png`, { - mask: [tokenCountMask], + mask: [tokenCountMask, messageTimeMask], }) if (scenario.webviewSnapshot) { diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index eb51c1d67d..f5757a51c7 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -280,6 +280,21 @@ export const globalSettingsSchema = z.object({ * When unset (or `null`), the webview inherits VS Code's `--vscode-font-size`. */ chatFontSize: z.number().int().min(8).max(32).nullish(), + /** + * Clock format used when rendering timestamps. "auto" follows the display language's locale. + * @default "auto" + */ + timeFormat: z.enum(["auto", "12h", "24h"]).optional(), + /** + * How dates are written. "auto" follows the display language's locale. + * @default "auto" + */ + dateFormat: z.enum(["auto", "mdy", "dmy", "ymd"]).optional(), + /** + * Show relative times ("5 min ago") in task history; when off, show the absolute date/time. + * @default true + */ + showRelativeTimes: z.boolean().optional(), /** * Controls the keyboard behavior for sending messages in the chat input. * - "send": Enter sends message, Shift+Enter creates newline (default) diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index dc407325d9..0bf3348d8d 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -339,6 +339,9 @@ export type ExtensionState = Pick< | "includeTaskHistoryInEnhance" | "reasoningBlockCollapsed" | "chatFontSize" + | "timeFormat" + | "dateFormat" + | "showRelativeTimes" | "enterBehavior" | "includeCurrentTime" | "includeCurrentCost" diff --git a/src/core/config/ContextProxy.ts b/src/core/config/ContextProxy.ts index 1250a7baca..1a5c4f980f 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -23,6 +23,7 @@ import { TelemetryService } from "@roo-code/telemetry" import { logger } from "../../utils/logging" import { supportPrompt } from "../../shared/support-prompt" import { downgradeLegacyRooConfig } from "./routerRemoval" +import { forgetSettingStore, isTrackedSetting, noteSettingStored } from "./settingWrites" type GlobalStateKey = keyof GlobalState type SecretStateKey = keyof SecretState @@ -361,12 +362,29 @@ export class ContextProxy { } updateGlobalState(key: K, value: GlobalState[K]) { + const storeNumber = noteSettingStored(key) + if (isPassThroughStateKey(key)) { return this.originalContext.globalState.update(key, value) } this.stateCache[key] = value - return this.originalContext.globalState.update(key, value) + const updated = this.originalContext.globalState.update(key, value) + // Annotated, so it is a plain flag: the key is not narrowed to the tracked ones below. + const tracked: boolean = isTrackedSetting(key) + if (!tracked) return updated + + // A rejected store may or may not have reached storage (it can fail after changing it). A rollback must not + // treat it as someone else's store, and the cache must show what storage really holds, so that the next save + // reads, and a rollback restores, the true value. + return Promise.resolve(updated).catch((error: unknown) => { + if (storeNumber !== undefined) forgetSettingStore(key, storeNumber) + // Unless a newer store has replaced the value meanwhile: that value stays. + if (this.stateCache[key] === value) { + this.stateCache[key] = this.originalContext.globalState.get(key) + } + throw error + }) } private getAllGlobalState(): GlobalState { diff --git a/src/core/config/__tests__/ContextProxy.spec.ts b/src/core/config/__tests__/ContextProxy.spec.ts index b4d3b1b8aa..70fdecda1e 100644 --- a/src/core/config/__tests__/ContextProxy.spec.ts +++ b/src/core/config/__tests__/ContextProxy.spec.ts @@ -8,6 +8,7 @@ import { clearAllMocks } from "../../../test-utils/reset" import { makeExtensionContext, makeUri } from "../../../test-utils/vscode" import { ContextProxy } from "../ContextProxy" +import { beginSettingWrite, finishSettingWrites, performSettingWrite, undoSettingWrite } from "../settingWrites" import { providerIdentifiers, retiredProviderIdentifiers } from "@roo-code/types/provider-identifiers" vi.mock("vscode", () => ({ @@ -157,6 +158,111 @@ describe("ContextProxy", () => { }) describe("updateGlobalState", () => { + it("does not count a store of a tracked setting that was rejected, so a rollback still happens", async () => { + await proxy.setValue("timeFormat", "auto") + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(proxy, write) + mockGlobalState.get.mockReturnValue("24h") // storage holds the write's value + mockGlobalState.update.mockRejectedValueOnce(new Error("storage unavailable")) + + await expect(proxy.setValue("timeFormat", "24h")).rejects.toThrow("storage unavailable") + await undoSettingWrite(proxy, write) + + expect(mockGlobalState.update).toHaveBeenLastCalledWith("timeFormat", "auto") + finishSettingWrites([write]) + }) + + describe("the cache after a rejected store of a tracked setting", () => { + // Storage as the extension host has it: a store changes it, unless the test makes the store fail first. + let durable: Map + let failAfterStoring: boolean + + beforeEach(() => { + durable = new Map() + failAfterStoring = false + mockGlobalState.get.mockImplementation((key: string) => durable.get(key)) + mockGlobalState.update.mockImplementation(async (key: string, value: unknown) => { + if (failAfterStoring) { + durable.set(key, value) + throw new Error("storage unavailable") + } + durable.set(key, value) + }) + }) + + it("shows what storage holds when the store failed before changing it, so the next save reads that", async () => { + await proxy.setValue("timeFormat", "24h") + mockGlobalState.update.mockRejectedValueOnce(new Error("storage unavailable")) + + await expect(proxy.setValue("timeFormat", "12h")).rejects.toThrow("storage unavailable") + + expect(proxy.getValue("timeFormat")).toBe("24h") + const write = beginSettingWrite("timeFormat", undefined, "auto") + await performSettingWrite(proxy, write) + expect(write.previous).toBe("24h") // what is really stored, not the rejected 12h + finishSettingWrites([write]) + }) + + it("shows the new value when the store failed after changing storage, so a rollback can put it back", async () => { + await proxy.setValue("timeFormat", "auto") + const write = beginSettingWrite("timeFormat", undefined, "12h") + + failAfterStoring = true + await expect(performSettingWrite(proxy, write)).rejects.toThrow("storage unavailable") + failAfterStoring = false + expect(durable.get("timeFormat")).toBe("12h") // storage was changed by the failed store + + await undoSettingWrite(proxy, write) + + expect(durable.get("timeFormat")).toBe("auto") + expect(proxy.getValue("timeFormat")).toBe("auto") + }) + + it("keeps a newer store's value when the older one is rejected afterwards", async () => { + await proxy.setValue("timeFormat", "24h") + let rejectOlder!: (error: Error) => void + mockGlobalState.update.mockImplementationOnce( + () => new Promise((_resolve, reject) => (rejectOlder = reject)), + ) + const older = proxy.setValue("timeFormat", "12h").catch(() => undefined) // pending + await proxy.setValue("timeFormat", "auto") // a newer store lands + + rejectOlder(new Error("storage unavailable")) + await older + + expect(proxy.getValue("timeFormat")).toBe("auto") + }) + + it("leaves an untracked setting's cache as it always was after a rejection", async () => { + mockGlobalState.update.mockRejectedValueOnce(new Error("untracked failure")) + + await expect(proxy.updateGlobalState("apiProvider", "deepseek")).rejects.toThrow("untracked failure") + + expect(proxy.getValue("apiProvider")).toBe("deepseek") + }) + }) + + it("rejects with the original error, and only a tracked setting's store is wrapped", async () => { + mockGlobalState.update.mockRejectedValueOnce(new Error("untracked failure")) + + await expect(proxy.updateGlobalState("apiProvider", "deepseek")).rejects.toThrow("untracked failure") + }) + + it.each([ + ["setValue", (p: ContextProxy) => p.setValue("timeFormat", "24h")], + ["setValues", (p: ContextProxy) => p.setValues({ timeFormat: "24h" })], + ["updateGlobalState", async (p: ContextProxy) => p.updateGlobalState("timeFormat", "24h")], + ])("reports a store of a tracked setting made by %s, so a rollback does not undo it", async (_name, store) => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(proxy, write) + await store(proxy) // someone else stores the same value after the save's write + + await undoSettingWrite(proxy, write) + + expect(proxy.getValue("timeFormat")).toBe("24h") + finishSettingWrites([write]) + }) + it("should update state directly in original context", async () => { await proxy.updateGlobalState("apiProvider", "deepseek") diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index 90db81d8a1..5ad3ac1feb 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -19,6 +19,7 @@ import { } from "../importExport" import { ProviderSettingsManager } from "../ProviderSettingsManager" import { ContextProxy } from "../ContextProxy" +import { beginSettingWrite, performSettingWrite, settingWriteBookkeepingSize, undoSettingWrite } from "../settingWrites" import { CustomModesManager } from "../CustomModesManager" import { safeWriteJson } from "../../../utils/safeWriteJson" @@ -251,6 +252,282 @@ describe("importExport", () => { }) }) + describe("importSettingsFromPath display settings atomicity", () => { + type Stored = string | boolean | undefined + let stored: Map + let failingKeys: Set + let failingRestoreKeys: Set + // Keys whose write lands only after the given delay (ms), and keys whose write stores the value and then fails. + let slowKeys: Map + let failAfterStoringKeys: Set + // Runs right after a key was stored, to emulate another save writing meanwhile. + let afterStored: Map void> + + const importWith = (globalSettings: Record) => { + ;(fs.readFile as Mock).mockResolvedValue( + JSON.stringify({ + providerProfiles: { + currentApiConfigName: "test", + apiConfigs: { + test: { apiProvider: providerIdentifiers.openai, apiKey: "test-key", id: "test-id" }, + }, + }, + globalSettings, + }), + ) + return importSettingsFromPath("/mock/path/settings.json", { + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + } + + beforeEach(() => { + stored = new Map([ + ["timeFormat", "auto"], + ["dateFormat", "auto"], + ["showRelativeTimes", true], + ["mode", "architect"], + ]) + failingKeys = new Set() + failingRestoreKeys = new Set() + slowKeys = new Map() + failAfterStoringKeys = new Set() + afterStored = new Map void>() + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: providerIdentifiers.anthropic, id: "default-id" } }, + }) + mockProviderSettingsManager.import.mockResolvedValue(undefined) + mockProviderSettingsManager.listConfig.mockResolvedValue([]) + mockContextProxy.setProviderSettings.mockResolvedValue(undefined) + + // Map-backed double mirroring ContextProxy.setValues (Promise.all over setValue). + // The casts bridge the generic ContextProxy signatures to this untyped Map store. + const getValue = (key: string): Stored => stored.get(key) + const setValue = async (key: string, value: Stored) => { + if (failingKeys.has(key)) throw new Error(`write failed: ${key}`) + if (failingRestoreKeys.has(key) && stored.get(key) !== undefined && value === "auto") { + throw new Error(`restore failed: ${key}`) + } + const delay = slowKeys.get(key) + if (delay !== undefined) await new Promise((resolve) => setTimeout(resolve, delay)) + stored.set(key, value) + afterStored.get(key)?.() + // Only the import's own write fails; putting the previous value back works. + if (failAfterStoringKeys.has(key) && value !== "auto") + throw new Error(`write failed after storing: ${key}`) + } + mockContextProxy.getValue.mockImplementation(getValue as unknown as typeof mockContextProxy.getValue) + mockContextProxy.setValue.mockImplementation(setValue as unknown as typeof mockContextProxy.setValue) + mockContextProxy.setValues.mockImplementation((async (values: Record) => { + await Promise.all(Object.entries(values).map(([k, v]) => setValue(k, v as Stored))) + }) as unknown as typeof mockContextProxy.setValues) + }) + + const display = { timeFormat: "24h", dateFormat: "ymd", showRelativeTimes: false } + + it("reports a failure that is not an Error by its text when a restore also fails", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}) + mockContextProxy.setValues.mockImplementationOnce(async () => { + throw "plain failure" + }) + failingRestoreKeys.add("timeFormat") + + const result = await importWith({ timeFormat: "24h", mode: "code" }) + + expect(result).toEqual({ + success: false, + error: "plain failure (these settings could not be restored: timeFormat)", + }) + consoleError.mockRestore() + }) + + it("is the latest write of the setting, so a failed save that wrote the same value does not undo it", async () => { + // A save wrote "24h" and has not finished. The import then writes the same value, and succeeds. + const saveWrite = beginSettingWrite("timeFormat", "auto", "24h") + stored.set("timeFormat", "24h") + + const result = await importWith({ timeFormat: "24h" }) + expect(result).toMatchObject({ success: true }) + + // The save now fails and is rolled back: the import's value stands. + await undoSettingWrite(mockContextProxy as unknown as ContextProxy, saveWrite) + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("writes all three display settings on success", async () => { + const result = await importWith(display) + + expect(result).toMatchObject({ success: true }) + expect(stored.get("timeFormat")).toBe("24h") + expect(stored.get("dateFormat")).toBe("ymd") + expect(stored.get("showRelativeTimes")).toBe(false) + }) + + it("restores written display settings and reports the original error when one write fails", async () => { + failingKeys.add("dateFormat") + + const result = await importWith(display) + + expect(result).toEqual({ success: false, error: "write failed: dateFormat" }) + expect(stored.get("timeFormat")).toBe("auto") + expect(stored.get("dateFormat")).toBe("auto") + expect(stored.get("showRelativeTimes")).toBe(true) + }) + + it("keeps restoring after a failed restore, and reports which settings could not be restored", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}) + failingKeys.add("dateFormat") + failingRestoreKeys.add("timeFormat") + + const result = await importWith(display) + + // The original error comes first, then the part of the import that is still stored. + expect(result).toEqual({ + success: false, + error: "write failed: dateFormat (these settings could not be restored: timeFormat)", + }) + expect(stored.get("timeFormat")).toBe("24h") + expect(stored.get("showRelativeTimes")).toBe(true) + expect(consoleError).toHaveBeenCalledWith( + 'Failed to restore setting "timeFormat" after import failure:', + expect.any(Error), + ) + consoleError.mockRestore() + }) + + it("does not undo a display setting changed concurrently after the import wrote it", async () => { + failingKeys.add("mode") + // Another save changes two of them right after the import wrote the last one. + afterStored.set("showRelativeTimes", () => { + stored.set("timeFormat", "12h") + stored.set("dateFormat", "dmy") + }) + + const result = await importWith({ ...display, mode: "code" }) + + expect(result).toEqual({ success: false, error: "write failed: mode" }) + expect(stored.get("timeFormat")).toBe("12h") + expect(stored.get("dateFormat")).toBe("dmy") + // Matches what the import wrote, so it is rolled back. + expect(stored.get("showRelativeTimes")).toBe(true) + expect(mockContextProxy.setValue).not.toHaveBeenCalledWith("timeFormat", "auto") + expect(mockContextProxy.setValue).not.toHaveBeenCalledWith("dateFormat", "auto") + }) + + it("waits for every display write before restoring, so a slow write cannot land after its restore", async () => { + failingKeys.add("dateFormat") + slowKeys.set("timeFormat", 30) + + const result = await importWith(display) + + expect(result).toEqual({ success: false, error: "write failed: dateFormat" }) + // Look again after the slow write's delay: a restore made before it landed would be overwritten by now. + await new Promise((resolve) => setTimeout(resolve, 80)) + expect(stored.get("timeFormat")).toBe("auto") + expect(stored.get("showRelativeTimes")).toBe(true) + }) + + it("puts back a display setting whose own write stored the value and then failed", async () => { + failAfterStoringKeys.add("dateFormat") + + const result = await importWith(display) + + expect(result).toEqual({ success: false, error: "write failed after storing: dateFormat" }) + expect(stored.get("dateFormat")).toBe("auto") + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("reports a failure of another setting, and puts the display settings back", async () => { + failingKeys.add("mode") + + const result = await importWith({ ...display, mode: "code" }) + + expect(result).toEqual({ success: false, error: "write failed: mode" }) + expect(stored.get("timeFormat")).toBe("auto") + expect(stored.get("dateFormat")).toBe("auto") + expect(stored.get("showRelativeTimes")).toBe(true) + }) + + describe("when a step after the settings were written fails", () => { + it("puts the display settings back and reports the failure of that step", async () => { + mockContextProxy.setProviderSettings.mockRejectedValue(new Error("provider settings failed")) + + const result = await importWith(display) + + expect(result).toEqual({ success: false, error: "provider settings failed" }) + expect(stored.get("timeFormat")).toBe("auto") + expect(stored.get("dateFormat")).toBe("auto") + expect(stored.get("showRelativeTimes")).toBe(true) + }) + + it("says which display settings could not be put back", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}) + mockContextProxy.setProviderSettings.mockRejectedValue(new Error("provider settings failed")) + failingRestoreKeys.add("timeFormat") + + const result = await importWith(display) + + expect(result).toEqual({ + success: false, + error: "provider settings failed (these settings could not be restored: timeFormat)", + }) + expect(stored.get("dateFormat")).toBe("auto") + consoleError.mockRestore() + }) + + it("keeps a save that wrote the same setting while the step was running", async () => { + mockContextProxy.setProviderSettings.mockImplementation(async () => { + const save = beginSettingWrite("timeFormat", stored.get("timeFormat"), "24h") + await performSettingWrite(mockContextProxy as unknown as ContextProxy, save) + throw new Error("provider settings failed") + }) + + await importWith(display) + + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("keeps the display settings when every step succeeds, and keeps nothing about the writes", async () => { + const result = await importWith(display) + + expect(result).toMatchObject({ success: true }) + expect(stored.get("timeFormat")).toBe("24h") + expect(settingWriteBookkeepingSize()).toEqual({ latestWrites: 0, queuedKeys: 0 }) + }) + }) + + it("leaves display settings absent from the file untouched on failure", async () => { + // Not the defaults, so a reset to the default could not pass for "untouched". + stored.set("timeFormat", "12h") + stored.set("showRelativeTimes", false) + failingKeys.add("dateFormat") + + await importWith({ dateFormat: "ymd" }) + + expect(stored.get("timeFormat")).toBe("12h") + expect(stored.get("showRelativeTimes")).toBe(false) + expect(mockContextProxy.setValue).not.toHaveBeenCalledWith("timeFormat", expect.anything()) + expect(mockContextProxy.setValue).not.toHaveBeenCalledWith("showRelativeTimes", expect.anything()) + // setValues does not go through the setValue spy, so check what it was given as well. + for (const [values] of mockContextProxy.setValues.mock.calls) { + expect(Object.keys(values)).not.toEqual(expect.arrayContaining(["timeFormat"])) + expect(Object.keys(values)).not.toEqual(expect.arrayContaining(["showRelativeTimes"])) + } + }) + + it("does not restore non-display settings when their write fails", async () => { + failingKeys.add("mode") + + const result = await importWith({ ...display, mode: "code" }) + + expect(result).toEqual({ success: false, error: "write failed: mode" }) + expect(stored.get("mode")).toBe("architect") + expect(mockContextProxy.setValue).not.toHaveBeenCalledWith("mode", "architect") + }) + }) + describe("importSettings", () => { it("should return success: false when user cancels file selection", async () => { ;(vscode.window.showOpenDialog as Mock).mockResolvedValue(undefined) diff --git a/src/core/config/__tests__/settingWrites.spec.ts b/src/core/config/__tests__/settingWrites.spec.ts new file mode 100644 index 0000000000..65c88496b4 --- /dev/null +++ b/src/core/config/__tests__/settingWrites.spec.ts @@ -0,0 +1,457 @@ +import { + TRACKED_SETTING_KEYS, + beginSettingWrite, + finishSettingWrites, + forgetSettingStore, + isTrackedSetting, + noteSettingStored, + performSettingWrite, + undoSettingWrite, +} from "../settingWrites" +import type { ContextProxy } from "../ContextProxy" + +describe("settingWrites", () => { + let stored: Map + // Only the two methods the module uses. The cast says so: the real ContextProxy has far more. + let contextProxy: ContextProxy + + beforeEach(() => { + stored = new Map([["timeFormat", "auto"]]) + contextProxy = { + getValue: (key: string) => stored.get(key), + setValue: async (key: string, value: unknown) => { + stored.set(key, value) + }, + } as unknown as ContextProxy + }) + + /** Stores the value, as the write that was just recorded would have. */ + const store = (key: string, value: unknown) => stored.set(key, value) + + describe("which settings are tracked", () => { + it.each(TRACKED_SETTING_KEYS)("tracks %s", (key) => { + expect(isTrackedSetting(key)).toBe(true) + }) + + it.each(["soundEnabled", "language", "", "TimeFormat"])("does not track %j", (key) => { + expect(isTrackedSetting(key)).toBe(false) + }) + }) + + describe("a save and an import writing the same setting", () => { + it("a failed save does not undo an import that wrote the same value afterwards", async () => { + const save = beginSettingWrite("timeFormat", "auto", "24h") + store("timeFormat", "24h") + const importWrite = beginSettingWrite("timeFormat", "24h", "24h") + store("timeFormat", "24h") + finishSettingWrites([importWrite]) + + await undoSettingWrite(contextProxy, save) + + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("a failed import does not undo a save that wrote the same value afterwards", async () => { + const importWrite = beginSettingWrite("timeFormat", "auto", "12h") + store("timeFormat", "12h") + const save = beginSettingWrite("timeFormat", "12h", "12h") + store("timeFormat", "12h") + finishSettingWrites([save]) + + await undoSettingWrite(contextProxy, importWrite) + + expect(stored.get("timeFormat")).toBe("12h") + }) + + it("when both fail, each restores what was stored before either of them", async () => { + const save = beginSettingWrite("timeFormat", "auto", "24h") + store("timeFormat", "24h") + const importWrite = beginSettingWrite("timeFormat", "24h", "12h") + store("timeFormat", "12h") + + await undoSettingWrite(contextProxy, save) // superseded by the import: nothing changes yet + expect(stored.get("timeFormat")).toBe("12h") + + await undoSettingWrite(contextProxy, importWrite) + expect(stored.get("timeFormat")).toBe("auto") + }) + }) + + describe("a store made by anyone else (the public API, other callers of setValues)", () => { + beforeEach(() => { + // As ContextProxy: every store is reported, synchronously, when it is made. + contextProxy = { + getValue: (key: string) => stored.get(key), + setValue: async (key: string, value: unknown) => { + noteSettingStored(key) + stored.set(key, value) + }, + } as unknown as ContextProxy + }) + + it("is kept when a failed write is rolled back, even if it stored the same value", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(contextProxy, write) + + await contextProxy.setValue("timeFormat" as never, "24h" as never) // the API stores the same value + + await undoSettingWrite(contextProxy, write) + + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("keeps a store made while the restore of the undone latest write is still waiting in the queue", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(contextProxy, write) + + const undone = undoSettingWrite(contextProxy, write) // the restore waits its turn in the queue + await contextProxy.setValue("timeFormat" as never, "24h" as never) // the same value, by someone else + await undone + + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("keeps the key pending until every waiting restore has run", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + + const undoSecond = undoSettingWrite(contextProxy, second) // two restores wait in the queue + const undoFirst = undoSettingWrite(contextProxy, first) + await undoSecond + await undoFirst + + expect(stored.get("timeFormat")).toBe("auto") + expect(noteSettingStored("timeFormat")).toBeUndefined() // all restores have run + }) + + it("still puts the write back when nobody stored the key while the restore waited", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(contextProxy, write) + + await undoSettingWrite(contextProxy, write) + + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("keeps nothing for a store made after the restore has run", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(contextProxy, write) + await undoSettingWrite(contextProxy, write) + + expect(noteSettingStored("timeFormat")).toBeUndefined() + }) + + it("does not stop a write from being put back when nobody else stored the key", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(contextProxy, write) + + await undoSettingWrite(contextProxy, write) + + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("only counts stores of tracked keys", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(contextProxy, write) + + await contextProxy.setValue("soundEnabled" as never, true as never) + await undoSettingWrite(contextProxy, write) + + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("does not count the restore of a rolled-back write as a store by someone else", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + + await undoSettingWrite(contextProxy, second) // restores "12h", which the first write owns again + await undoSettingWrite(contextProxy, first) + + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("keeps the value of a write that finished between two that fail", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + const third = beginSettingWrite("timeFormat", "24h", "9h") // registered while the second is pending + finishSettingWrites([second]) // committed, between the other two + await performSettingWrite(contextProxy, third) + + await undoSettingWrite(contextProxy, first) // superseded: the committed value is not its to restore + await undoSettingWrite(contextProxy, third) + + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("leaves what a write read when it started, if that write had already started when the earlier one finished", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + const third = beginSettingWrite("timeFormat", "24h", "9h") + await performSettingWrite(contextProxy, third) // started: it read "24h" + + finishSettingWrites([second]) + await undoSettingWrite(contextProxy, third) + + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("keeps an external store made after a write stored, when that write is handed over", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + await contextProxy.setValue("timeFormat" as never, "12h" as never) // external store before the handover + const third = beginSettingWrite("timeFormat", "24h", "9h") + + finishSettingWrites([second]) + await performSettingWrite(contextProxy, third) + await undoSettingWrite(contextProxy, third) + + expect(stored.get("timeFormat")).toBe("12h") + }) + + it("keeps an external store made after a superseded write stored, before the write after it starts", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await contextProxy.setValue("timeFormat" as never, "9h" as never) // external store before the superseded undo + + await undoSettingWrite(contextProxy, first) // superseded by second + await performSettingWrite(contextProxy, second) + await undoSettingWrite(contextProxy, second) + + expect(stored.get("timeFormat")).toBe("9h") + }) + + it("hands over the value of a write that finished without having been stored through performSettingWrite", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + store("timeFormat", "12h") + const second = beginSettingWrite("timeFormat", "12h", "24h") + + finishSettingWrites([first]) + await performSettingWrite(contextProxy, second) + await undoSettingWrite(contextProxy, second) + + expect(stored.get("timeFormat")).toBe("12h") + }) + + it("keeps the value of a write that finished before a queued write that then fails", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + const third = beginSettingWrite("timeFormat", "24h", "9h") // queued, not started + + await undoSettingWrite(contextProxy, first) + finishSettingWrites([second]) + await performSettingWrite(contextProxy, third) + await undoSettingWrite(contextProxy, third) + + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("restores through a whole chain of failed writes: no restore is taken for a store by someone else", async () => { + const first = beginSettingWrite("timeFormat", "auto", "9h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "9h", "12h") + await performSettingWrite(contextProxy, second) + const third = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, third) + + await undoSettingWrite(contextProxy, third) + await undoSettingWrite(contextProxy, second) + await undoSettingWrite(contextProxy, first) + + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("does not hand a rolled-back value to a write that already started after someone else stored the key", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + await contextProxy.setValue("timeFormat" as never, "9h" as never) // someone else stores + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) // started: it read "9h" + + await undoSettingWrite(contextProxy, first) // superseded + await undoSettingWrite(contextProxy, second) + + expect(stored.get("timeFormat")).toBe("9h") + }) + + it("does not hand a rolled-back value on when the other store has the same value", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + await contextProxy.setValue("timeFormat" as never, "12h" as never) // the same value, by someone else + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + + await undoSettingWrite(contextProxy, first) + await undoSettingWrite(contextProxy, second) + + expect(stored.get("timeFormat")).toBe("12h") + }) + + it("keeps a store made after the restore that handed the value back, even with the same value", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + + await undoSettingWrite(contextProxy, second) + await contextProxy.setValue("timeFormat" as never, "12h" as never) // someone else stores the same value + await undoSettingWrite(contextProxy, first) + + expect(stored.get("timeFormat")).toBe("12h") + }) + + it("reads what a handed-over write replaces when someone else stored the key before it started", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") // queued, not started + + await undoSettingWrite(contextProxy, first) // hands "auto" to the second write + await contextProxy.setValue("timeFormat" as never, "9h" as never) // someone else stores before it starts + await performSettingWrite(contextProxy, second) + await undoSettingWrite(contextProxy, second) + + expect(stored.get("timeFormat")).toBe("9h") + }) + + it("keeps a handed-over value when nobody else stored the key before the write started", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + + await undoSettingWrite(contextProxy, first) + await performSettingWrite(contextProxy, second) + await undoSettingWrite(contextProxy, second) + + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("puts back the earlier write when the write after it is rolled back, and that one then fails too", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + await performSettingWrite(contextProxy, first) + const second = beginSettingWrite("timeFormat", "12h", "24h") + await performSettingWrite(contextProxy, second) + + await undoSettingWrite(contextProxy, second) // puts "12h" back: the first owns the value again + expect(stored.get("timeFormat")).toBe("12h") + await undoSettingWrite(contextProxy, first) + + expect(stored.get("timeFormat")).toBe("auto") + }) + }) + + describe("noteSettingStored and forgetSettingStore", () => { + it("number the stores of a tracked key made while a write of it is pending, and tolerate an unknown one", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + expect(noteSettingStored("soundEnabled")).toBeUndefined() + const first = noteSettingStored("timeFormat") + const second = noteSettingStored("timeFormat") + expect(first).toEqual(expect.any(Number)) + expect(second).toBeGreaterThan(first as number) + + forgetSettingStore("soundEnabled", 1) // not tracked: ignored + forgetSettingStore("dateFormat", 1) // nothing recorded for the key + forgetSettingStore("timeFormat", first as number) + forgetSettingStore("timeFormat", first as number) // already forgotten + forgetSettingStore("timeFormat", second as number) + finishSettingWrites([write]) + }) + + it("keeps nothing for a store made while no write of the key is pending", () => { + expect(noteSettingStored("timeFormat")).toBeUndefined() + expect(noteSettingStored("dateFormat")).toBeUndefined() + }) + + it("does not let stores made before a write stored block its rollback, nor grow without a write", async () => { + for (let i = 0; i < 1000; i++) noteSettingStored("timeFormat") // no write pending: nothing is recorded + + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(contextProxy, write) + await undoSettingWrite(contextProxy, write) + + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("a rejected store cancels only itself, not a store made by someone else after it", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + await performSettingWrite(contextProxy, write) + const rejected = noteSettingStored("timeFormat") as number // an outside store, still pending + const later = noteSettingStored("timeFormat") as number // another outside store, which lands + void later + forgetSettingStore("timeFormat", rejected) // the first one is rejected + + await undoSettingWrite(contextProxy, write) + + // The later store is not forgotten: the failed write leaves the value alone. + expect(stored.get("timeFormat")).toBe("24h") + }) + }) + + describe("performSettingWrite", () => { + it("reads what the write replaces when it starts", async () => { + const write = beginSettingWrite("timeFormat", "stale", "24h") + store("timeFormat", "12h") // an earlier write reached storage after this one registered + + await performSettingWrite(contextProxy, write) + + expect(write.previous).toBe("12h") + expect(stored.get("timeFormat")).toBe("24h") + }) + + it("keeps the value handed over by a write that was rolled back before this one started", async () => { + const first = beginSettingWrite("timeFormat", "auto", "12h") + store("timeFormat", "12h") + const second = beginSettingWrite("timeFormat", "12h", "24h") + + await undoSettingWrite(contextProxy, first) // superseded: hands "auto" to the second write + await performSettingWrite(contextProxy, second) + expect(second.previous).toBe("auto") + + await undoSettingWrite(contextProxy, second) + expect(stored.get("timeFormat")).toBe("auto") + }) + }) + + it("puts a failed write back to what was stored before it", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + store("timeFormat", "24h") + + await undoSettingWrite(contextProxy, write) + + expect(stored.get("timeFormat")).toBe("auto") + }) + + it("leaves a value alone that was changed by something else after the write", async () => { + const write = beginSettingWrite("timeFormat", "auto", "24h") + store("timeFormat", "12h") + + await undoSettingWrite(contextProxy, write) + + expect(stored.get("timeFormat")).toBe("12h") + }) + + it("does not link a new write to one that has finished", async () => { + const first = beginSettingWrite("timeFormat", "auto", "24h") + store("timeFormat", "24h") + finishSettingWrites([first]) + const second = beginSettingWrite("timeFormat", "24h", "12h") + store("timeFormat", "12h") + + await undoSettingWrite(contextProxy, second) + + // Back to the finished write's value, not past it. + expect(stored.get("timeFormat")).toBe("24h") + expect(second.prev).toBeUndefined() + }) +}) diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index 9882039ad3..ab9a17be73 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -18,6 +18,15 @@ import { TelemetryService } from "@roo-code/telemetry" import { ProviderSettingsManager, providerProfilesSchema } from "./ProviderSettingsManager" import { ContextProxy } from "./ContextProxy" +import { + TRACKED_SETTING_KEYS, + beginSettingWrite, + finishSettingWrites, + isTrackedSetting, + performSettingWrite, + undoSettingWrite, + type SettingWrite, +} from "./settingWrites" import { CustomModesManager } from "./CustomModesManager" import { downgradeLegacyRooConfig, ROUTER_REMOVAL_IMPORT_WARNING } from "./routerRemoval" import { resolveDefaultSaveUri, saveLastExportPath } from "../../utils/export" @@ -147,6 +156,9 @@ export async function importSettingsFromPath( globalSettings: z.unknown().optional(), }) + // Display writes of this import that are not yet final: they are put back if a later step fails. + let displayWrites: SettingWrite[] = [] + try { const previousProviderProfiles = await providerSettingsManager.export() @@ -228,7 +240,7 @@ export async function importSettingsFromPath( // They will be imported automatically with the config - no special handling needed await providerSettingsManager.import(providerProfiles) - await contextProxy.setValues(sanitizedGlobalSettings) + displayWrites = await setValuesRestoringDisplaySettings(contextProxy, sanitizedGlobalSettings) // Set the current provider. const currentProviderName = providerProfiles.currentApiConfigName @@ -244,13 +256,18 @@ export async function importSettingsFromPath( await contextProxy.setValue("listApiConfigMeta", await providerSettingsManager.listConfig()) + // Every step has succeeded: the display writes are final. + finishSettingWrites(displayWrites) + return { providerProfiles, globalSettings: sanitizedGlobalSettings, success: true, warnings: warnings.length > 0 ? warnings : undefined, } - } catch (e) { + } catch (failure) { + // A step after the settings were written failed: the display settings are put back with the rest undone. + const e = withRestoreReport(failure, await restoreDisplayWrites(contextProxy, displayWrites)) let error = "Unknown error" if (e instanceof ZodError) { @@ -264,6 +281,68 @@ export async function importSettingsFromPath( } } +/** + * Puts the writes of an import back, last first. Returns the settings that could not be put back. + */ +async function restoreDisplayWrites(contextProxy: ContextProxy, writes: SettingWrite[]): Promise { + const notRestored: string[] = [] + + for (const write of [...writes].reverse()) { + try { + await undoSettingWrite(contextProxy, write) + } catch (restoreError) { + notRestored.push(write.key) + console.error(`Failed to restore setting "${write.key}" after import failure:`, restoreError) + } + } + + return notRestored +} + +/** The failure with a note of what is still stored, when part of the import could not be put back. */ +function withRestoreReport(failure: unknown, notRestored: string[]): unknown { + if (notRestored.length === 0) return failure + const reason = failure instanceof Error ? failure.message : String(failure) + return new Error(`${reason} (these settings could not be restored: ${notRestored.join(", ")})`) +} + +/** + * Writes global settings. The display settings are written all-or-nothing: every write is awaited before anything is + * restored (`setValues` rejects at the first failure while its other writes are still running, and a restore made + * then could be overwritten by a write that lands later), and then the ones this import changed are put back. The + * writes are recorded with the same bookkeeping as saves (`settingWrites`), so a failed import does not undo a save + * that wrote the same setting meanwhile, and a failed save does not undo this import. The original error is + * rethrown, so the caller's failure handling is unchanged. Other settings are written as before. + * + * On success the display writes are returned still active: the caller finishes them when the whole import has + * succeeded, or puts them back (`restoreDisplayWrites`) if a later step fails. + */ +async function setValuesRestoringDisplaySettings( + contextProxy: ContextProxy, + settings: GlobalSettings, +): Promise { + const displayKeys = TRACKED_SETTING_KEYS.filter((key) => key in settings) + if (displayKeys.length === 0) { + await contextProxy.setValues(settings) + return [] + } + + const writes = displayKeys.map((key) => beginSettingWrite(key, contextProxy.getValue(key), settings[key])) + const others = Object.fromEntries( + Object.entries(settings).filter(([key]) => !isTrackedSetting(key)), + ) as GlobalSettings + + const results = await Promise.allSettled([ + contextProxy.setValues(others), + ...writes.map((write) => performSettingWrite(contextProxy, write)), + ]) + const failed = results.find((result): result is PromiseRejectedResult => result.status === "rejected") + if (!failed) return writes + + // Part of the import may still be stored: say so, instead of reporting only the first failure. + throw withRestoreReport(failed.reason, await restoreDisplayWrites(contextProxy, writes)) +} + /** * Import settings from a file using a file dialog * @param options - Import options containing managers and proxy diff --git a/src/core/config/settingWrites.ts b/src/core/config/settingWrites.ts new file mode 100644 index 0000000000..3c35bc0783 --- /dev/null +++ b/src/core/config/settingWrites.ts @@ -0,0 +1,237 @@ +import type { RooCodeSettings } from "@roo-code/types" + +import type { ContextProxy } from "./ContextProxy" + +/** The settings whose writes are tracked, so a failed save or import can be put back without undoing another one. */ +export const TRACKED_SETTING_KEYS = ["timeFormat", "dateFormat", "showRelativeTimes"] as const + +export type TrackedSettingKey = (typeof TRACKED_SETTING_KEYS)[number] + +export const isTrackedSetting = (key: string): key is TrackedSettingKey => + (TRACKED_SETTING_KEYS as readonly string[]).includes(key) + +/** One write of a tracked setting by a save or an import, linked to the writes of others that overlap it. */ +export interface SettingWrite { + key: TrackedSettingKey + /** What was stored when this write was made. With overlapping writes, this can be another write's pending value. */ + previous: unknown + /** The store clock just after this write's own store. A later store by someone else means something else wrote it. */ + generation?: number + /** + * Set when `previous` was handed over by another write, to the store clock at the time that write stored its own + * value. The handed-over value is kept only while no store by someone else is later than that. + */ + handedOverAt?: number + wrote: unknown + /** The write before this one that has not finished yet, and the one after it. */ + prev?: SettingWrite + next?: SettingWrite + /** The save or import has finished: it succeeded, or it has been rolled back. */ + done: boolean +} + +/** + * The latest write of each tracked setting. The store is shared by every provider (sidebar, editor tab) and by the + * settings import, so writes can overlap, and the writes to one key form a chain. Rolling a write back puts the key + * back only while it is the latest one and the stored value is still what it wrote, so another write's value is kept + * even when it is the same value. A write that is rolled back while a newer one is pending is taken out of the chain, + * so that if the newer one fails too, it restores what was stored before either of them. + */ +const lastWriteOfSetting = new Map() + +/** + * The stores of tracked keys made by anyone but the saves and imports here. `ContextProxy` reports every store + * (`noteSettingStored`), including the ones that do not go through a save or an import (the public API, other callers + * of `setValues`). Each is numbered on a clock that only they advance. A write remembers the clock just after its own + * store, and is put back only if no store by someone else has a later number, so a store made by anyone else in + * between is kept, even when it stored the same value. A store that is rejected is taken out by its own number, so it + * cannot cancel another store's. + */ +const othersStores = new Map>() +let storeClockValue = 0 + +const storeClock = () => storeClockValue + +/** Whether anyone but the saves and imports here stored the key after the clock read `since`. */ +const othersStoredSince = (key: TrackedSettingKey, since: number) => { + for (const seq of othersStores.get(key) ?? []) if (seq > since) return true + return false +} + +/** The stores by others only matter while writes of the key are pending. */ +const pruneOthersStores = (key: TrackedSettingKey) => { + if (!isPending(key)) othersStores.delete(key) +} + +/** Restores of an undone write that are still waiting in the queue: the key is not settled until they have run. */ +const pendingRestores = new Map() + +const changePendingRestores = (key: TrackedSettingKey, change: 1 | -1) => { + const count = (pendingRestores.get(key) ?? 0) + change + if (count > 0) pendingRestores.set(key, count) + else pendingRestores.delete(key) +} + +/** A write of the key is pending, or a restore of one is still waiting its turn. */ +const isPending = (key: TrackedSettingKey) => lastWriteOfSetting.has(key) || (pendingRestores.get(key) ?? 0) > 0 + +/** The key whose store is being issued right now by a save or an import, so that its report is not counted. */ +let ownStoreOf: TrackedSettingKey | undefined + +/** Issues a store made here. `ContextProxy` reports it synchronously, while `store` runs. */ +const storeHere = (key: TrackedSettingKey, store: () => T): T => { + ownStoreOf = key + try { + return store() + } finally { + ownStoreOf = undefined + } +} + +/** + * `ContextProxy` calls this, synchronously, each time a setting is stored. Returns the number of the store when it was + * counted as one made by someone else (only while a write of the key is pending); if that store is then rejected, the caller reports it with + * `forgetSettingStore`. + */ +export const noteSettingStored = (key: string): number | undefined => { + // Only a store that a pending write could be mistaken for matters, so nothing is kept while no write of the key is + // pending: the set stays as small as the writes that are in flight, however often others store the key. + if (!isTrackedSetting(key) || key === ownStoreOf || !isPending(key)) return undefined + const seq = ++storeClockValue + othersStores.set(key, (othersStores.get(key) ?? new Set()).add(seq)) + return seq +} + +/** A store counted by `noteSettingStored` was rejected: it never reached storage, so it is not a store to protect. */ +export const forgetSettingStore = (key: string, seq: number) => { + if (isTrackedSetting(key)) othersStores.get(key)?.delete(seq) +} + +/** Records a write that is about to be made, as the latest one of its key. */ +export const beginSettingWrite = (key: TrackedSettingKey, previous: unknown, wrote: unknown): SettingWrite => { + const before = lastWriteOfSetting.get(key) + const write: SettingWrite = { key, previous, wrote, prev: before && !before.done ? before : undefined, done: false } + if (write.prev) write.prev.next = write + lastWriteOfSetting.set(key, write) + return write +} + +/** + * The writes to one key, one at a time, from every save, import and rollback, across providers. A write is only + * started once the one before it has reached durable storage, so a slower write cannot complete after a newer one + * and overwrite it, and a rollback cannot land after a newer write has started. A write that fails does not hold up + * the ones behind it. + */ +const settingWriteQueues = new Map>() + +const queuedSettingWrite = (key: TrackedSettingKey, write: () => Promise): Promise => { + const run = (settingWriteQueues.get(key) ?? Promise.resolve()).then(async () => { + await write() + }) + const tail = run.catch(() => undefined) + settingWriteQueues.set(key, tail) + void tail.then(() => { + if (settingWriteQueues.get(key) === tail) settingWriteQueues.delete(key) + }) + return run +} + +/** @internal test hook: how much is kept about writes. It is nothing once the saves and imports are over. */ +export const settingWriteBookkeepingSize = () => ({ + latestWrites: lastWriteOfSetting.size, + queuedKeys: settingWriteQueues.size, +}) + +/** + * Stores a recorded write, in line behind the writes of its key. What it replaces is read when it starts, after the + * writes before it have reached storage, unless a rolled-back write handed its own value over. + */ +export const performSettingWrite = (contextProxy: ContextProxy, write: SettingWrite): Promise => + queuedSettingWrite(write.key, () => { + // A handed-over value is kept only while nobody else has stored the key since it was handed over. + if (write.handedOverAt === undefined || othersStoredSince(write.key, write.handedOverAt)) { + write.previous = contextProxy.getValue(write.key as keyof RooCodeSettings) + } + const stored = storeHere(write.key, () => + contextProxy.setValue(write.key as keyof RooCodeSettings, write.wrote as never), + ) + write.generation = storeClock() + return stored + }) + +/** Takes a write out of its chain, joining the writes on either side of it. */ +const unlinkSettingWrite = (write: SettingWrite) => { + if (write.prev) write.prev.next = write.next + if (write.next) write.next.prev = write.prev + write.prev = undefined + write.next = undefined +} + +/** + * The save or import that made these writes succeeded. Nothing is kept about them once they are over. The value is + * committed, so it is what a later write that has not started yet replaces, and the writes on either side of it no + * longer hand values to each other: an earlier write that fails now has nothing left to put back behind it. + */ +export const finishSettingWrites = (writes: SettingWrite[]) => { + for (const write of writes) { + write.done = true + if (lastWriteOfSetting.get(write.key) === write) lastWriteOfSetting.delete(write.key) + + const { next } = write + if (next && !next.done) { + if (next.generation === undefined) { + next.previous = write.wrote + next.handedOverAt = write.generation ?? storeClock() + } + next.prev = undefined + } + if (write.prev) write.prev.next = undefined + write.prev = undefined + write.next = undefined + pruneOthersStores(write.key) + } +} + +/** Puts a failed write back, unless another write has superseded it (see `lastWriteOfSetting`). */ +export const undoSettingWrite = async (contextProxy: ContextProxy, write: SettingWrite) => { + const { key } = write + write.done = true + + if (lastWriteOfSetting.get(key) !== write) { + // A newer write exists. Make it restore the value from before this one if it fails as well, unless someone + // else stored the key since this write stored: a write that already started has read that store. + const heldByOthers = write.generation !== undefined && othersStoredSince(key, write.generation) + if (write.next && !write.next.done && !heldByOthers) { + write.next.previous = write.previous + write.next.handedOverAt = write.generation ?? storeClock() + } + unlinkSettingWrite(write) + return + } + + // The write before this one is pending again: it is the latest, and the value now stored is its value. + const { prev } = write + if (prev && !prev.done) lastWriteOfSetting.set(key, prev) + else lastWriteOfSetting.delete(key) + unlinkSettingWrite(write) + + // The restore waits in the queue. Until it has run the key is still pending, so a store by someone else that + // lands meanwhile is recorded, and the restore leaves it alone. + changePendingRestores(key, 1) + try { + await queuedSettingWrite(key, async () => { + const untouched = write.generation === undefined || !othersStoredSince(key, write.generation) + if (untouched && contextProxy.getValue(key as keyof RooCodeSettings) === write.wrote) { + const restored = storeHere(key, () => + contextProxy.setValue(key as keyof RooCodeSettings, write.previous as never), + ) + // The write before this one is the owner of the value again. + if (prev && !prev.done) prev.generation = storeClock() + await restored + } + }) + } finally { + changePendingRestores(key, -1) + pruneOthersStores(key) + } +} diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 39c6184d92..5037e5e20a 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2860,6 +2860,9 @@ export class ClineProvider historyPreviewCollapsed, reasoningBlockCollapsed, chatFontSize, + timeFormat, + dateFormat, + showRelativeTimes, enterBehavior, cloudUserInfo, cloudIsAuthenticated, @@ -3041,6 +3044,9 @@ export class ClineProvider historyPreviewCollapsed: historyPreviewCollapsed ?? false, reasoningBlockCollapsed: reasoningBlockCollapsed ?? true, chatFontSize, + timeFormat: timeFormat ?? "auto", + dateFormat: dateFormat ?? "auto", + showRelativeTimes: showRelativeTimes ?? true, enterBehavior: enterBehavior ?? "send", cloudUserInfo, cloudIsAuthenticated: cloudIsAuthenticated ?? false, @@ -3274,6 +3280,9 @@ export class ClineProvider historyPreviewCollapsed: stateValues.historyPreviewCollapsed ?? false, reasoningBlockCollapsed: stateValues.reasoningBlockCollapsed ?? true, chatFontSize: stateValues.chatFontSize, + timeFormat: stateValues.timeFormat ?? "auto", + dateFormat: stateValues.dateFormat ?? "auto", + showRelativeTimes: stateValues.showRelativeTimes ?? true, enterBehavior: stateValues.enterBehavior ?? "send", cloudUserInfo, cloudIsAuthenticated, diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 779313d385..668c3bc36c 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -2119,6 +2119,81 @@ describe("ClineProvider", () => { expect(state.allowedReadFiles).toEqual([]) }) + describe("date and time display settings", () => { + const defaults = { timeFormat: "auto", dateFormat: "auto", showRelativeTimes: true } + + /** Saves through the real `updateSettings` handler, as the settings view does. */ + const save = async (updatedSettings: Record) => { + await provider.resolveWebviewView(mockWebviewView) + const messageHandler = vi.mocked(mockWebviewView.webview.onDidReceiveMessage).mock.calls[0][0] + await messageHandler({ type: "updateSettings", updatedSettings }) + } + + test.each(Object.entries(defaults))("%s is %j when it was never saved", async (key, expected) => { + await provider.resolveWebviewView(mockWebviewView) + + expect(await provider.getState()).toMatchObject({ [key]: expected }) + expect(await provider.getStateToPostToWebview()).toMatchObject({ [key]: expected }) + }) + + test.each([ + ["timeFormat", "12h"], + ["timeFormat", "24h"], + ["timeFormat", "auto"], + ["dateFormat", "mdy"], + ["dateFormat", "dmy"], + ["dateFormat", "ymd"], + ["dateFormat", "auto"], + ["showRelativeTimes", true], + ["showRelativeTimes", false], + ])("a saved %s of %j is stored and returned to the webview", async (key, value) => { + mockPostMessage.mockClear() + await save({ [key]: value }) + + expect(mockContext.globalState.update).toHaveBeenCalledWith(key, value) + expect(await provider.getState()).toMatchObject({ [key]: value }) + expect(await provider.getStateToPostToWebview()).toMatchObject({ [key]: value }) + // The open webview is told, so the saved value does not revert on screen. + expect(mockPostMessage).toHaveBeenCalledWith( + expect.objectContaining({ type: "state", state: expect.objectContaining({ [key]: value }) }), + ) + }) + + test("the state refresh after a save reaches the active webview with the saved time format", async () => { + mockPostMessage.mockClear() + + await save({ timeFormat: "24h" }) + + const posted: unknown[][] = mockPostMessage.mock.calls + const states = posted + .map(([message]) => message as { type?: string; state?: { timeFormat?: string } }) + .filter((message) => message.type === "state") + expect(states.length).toBeGreaterThan(0) + expect(states.at(-1)?.state?.timeFormat).toBe("24h") + }) + + test("an explicit false is kept, not replaced by the default of true", async () => { + await save({ showRelativeTimes: false }) + + expect((await provider.getState()).showRelativeTimes).toBe(false) + expect((await provider.getStateToPostToWebview()).showRelativeTimes).toBe(false) + }) + + test("all three are saved together and returned together", async () => { + await save({ timeFormat: "24h", dateFormat: "dmy", showRelativeTimes: false }) + + const expected = { timeFormat: "24h", dateFormat: "dmy", showRelativeTimes: false } + expect(await provider.getState()).toMatchObject(expected) + expect(await provider.getStateToPostToWebview()).toMatchObject(expected) + }) + + test("saving one setting leaves the others at their defaults", async () => { + await save({ timeFormat: "12h" }) + + expect(await provider.getStateToPostToWebview()).toMatchObject({ ...defaults, timeFormat: "12h" }) + }) + }) + test("getState returns the saved allowed write files", async () => { await provider.contextProxy.setValue("allowedWriteFiles", ["notes.md"]) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 685a7b11c1..03c6c2ced8 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -82,6 +82,7 @@ import * as vsCodeLmProvider from "../../../api/providers/vscode-lm" import * as copilotProvider from "../../../api/providers/github-copilot" import { webviewMessageHandler } from "../webviewMessageHandler" +import { settingWriteBookkeepingSize } from "../../config/settingWrites" import { registerCopilotModelBroadcast } from "../../../services/language-models/copilotModelBroadcast" import type { ClineProvider } from "../ClineProvider" import { flushModels, getModels } from "../../../api/providers/fetchers/modelCache" @@ -772,7 +773,7 @@ describe("webviewMessageHandler - theme fixture probes", () => { }) }) -import { t } from "../../../i18n" +import { changeLanguage, t } from "../../../i18n" vi.mock("vscode", () => { const showInformationMessage = vi.fn() @@ -801,6 +802,7 @@ vi.mock("vscode", () => { }) vi.mock("../../../i18n", () => ({ + changeLanguage: vi.fn(), t: vi.fn((key: string, args?: Record) => { // For the delete confirmation with rules, we need to return the interpolated string if (key === "common:confirmation.delete_custom_mode_with_rules" && args) { @@ -1853,6 +1855,487 @@ describe("webviewMessageHandler - message dialog preferences", () => { }) }) +describe("webviewMessageHandler - the display settings are saved all-or-nothing", () => { + const stored: Record = {} + + type WriteHook = (key: string, value: unknown) => Promise | void + + /** + * Makes the write of one key fail. `beforeThrow` can hold the failure back (to let another save run) or + * change what is stored, as a store whose cache was already updated when the write failed. + */ + const failWriteOf = (key: string, value: unknown, beforeThrow?: WriteHook) => { + const real = vi.mocked(mockClineProvider.contextProxy.setValue).getMockImplementation()! + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (k: string, v: unknown) => { + if (k === key && v === value) { + await beforeThrow?.(k, v) + throw new Error("storage unavailable") + } + return real(k as never, v as never) + }) + } + + const save = (updatedSettings: Record) => + webviewMessageHandler(mockClineProvider, { type: "updateSettings", updatedSettings }) + + beforeEach(() => { + vi.clearAllMocks() + for (const key of Object.keys(stored)) delete stored[key] + Object.assign(stored, { timeFormat: "auto", dateFormat: "auto", soundEnabled: false }) + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => stored[key] as never) + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (key: string, value: unknown) => { + stored[key] = value + }) + vi.mocked(mockClineProvider.postStateToWebview).mockResolvedValue(undefined as never) + }) + + it("stores every setting and refreshes the webview", async () => { + await save({ timeFormat: "24h", dateFormat: "ymd", showRelativeTimes: false }) + + expect(stored).toMatchObject({ timeFormat: "24h", dateFormat: "ymd", showRelativeTimes: false }) + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("puts back the values already stored when a later one fails, and reports it", async () => { + failWriteOf("showRelativeTimes", false) + + await save({ timeFormat: "24h", dateFormat: "ymd", showRelativeTimes: false }) + + expect(stored).toMatchObject({ timeFormat: "auto", dateFormat: "auto" }) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.settings_save_failed") + expect(mockClineProvider.log).toHaveBeenCalledWith(expect.stringContaining("Save failed")) + // The webview is re-synced with what is really stored: after the values were put back, not before. + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + const lastWrite = Math.max(...vi.mocked(mockClineProvider.contextProxy.setValue).mock.invocationCallOrder) + expect(vi.mocked(mockClineProvider.postStateToWebview).mock.invocationCallOrder[0]).toBeGreaterThan(lastWrite) + }) + + it("puts back the value whose own write failed after it had changed the stored value", async () => { + // As a store whose cache is updated before its write to disk fails. + failWriteOf("dateFormat", "dmy", (key, value) => { + stored[key] = value + }) + + await save({ dateFormat: "dmy" }) + + expect(stored.dateFormat).toBe("auto") + }) + + it("restores in reverse order", async () => { + const order: string[] = [] + const real = vi.mocked(mockClineProvider.contextProxy.setValue).getMockImplementation()! + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (key: string, value: unknown) => { + order.push(key) + if (key === "showRelativeTimes" && value === false) throw new Error("boom") + return real(key as never, value as never) + }) + + await save({ timeFormat: "24h", dateFormat: "ymd", showRelativeTimes: false }) + + // Three writes forward (the last one fails and never changed the stored value, so there is nothing to put + // back for it), then the restores in the opposite order. + expect(order).toEqual(["timeFormat", "dateFormat", "showRelativeTimes", "dateFormat", "timeFormat"]) + }) + + it("clears a value again that had no stored value before the save", async () => { + failWriteOf("dateFormat", "ymd") + + await save({ showRelativeTimes: false, dateFormat: "ymd" }) + + expect(stored.showRelativeTimes).toBeUndefined() + }) + + describe("the display language is validated before it is applied or stored", () => { + it.each([["xx-not-a-language"], ["en-US-garbage!"], [42], [""], [{}]])( + "refuses language = %j, and applies and stores nothing", + async (value) => { + await save({ language: value, soundEnabled: true }) + + expect(changeLanguage).not.toHaveBeenCalled() + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalled() + expect(stored.soundEnabled).toBe(false) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.settings_save_failed") + }, + ) + + it.each([["de"], ["ja"], ["zh-TW"], [undefined], [null]])("accepts language = %j", async (value) => { + await save({ language: value }) + + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + expect(changeLanguage).toHaveBeenCalledWith(value ?? "en") + expect(stored.language).toBe(value ?? "en") + }) + }) + + describe("the values are validated before anything is stored", () => { + it.each([ + ["timeFormat", "13h"], + ["timeFormat", 24], + ["dateFormat", "dd/mm"], + ["showRelativeTimes", "yes"], + ])("refuses %s = %j, and stores nothing, not even the valid values sent with it", async (key, value) => { + await save({ [key]: value, soundEnabled: true }) + + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalled() + expect(stored).toMatchObject({ timeFormat: "auto", dateFormat: "auto", soundEnabled: false }) + }) + + it.each([ + ["timeFormat", "12h"], + ["dateFormat", "mdy"], + ["showRelativeTimes", true], + ["timeFormat", undefined], + ])("accepts %s = %j", async (key, value) => { + await save({ [key]: value }) + + expect(stored[key]).toBe(value) + }) + }) + + describe("writes to one setting are ordered, down to durable storage", () => { + const slowStore = () => { + const events: string[] = [] + const gates = new Map void>() + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation( + async (key: string, value: unknown) => { + events.push(`start ${key}=${String(value)}`) + await new Promise((resolve) => gates.set(`${key}=${String(value)}`, resolve)) + stored[key] = value + events.push(`end ${key}=${String(value)}`) + }, + ) + const release = async (what: string) => { + await vi.waitFor(() => expect(gates.has(what)).toBe(true)) + gates.get(what)!() + gates.delete(what) + } + return { events, release } + } + + it("does not start a newer save's write of a key before the older one has reached storage", async () => { + const { events, release } = slowStore() + + const a = save({ timeFormat: "12h" }) + await vi.waitFor(() => expect(events).toContain("start timeFormat=12h")) + const b = save({ timeFormat: "24h" }) + await new Promise((resolve) => setTimeout(resolve, 20)) + expect(events).not.toContain("start timeFormat=24h") + + await release("timeFormat=12h") + await release("timeFormat=24h") + await Promise.all([a, b]) + + expect(events).toEqual([ + "start timeFormat=12h", + "end timeFormat=12h", + "start timeFormat=24h", + "end timeFormat=24h", + ]) + expect(stored.timeFormat).toBe("24h") + }) + + it("does not make writes of different keys wait for each other", async () => { + const { events, release } = slowStore() + + const a = save({ timeFormat: "12h" }) + await vi.waitFor(() => expect(events).toContain("start timeFormat=12h")) + const b = save({ dateFormat: "ymd" }) + await vi.waitFor(() => expect(events).toContain("start dateFormat=ymd")) + + await release("dateFormat=ymd") + await release("timeFormat=12h") + await Promise.all([a, b]) + }) + + it("a failed save restores what the save before it wrote, even if that was still queued when it registered", async () => { + const { events, release } = slowStore() + + const q = save({ timeFormat: "12h" }) + await vi.waitFor(() => expect(events).toContain("start timeFormat=12h")) + const p = save({ timeFormat: "24h" }) + await new Promise((resolve) => setTimeout(resolve, 10)) + failWriteOf("soundEnabled", true) + const a = save({ timeFormat: "auto", soundEnabled: true }) + await new Promise((resolve) => setTimeout(resolve, 10)) + + await release("timeFormat=12h") + await release("timeFormat=24h") + await release("timeFormat=auto") + // A fails on soundEnabled, and its restore of timeFormat is a write too. + await release("timeFormat=24h") + await Promise.all([q, p, a.catch(() => undefined)]) + + // P succeeded, so A's failure puts back P's value, not Q's. + expect(stored.timeFormat).toBe("24h") + }) + }) + + describe("nothing is kept about writes once the saves are over", () => { + const settled = () => new Promise((resolve) => setTimeout(resolve, 0)) + + it("after a save that succeeded", async () => { + await save({ timeFormat: "24h", dateFormat: "ymd" }) + await settled() + + expect(settingWriteBookkeepingSize()).toEqual({ latestWrites: 0, queuedKeys: 0 }) + }) + + it("after a save that failed and was rolled back", async () => { + failWriteOf("showRelativeTimes", false) + + await save({ timeFormat: "24h", showRelativeTimes: false }) + await settled() + + expect(settingWriteBookkeepingSize()).toEqual({ latestWrites: 0, queuedKeys: 0 }) + }) + }) + + describe("two saves at once", () => { + it("a failed save does not undo another save that wrote the same value", async () => { + let releaseFailure!: () => void + const held = new Promise((resolve) => (releaseFailure = resolve)) + failWriteOf("dateFormat", "ymd", () => held) + + // Save A stores timeFormat 24h, then waits at its failing write. + const a = save({ timeFormat: "24h", dateFormat: "ymd" }) + await vi.waitFor(() => expect(stored.timeFormat).toBe("24h")) + + // Save B stores the same value for the same key, and finishes. + await save({ timeFormat: "24h" }) + + // Save A now fails and rolls back. B's write of the same value must stand. + releaseFailure() + await a + + expect(stored.timeFormat).toBe("24h") + expect(vscode.window.showErrorMessage).toHaveBeenCalledTimes(1) + }) + + it("a failed save does not undo another save's different value", async () => { + let releaseFailure!: () => void + const held = new Promise((resolve) => (releaseFailure = resolve)) + failWriteOf("dateFormat", "ymd", () => held) + + const a = save({ timeFormat: "24h", dateFormat: "ymd" }) + await vi.waitFor(() => expect(stored.timeFormat).toBe("24h")) + await save({ timeFormat: "12h" }) + + releaseFailure() + await a + + expect(stored.timeFormat).toBe("12h") + }) + + /** Two saves that both write the same key and then fail, each held at its own failing write. */ + const twoFailingSaves = async (releaseFirst: "a" | "b") => { + const gates: Record<"a" | "b", () => void> = { a: () => {}, b: () => {} } + const heldA = new Promise((resolve) => (gates.a = resolve)) + const heldB = new Promise((resolve) => (gates.b = resolve)) + failWriteOf("dateFormat", "ymd", () => heldA) + failWriteOf("showRelativeTimes", false, () => heldB) + + const a = save({ timeFormat: "12h", dateFormat: "ymd" }) + await vi.waitFor(() => expect(stored.timeFormat).toBe("12h")) + const b = save({ timeFormat: "24h", showRelativeTimes: false }) + await vi.waitFor(() => expect(stored.timeFormat).toBe("24h")) + + const [first, second] = releaseFirst === "a" ? (["a", "b"] as const) : (["b", "a"] as const) + gates[first]() + await (first === "a" ? a : b) + const afterFirst = stored.timeFormat + gates[second]() + await (second === "a" ? a : b) + return { afterFirst, final: stored.timeFormat } + } + + it("puts back the value from before both saves when both fail, the earlier one failing first", async () => { + const { afterFirst, final } = await twoFailingSaves("a") + + // The later save is still pending with 24h, so the first failure leaves it. Then it restores "auto". + expect(afterFirst).toBe("24h") + expect(final).toBe("auto") + }) + + it("puts back the value from before both saves when both fail, the later one failing first", async () => { + const { afterFirst, final } = await twoFailingSaves("b") + + // The later save restores the earlier one's pending value, and the earlier one then restores "auto". + expect(afterFirst).toBe("12h") + expect(final).toBe("auto") + }) + + it("puts back the original value when three overlapping saves all fail, the middle one first", async () => { + const release: Record<"a" | "b" | "c", () => void> = { a: () => {}, b: () => {}, c: () => {} } + const gate = (name: "a" | "b" | "c") => new Promise((resolve) => (release[name] = resolve)) + const gates = { a: gate("a"), b: gate("b"), c: gate("c") } + // Each save fails on a setting that is not tracked, so a held failure does not hold up the other saves' + // writes of the display settings, which are queued per key. + failWriteOf("soundEnabled", true, () => gates.a) + failWriteOf("soundVolume", 0.5, () => gates.b) + failWriteOf("ttsEnabled", true, () => gates.c) + + const a = save({ timeFormat: "12h", soundEnabled: true }) + await vi.waitFor(() => expect(stored.timeFormat).toBe("12h")) + const b = save({ timeFormat: "24h", soundVolume: 0.5 }) + await vi.waitFor(() => expect(stored.timeFormat).toBe("24h")) + const c = save({ timeFormat: "auto", ttsEnabled: true }) + await vi.waitFor(() => + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("timeFormat", "auto"), + ) + + release.b() + await b.catch(() => undefined) // a failed write of a setting that is not tracked is rethrown + expect(stored.timeFormat).toBe("auto") // the newest is still pending + + release.c() + await c.catch(() => undefined) // a failed write of a setting that is not tracked is rethrown + expect(stored.timeFormat).toBe("12h") // the oldest save's pending value, not the middle one's + + release.a() + await a.catch(() => undefined) // a failed write of a setting that is not tracked is rethrown + expect(stored.timeFormat).toBe("auto") // from before all three + }) + + it("keeps the earlier save's value when the later one fails and the earlier one succeeds", async () => { + let releaseFailure!: () => void + const held = new Promise((resolve) => (releaseFailure = resolve)) + failWriteOf("showRelativeTimes", false, () => held) + + const later = (async () => { + await vi.waitFor(() => expect(stored.timeFormat).toBe("12h")) + return save({ timeFormat: "24h", showRelativeTimes: false }) + })() + await save({ timeFormat: "12h" }) + await vi.waitFor(() => expect(stored.timeFormat).toBe("24h")) + + releaseFailure() + await later + + expect(stored.timeFormat).toBe("12h") + }) + + it("keeps a value changed outside a save, such as an import, when a failed save rolls back", async () => { + failWriteOf("dateFormat", "ymd", () => { + stored.timeFormat = "12h" // an import writes the same key meanwhile + }) + + await save({ timeFormat: "24h", dateFormat: "ymd" }) + + expect(stored.timeFormat).toBe("12h") + }) + + it("the failed save still puts back the keys the other save did not touch", async () => { + let releaseFailure!: () => void + const held = new Promise((resolve) => (releaseFailure = resolve)) + failWriteOf("dateFormat", "ymd", () => held) + + const a = save({ timeFormat: "24h", dateFormat: "ymd" }) + await vi.waitFor(() => expect(stored.timeFormat).toBe("24h")) + await save({ showRelativeTimes: false }) + + releaseFailure() + await a + + expect(stored.timeFormat).toBe("auto") // A's own write is put back + expect(stored.showRelativeTimes).toBe(false) // B's is untouched + }) + }) + + it("keeps going when a restore itself fails, and says which one", async () => { + const real = vi.mocked(mockClineProvider.contextProxy.setValue).getMockImplementation()! + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (key: string, value: unknown) => { + if (key === "dateFormat" && value === "ymd") throw new Error("dateFormat is broken") + if (key === "timeFormat" && value === "auto") throw new Error("cannot restore") + return real(key as never, value as never) + }) + + await save({ timeFormat: "24h", dateFormat: "ymd" }) + + expect(mockClineProvider.log).toHaveBeenCalledWith(expect.stringContaining("Could not restore timeFormat")) + expect(vscode.window.showErrorMessage).toHaveBeenCalled() + }) + + it("reports a restore failure and a save failure that are not Errors", async () => { + const real = vi.mocked(mockClineProvider.contextProxy.setValue).getMockImplementation()! + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (key: string, value: unknown) => { + if (key === "dateFormat" && value === "ymd") throw "plain save failure" + if (key === "timeFormat" && value === "auto") throw "plain restore failure" + return real(key as never, value as never) + }) + + await save({ timeFormat: "24h", dateFormat: "ymd" }) + + expect(mockClineProvider.log).toHaveBeenCalledWith(expect.stringContaining("plain restore failure")) + expect(mockClineProvider.log).toHaveBeenCalledWith(expect.stringContaining("plain save failure")) + }) + + it("leaves every other setting as it was written: it is not put back when a later write fails", async () => { + failWriteOf("dateFormat", "ymd") + + await save({ soundEnabled: true, dateFormat: "ymd" }) + + expect(stored.soundEnabled).toBe(true) + expect(vscode.window.showErrorMessage).toHaveBeenCalled() + }) + + describe("a failure while writing another setting", () => { + it("puts the display settings back, refreshes the webview, and then follows the existing error path", async () => { + failWriteOf("soundEnabled", true) + + await expect(save({ timeFormat: "24h", soundEnabled: true })).rejects.toThrow("storage unavailable") + + expect(stored.timeFormat).toBe("auto") + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + // No toast: the existing path did not show one for these settings. + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("does not reject for a failure of a display setting, which is reported to the user instead", async () => { + failWriteOf("timeFormat", "24h") + + await expect(save({ timeFormat: "24h" })).resolves.toBeUndefined() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.settings_save_failed") + }) + }) + + it("does nothing when there are no settings to save", async () => { + await webviewMessageHandler(mockClineProvider, { type: "updateSettings" }) + + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalled() + expect(mockClineProvider.postStateToWebview).not.toHaveBeenCalled() + }) + + describe("when the state refresh fails", () => { + it("keeps a stored save and does not report a failure", async () => { + vi.mocked(mockClineProvider.postStateToWebview).mockRejectedValueOnce(new Error("state unavailable")) + + await save({ timeFormat: "24h", dateFormat: "ymd" }) + + expect(stored).toMatchObject({ timeFormat: "24h", dateFormat: "ymd" }) + expect(mockClineProvider.log).toHaveBeenCalledWith(expect.stringContaining("state unavailable")) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("logs a refresh failure that is not an Error", async () => { + vi.mocked(mockClineProvider.postStateToWebview).mockRejectedValueOnce("plain refresh failure") + + await save({ timeFormat: "24h" }) + + expect(mockClineProvider.log).toHaveBeenCalledWith(expect.stringContaining("plain refresh failure")) + }) + + it("still reports a failed save", async () => { + failWriteOf("timeFormat", "24h") + vi.mocked(mockClineProvider.postStateToWebview).mockRejectedValueOnce(new Error("state unavailable")) + + await save({ timeFormat: "24h" }) + + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.settings_save_failed") + }) + }) +}) + describe("webviewMessageHandler - mcpEnabled", () => { let mockMcpHub: any diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index bd7c589958..03b4bd942b 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -29,6 +29,7 @@ import { OpenAiModelsMessageType, RouterModelsMessageType, VsCodeLmModelsMessageType, + globalSettingsSchema, isTelemetryOptedIn, } from "@roo-code/types" import { customToolRegistry } from "@roo-code/core" @@ -37,6 +38,14 @@ import { TelemetryService } from "@roo-code/telemetry" import { type ApiMessage } from "../task-persistence/apiMessages" import { saveTaskMessages } from "../task-persistence" +import { + beginSettingWrite, + finishSettingWrites, + isTrackedSetting, + performSettingWrite, + undoSettingWrite, + type SettingWrite, +} from "../config/settingWrites" import { importRooTaskHistory } from "../task-persistence/importRooTaskHistory" import { ClineProvider } from "./ClineProvider" @@ -130,6 +139,18 @@ import { const handlerFor = (apiConfiguration: ProviderSettings, provider: RouterName): ApiHandler => buildApiHandler({ ...apiConfiguration, apiProvider: provider }) +/** + * What the webview may send for each display value (and the display language): the schema the setting is defined with. The webview is not + * trusted, and its own controls do not bind a message that was written by hand. + */ +const DISPLAY_VALUE_SCHEMAS = { + timeFormat: globalSettingsSchema.shape.timeFormat, + dateFormat: globalSettingsSchema.shape.dateFormat, + showRelativeTimes: globalSettingsSchema.shape.showRelativeTimes, + // The display language, which the formatters take their locale from. Null or unset means the default. + language: globalSettingsSchema.shape.language.nullish(), +} + export const webviewMessageHandler = async ( provider: ClineProvider, message: WebviewMessage, @@ -737,8 +758,31 @@ export const webviewMessageHandler = async ( } break - case "updateSettings": - if (message.updatedSettings) { + case "updateSettings": { + // The display settings are saved all-or-nothing: if a write fails, the ones this save already stored are + // put back. A key is recorded before it is written, so a write that failed after changing the stored + // value is put back too (see `undoSettingWrite` for when a key is left alone). A failure while writing + // any other setting is not handled here: it follows the existing error path, after the display + // settings have been put back. + const written: SettingWrite[] = [] + let failedKey: string | undefined + let unhandledFailure: unknown + + try { + if (!message.updatedSettings) break + + // Checked before anything else happens, so an invalid value changes nothing: no side effect runs and + // no setting is stored, not even the valid ones sent with it. + for (const [key, schema] of Object.entries(DISPLAY_VALUE_SCHEMAS)) { + if ( + key in message.updatedSettings && + !schema.safeParse(Reflect.get(message.updatedSettings, key)).success + ) { + failedKey = key + throw new Error(`Invalid value for ${key}`) + } + } + if (message.updatedSettings.destructiveCommandGuardEnabled === true) { try { const { ensureDcgInstalled } = await import("../../services/destructive-command-guard") @@ -758,6 +802,8 @@ export const webviewMessageHandler = async ( } for (const [key, value] of Object.entries(message.updatedSettings)) { + failedKey = key + const previous = provider.contextProxy.getValue(key as keyof RooCodeSettings) let newValue = value if (key === "language") { @@ -867,13 +913,53 @@ export const webviewMessageHandler = async ( } } + if (isTrackedSetting(key)) { + const write = beginSettingWrite(key, previous, newValue) + written.push(write) + await performSettingWrite(provider.contextProxy, write) + continue + } + await provider.contextProxy.setValue(key as keyof RooCodeSettings, newValue) } + finishSettingWrites(written) + } catch (error) { + const failure = error instanceof Error ? error.message : String(error) + + for (const write of written.reverse()) { + try { + await undoSettingWrite(provider.contextProxy, write) + } catch (rollbackError) { + provider.log( + `[updateSettings] Could not restore ${write.key}: ${rollbackError instanceof Error ? rollbackError.message : String(rollbackError)}`, + ) + } + } + + provider.log(`[updateSettings] Save failed: ${failure}`) + + if (failedKey !== undefined && (isTrackedSetting(failedKey) || failedKey === "language")) { + vscode.window.showErrorMessage(t("common:errors.settings_save_failed", { error: failure })) + } else { + // Not a display setting: handled as it always was, once the webview has been refreshed. + unhandledFailure = error + } + } + + // The outcome is decided above. A failed refresh must not undo a stored save. + try { await provider.postStateToWebview() + } catch (error) { + provider.log( + `[updateSettings] Could not refresh the webview state: ${error instanceof Error ? error.message : String(error)}`, + ) } + if (unhandledFailure !== undefined) throw unhandledFailure + break + } case "terminalOperation": if (message.terminalOperation) { diff --git a/src/i18n/locales/ca/common.json b/src/i18n/locales/ca/common.json index 579cd6b7d7..16561c5bf7 100644 --- a/src/i18n/locales/ca/common.json +++ b/src/i18n/locales/ca/common.json @@ -62,6 +62,7 @@ "cannot_access_path": "No es pot accedir a la ruta {{path}}: {{error}}", "path_outside_workspace": "La ruta és fora de l'espai de treball", "settings_import_failed": "Ha fallat la importació de la configuració: {{error}}.", + "settings_save_failed": "No s'ha pogut desar la configuració. Alguns valors poden haver quedat parcialment canviats: {{error}}", "mistake_limit_guidance": "Això pot indicar un error en el procés de pensament del model o la incapacitat d'utilitzar una eina correctament, que es pot mitigar amb orientació de l'usuari (p. ex. \"Prova de dividir la tasca en passos més petits\").", "violated_organization_allowlist": "Ha fallat l'execució de la tasca: el perfil actual no és compatible amb la configuració de la teva organització", "condense_failed": "Ha fallat la condensació del context", diff --git a/src/i18n/locales/de/common.json b/src/i18n/locales/de/common.json index 3d8175a77e..4eac519f9b 100644 --- a/src/i18n/locales/de/common.json +++ b/src/i18n/locales/de/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Zugriff auf Pfad {{path}} nicht möglich: {{error}}", "path_outside_workspace": "Pfad liegt außerhalb des Arbeitsbereichs", "settings_import_failed": "Fehler beim Importieren der Einstellungen: {{error}}.", + "settings_save_failed": "Die Einstellungen konnten nicht gespeichert werden. Einige könnten teilweise geändert geblieben sein: {{error}}", "mistake_limit_guidance": "Dies kann auf einen Fehler im Denkprozess des Modells oder die Unfähigkeit hinweisen, ein Tool richtig zu verwenden, was durch Benutzerführung behoben werden kann (z.B. \"Versuche, die Aufgabe in kleinere Schritte zu unterteilen\").", "violated_organization_allowlist": "Aufgabe konnte nicht ausgeführt werden: Das aktuelle Profil ist nicht kompatibel mit den Einstellungen deiner Organisation", "condense_failed": "Fehler beim Verdichten des Kontexts", diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index a7d616028f..ea4ca74c7d 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Cannot access path {{path}}: {{error}}", "path_outside_workspace": "Path is outside the workspace", "settings_import_failed": "Settings import failed: {{error}}.", + "settings_save_failed": "Settings could not be saved. Some may be left partly changed: {{error}}", "mistake_limit_guidance": "This may indicate a failure in the model's thought process or inability to use a tool properly, which can be mitigated with some user guidance (e.g. \"Try breaking down the task into smaller steps\").", "violated_organization_allowlist": "Failed to run task: the current profile isn't compatible with your organization settings", "condense_failed": "Failed to condense context", diff --git a/src/i18n/locales/es/common.json b/src/i18n/locales/es/common.json index 2eb6848f11..4f4b6fd4c6 100644 --- a/src/i18n/locales/es/common.json +++ b/src/i18n/locales/es/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "No se puede acceder a la ruta {{path}}: {{error}}", "path_outside_workspace": "La ruta está fuera del espacio de trabajo", "settings_import_failed": "Error al importar la configuración: {{error}}.", + "settings_save_failed": "No se pudieron guardar los ajustes. Algunos pueden haber quedado parcialmente modificados: {{error}}", "mistake_limit_guidance": "Esto puede indicar un fallo en el proceso de pensamiento del modelo o la incapacidad de usar una herramienta correctamente, lo cual puede mitigarse con orientación del usuario (ej. \"Intenta dividir la tarea en pasos más pequeños\").", "violated_organization_allowlist": "Error al ejecutar la tarea: el perfil actual no es compatible con la configuración de tu organización", "condense_failed": "Error al condensar el contexto", diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index b6b65a88de..dcdb3c1dd3 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Impossible d'accéder au chemin {{path}} : {{error}}", "path_outside_workspace": "Le chemin est à l'extérieur de l'espace de travail", "settings_import_failed": "Échec de l'importation des paramètres : {{error}}", + "settings_save_failed": "Impossible d'enregistrer les paramètres. Certains peuvent être restés partiellement modifiés : {{error}}", "mistake_limit_guidance": "Cela peut indiquer un échec dans le processus de réflexion du modèle ou une incapacité à utiliser un outil correctement, ce qui peut être atténué avec des conseils de l'utilisateur (par ex. \"Essaie de diviser la tâche en étapes plus petites\").", "violated_organization_allowlist": "Échec de l'exécution de la tâche : le profil actuel n'est pas compatible avec les paramètres de votre organisation", "condense_failed": "Échec de la condensation du contexte", diff --git a/src/i18n/locales/hi/common.json b/src/i18n/locales/hi/common.json index f69b5adccf..d4572871f7 100644 --- a/src/i18n/locales/hi/common.json +++ b/src/i18n/locales/hi/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "पाथ {{path}} तक पहुंच नहीं पा रहे हैं: {{error}}", "path_outside_workspace": "पथ वर्कस्पेस से बाहर है", "settings_import_failed": "सेटिंग्स इम्पोर्ट करने में विफल: {{error}}।", + "settings_save_failed": "सेटिंग्स सहेजी नहीं जा सकीं। कुछ सेटिंग्स आंशिक रूप से बदली रह सकती हैं: {{error}}", "mistake_limit_guidance": "यह मॉडल की सोच प्रक्रिया में विफलता या किसी टूल का सही उपयोग न कर पाने का संकेत हो सकता है, जिसे उपयोगकर्ता के मार्गदर्शन से ठीक किया जा सकता है (जैसे \"कार्य को छोटे चरणों में बांटने की कोशिश करें\")।", "violated_organization_allowlist": "कार्य चलाने में विफल: वर्तमान प्रोफ़ाइल आपके संगठन की सेटिंग्स के साथ संगत नहीं है", "condense_failed": "संदर्भ को संक्षिप्त करने में विफल", diff --git a/src/i18n/locales/id/common.json b/src/i18n/locales/id/common.json index e401612bf3..85abdfaa8b 100644 --- a/src/i18n/locales/id/common.json +++ b/src/i18n/locales/id/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Tidak dapat mengakses path {{path}}: {{error}}", "path_outside_workspace": "Path berada di luar workspace", "settings_import_failed": "Impor pengaturan gagal: {{error}}.", + "settings_save_failed": "Pengaturan tidak dapat disimpan. Sebagian mungkin tetap berubah sebagian: {{error}}", "mistake_limit_guidance": "Ini mungkin menunjukkan kegagalan dalam proses pemikiran model atau ketidakmampuan untuk menggunakan tool dengan benar, yang dapat diatasi dengan beberapa panduan pengguna (misalnya \"Coba bagi tugas menjadi langkah-langkah yang lebih kecil\").", "violated_organization_allowlist": "Gagal menjalankan tugas: profil saat ini tidak kompatibel dengan pengaturan organisasi kamu", "condense_failed": "Gagal mengompres konteks", diff --git a/src/i18n/locales/it/common.json b/src/i18n/locales/it/common.json index aeb984940a..2f570174c6 100644 --- a/src/i18n/locales/it/common.json +++ b/src/i18n/locales/it/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Impossibile accedere al percorso {{path}}: {{error}}", "path_outside_workspace": "Il percorso è fuori dall'area di lavoro", "settings_import_failed": "Importazione delle impostazioni fallita: {{error}}.", + "settings_save_failed": "Impossibile salvare le impostazioni. Alcune potrebbero essere rimaste modificate in parte: {{error}}", "mistake_limit_guidance": "Questo può indicare un fallimento nel processo di pensiero del modello o l'incapacità di utilizzare correttamente uno strumento, che può essere mitigato con la guida dell'utente (ad es. \"Prova a suddividere l'attività in passaggi più piccoli\").", "violated_organization_allowlist": "Impossibile eseguire l'attività: il profilo corrente non è compatibile con le impostazioni della tua organizzazione", "condense_failed": "Impossibile condensare il contesto", diff --git a/src/i18n/locales/ja/common.json b/src/i18n/locales/ja/common.json index a7ec0c4de3..2fb1c19478 100644 --- a/src/i18n/locales/ja/common.json +++ b/src/i18n/locales/ja/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "パス {{path}} にアクセスできません：{{error}}", "path_outside_workspace": "パスはワークスペースの外にあります", "settings_import_failed": "設定のインポートに失敗しました：{{error}}", + "settings_save_failed": "設定を保存できませんでした。一部の設定が部分的に変更されたままになっている可能性があります：{{error}}", "mistake_limit_guidance": "これは、モデルの思考プロセスの失敗やツールを適切に使用できないことを示している可能性があり、ユーザーのガイダンスによって軽減できます（例：「タスクをより小さなステップに分割してみてください」）。", "violated_organization_allowlist": "タスクの実行に失敗しました: 現在のプロファイルは組織の設定と互換性がありません", "condense_failed": "コンテキストの圧縮に失敗しました", diff --git a/src/i18n/locales/ko/common.json b/src/i18n/locales/ko/common.json index 906652ac14..32f003633a 100644 --- a/src/i18n/locales/ko/common.json +++ b/src/i18n/locales/ko/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "경로 {{path}}에 접근할 수 없습니다: {{error}}", "path_outside_workspace": "경로가 작업 영역 밖에 있습니다", "settings_import_failed": "설정 가져오기 실패: {{error}}.", + "settings_save_failed": "설정을 저장할 수 없습니다. 일부 설정이 부분적으로 변경된 상태로 남아 있을 수 있습니다: {{error}}", "mistake_limit_guidance": "이는 모델의 사고 과정 실패나 도구를 제대로 사용하지 못하는 것을 나타낼 수 있으며, 사용자 가이드를 통해 완화할 수 있습니다 (예: \"작업을 더 작은 단계로 나누어 시도해보세요\").", "violated_organization_allowlist": "작업 실행 실패: 현재 프로필이 조직 설정과 호환되지 않습니다", "condense_failed": "컨텍스트 압축에 실패했습니다", diff --git a/src/i18n/locales/nl/common.json b/src/i18n/locales/nl/common.json index 34cdbbbe89..823b12b657 100644 --- a/src/i18n/locales/nl/common.json +++ b/src/i18n/locales/nl/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Kan pad {{path}} niet openen: {{error}}", "path_outside_workspace": "Pad is buiten de werkomgeving", "settings_import_failed": "Importeren van instellingen mislukt: {{error}}.", + "settings_save_failed": "De instellingen konden niet worden opgeslagen. Sommige zijn mogelijk gedeeltelijk gewijzigd gebleven: {{error}}", "mistake_limit_guidance": "Dit kan duiden op een fout in het denkproces van het model of het onvermogen om een tool correct te gebruiken, wat kan worden verminderd met gebruikersbegeleiding (bijv. \"Probeer de taak op te delen in kleinere stappen\").", "violated_organization_allowlist": "Taak uitvoeren mislukt: het huidige profiel is niet compatibel met de instellingen van uw organisatie", "condense_failed": "Comprimeren van context mislukt", diff --git a/src/i18n/locales/pl/common.json b/src/i18n/locales/pl/common.json index 2a743fc009..9f4b955cab 100644 --- a/src/i18n/locales/pl/common.json +++ b/src/i18n/locales/pl/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Nie można uzyskać dostępu do ścieżki {{path}}: {{error}}", "path_outside_workspace": "Ścieżka znajduje się poza obszarem roboczym", "settings_import_failed": "Nie udało się zaimportować ustawień: {{error}}.", + "settings_save_failed": "Nie udało się zapisać ustawień. Niektóre mogły pozostać częściowo zmienione: {{error}}", "mistake_limit_guidance": "To może wskazywać na błąd w procesie myślowym modelu lub niezdolność do prawidłowego użycia narzędzia, co można złagodzić poprzez wskazówki użytkownika (np. \"Spróbuj podzielić zadanie na mniejsze kroki\").", "violated_organization_allowlist": "Nie udało się uruchomić zadania: bieżący profil nie jest kompatybilny z ustawieniami Twojej organizacji", "condense_failed": "Nie udało się skondensować kontekstu", diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index eaf1cae06b..451dfa6cfe 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -62,6 +62,7 @@ "cannot_access_path": "Não é possível acessar o caminho {{path}}: {{error}}", "path_outside_workspace": "O caminho está fora do espaço de trabalho", "settings_import_failed": "Falha ao importar configurações: {{error}}", + "settings_save_failed": "Não foi possível salvar as configurações. Algumas podem ter ficado parcialmente alteradas: {{error}}", "mistake_limit_guidance": "Isso pode indicar uma falha no processo de pensamento do modelo ou incapacidade de usar uma ferramenta adequadamente, o que pode ser mitigado com orientação do usuário (ex. \"Tente dividir a tarefa em etapas menores\").", "violated_organization_allowlist": "Falha ao executar a tarefa: o perfil atual não é compatível com as configurações da sua organização", "condense_failed": "Falha ao condensar o contexto", diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index 6f48365ec1..f26457612a 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Невозможно получить доступ к пути {{path}}: {{error}}", "path_outside_workspace": "Путь находится вне рабочего пространства", "settings_import_failed": "Не удалось импортировать настройки: {{error}}.", + "settings_save_failed": "Не удалось сохранить настройки. Некоторые могли остаться частично изменёнными: {{error}}", "mistake_limit_guidance": "Это может указывать на сбой в процессе мышления модели или неспособность правильно использовать инструмент, что можно смягчить с помощью руководства пользователя (например, \"Попробуйте разбить задачу на более мелкие шаги\").", "violated_organization_allowlist": "Не удалось выполнить задачу: текущий профиль несовместим с настройками вашей организации", "condense_failed": "Не удалось сжать контекст", diff --git a/src/i18n/locales/tr/common.json b/src/i18n/locales/tr/common.json index b5c543c405..83d54b62a0 100644 --- a/src/i18n/locales/tr/common.json +++ b/src/i18n/locales/tr/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "{{path}} yoluna erişilemiyor: {{error}}", "path_outside_workspace": "Yol çalışma alanının dışında", "settings_import_failed": "Ayarlar içe aktarılamadı: {{error}}.", + "settings_save_failed": "Ayarlar kaydedilemedi. Bazıları kısmen değişmiş olarak kalmış olabilir: {{error}}", "mistake_limit_guidance": "Bu, modelin düşünce sürecindeki bir başarısızlığı veya bir aracı düzgün kullanamama durumunu gösterebilir, bu da kullanıcı rehberliği ile hafifletilebilir (örn. \"Görevi daha küçük adımlara bölmeyi deneyin\").", "violated_organization_allowlist": "Görev yürütülemedi: Geçerli profil kuruluşunuzun ayarlarıyla uyumlu değil", "condense_failed": "Bağlam sıkıştırılamadı", diff --git a/src/i18n/locales/vi/common.json b/src/i18n/locales/vi/common.json index cb2ed8e9b8..f1bce7743f 100644 --- a/src/i18n/locales/vi/common.json +++ b/src/i18n/locales/vi/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "Không thể truy cập đường dẫn {{path}}: {{error}}", "path_outside_workspace": "Đường dẫn nằm ngoài không gian làm việc", "settings_import_failed": "Nhập cài đặt thất bại: {{error}}.", + "settings_save_failed": "Không thể lưu cài đặt. Một số cài đặt có thể vẫn bị thay đổi một phần: {{error}}", "mistake_limit_guidance": "Điều này có thể cho thấy sự thất bại trong quá trình suy nghĩ của mô hình hoặc không thể sử dụng công cụ đúng cách, có thể được giảm thiểu bằng hướng dẫn của người dùng (ví dụ: \"Hãy thử chia nhỏ nhiệm vụ thành các bước nhỏ hơn\").", "violated_organization_allowlist": "Không thể chạy tác vụ: hồ sơ hiện tại không tương thích với cài đặt của tổ chức của bạn", "condense_failed": "Không thể nén ngữ cảnh", diff --git a/src/i18n/locales/zh-CN/common.json b/src/i18n/locales/zh-CN/common.json index 08cfe736b5..1ddac8f209 100644 --- a/src/i18n/locales/zh-CN/common.json +++ b/src/i18n/locales/zh-CN/common.json @@ -63,6 +63,7 @@ "cannot_access_path": "无法访问路径 {{path}}：{{error}}", "path_outside_workspace": "路径位于工作区之外", "settings_import_failed": "设置导入失败：{{error}}。", + "settings_save_failed": "无法保存设置。部分设置可能仍处于部分更改的状态：{{error}}", "mistake_limit_guidance": "这可能表明模型思维过程失败或无法正确使用工具，可通过用户指导来缓解（例如\"尝试将任务分解为更小的步骤\"）。", "violated_organization_allowlist": "执行任务失败：当前配置文件与您的组织设置不兼容", "condense_failed": "压缩上下文失败", diff --git a/src/i18n/locales/zh-TW/common.json b/src/i18n/locales/zh-TW/common.json index 92fdbaeb96..d78e73aab1 100644 --- a/src/i18n/locales/zh-TW/common.json +++ b/src/i18n/locales/zh-TW/common.json @@ -58,6 +58,7 @@ "cannot_access_path": "無法存取路徑 {{path}}：{{error}}", "path_outside_workspace": "路徑位於工作區之外", "settings_import_failed": "設定匯入失敗：{{error}}。", + "settings_save_failed": "無法儲存設定。部分設定可能仍處於部分變更的狀態：{{error}}", "mistake_limit_guidance": "這可能表明模型思維過程失敗或無法正確使用工具，可透過使用者指導來緩解（例如「嘗試將工作分解為更小的步驟」）。", "violated_organization_allowlist": "執行工作失敗：目前設定檔與您的組織設定不相容", "condense_failed": "壓縮上下文失敗", diff --git a/webview-ui/playwright/gallery/stories.tsx b/webview-ui/playwright/gallery/stories.tsx index 85d6919db4..aef15146a8 100644 --- a/webview-ui/playwright/gallery/stories.tsx +++ b/webview-ui/playwright/gallery/stories.tsx @@ -415,6 +415,11 @@ export const stories: Record = { ) }, + "language-settings": async () => { + const { LanguageSettingsStory } = + await import("@/components/settings/__tests__/LanguageSettings.visual.fixture") + return + }, "ui-settings": async () => { const { UISettingsStory } = await import("@/components/settings/__tests__/UISettings.visual.fixture") return diff --git a/webview-ui/src/__tests__/ContextWindowProgress.spec.tsx b/webview-ui/src/__tests__/ContextWindowProgress.spec.tsx index f5c2ba1062..052d58267e 100644 --- a/webview-ui/src/__tests__/ContextWindowProgress.spec.tsx +++ b/webview-ui/src/__tests__/ContextWindowProgress.spec.tsx @@ -5,8 +5,9 @@ import { providerIdentifiers } from "@roo-code/types" import TaskHeader from "@src/components/chat/TaskHeader" -// Mock formatLargeNumber function -vi.mock("@/utils/format", () => ({ +// Keep the real format helpers (other components in the tree use them) and override only formatLargeNumber. +vi.mock("@/utils/format", async (importOriginal) => ({ + ...(await importOriginal()), formatLargeNumber: vi.fn((num) => num.toString()), })) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 3b6a3ca66b..2040501b3f 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -38,6 +38,7 @@ import WarningRow from "./WarningRow" import McpResourceRow from "../mcp/McpResourceRow" import { Mention } from "./Mention" +import { isShowableTimestamp, MessageTimestamp } from "./MessageTimestamp" import { CheckpointSaved } from "./checkpoints/CheckpointSaved" import { FollowUpSuggest } from "./FollowUpSuggest" import { BatchFilePermission } from "./BatchFilePermission" @@ -231,6 +232,8 @@ export const ChatRowContent = ({ clineMessages, currentTaskItem, enableCheckpoints, + timeFormat, + dateFormat, } = useExtensionState() const { info: model } = useSelectedModel(apiConfiguration) const [isEditing, setIsEditing] = useState(false) @@ -1248,6 +1251,13 @@ export const ChatRowContent = ({ )} + {/* When the reply was sent. Held back while it streams, so the line does not appear and + shift as the text grows. */} + {!message.partial && isShowableTimestamp(message.ts) && ( +
+ +
+ )} ) case "user_feedback": @@ -1323,6 +1333,12 @@ export const ChatRowContent = ({ )} + {/* When the message was sent. Hidden while editing: the edit box replaces the bubble. */} + {!isEditing && isShowableTimestamp(message.ts) && ( +
+ +
+ )} ) case "user_feedback_diff": @@ -1384,6 +1400,12 @@ export const ChatRowContent = ({ ) : null} + {/* When the final result was sent. Held back while it streams, like other replies. */} + {!message.partial && isShowableTimestamp(message.ts) && ( +
+ +
+ )} ) case "shell_integration_warning": diff --git a/webview-ui/src/components/chat/MessageTimestamp.tsx b/webview-ui/src/components/chat/MessageTimestamp.tsx new file mode 100644 index 0000000000..c76ef2e14b --- /dev/null +++ b/webview-ui/src/components/chat/MessageTimestamp.tsx @@ -0,0 +1,51 @@ +import { Clock } from "lucide-react" + +import { cn } from "@src/lib/utils" +import { formatDateTime, formatMessageTime, type DateFormat, type TimeFormat } from "@/utils/format" + +import { StandardTooltip } from "../ui/standard-tooltip" + +interface MessageTimestampProps { + /** Epoch ms the message was sent. */ + ts: number + /** From the user's Language settings; both default to "auto". */ + timeFormat?: TimeFormat + dateFormat?: DateFormat + className?: string +} + +/** + * Whether `ts` is a time that can be shown. Number.isFinite is not enough: a finite value beyond ±8.64e15 ms is an + * Invalid Date, and toISOString() would throw during render and take the whole chat row down with it. Callers that + * wrap the timestamp in spacing use this, so an invalid time leaves no empty block behind. + */ +export const isShowableTimestamp = (ts: number) => + Number.isFinite(ts) && ts > 0 && !Number.isNaN(new Date(ts).getTime()) + +/** + * "🕒 Yesterday 11:43 AM" under a sent message. + * + * Purely presentational: the caller supplies the user's time/date formats, so + * the component has no dependency on extension state and renders anywhere. The + * tooltip always carries the full date and time. A semantic