Skip to content
This repository was archived by the owner on May 15, 2026. It is now read-only.

Commit 6c36806

Browse files
committed
feat: clear settings matching defaults on every startup
This implements 'Option 2' - every-startup clearing of default values. - Add clearDefaultSettings() function that checks all settings in settingDefaults and clears any that exactly match the default - Add runStartupSettingsMaintenance() as the main entry point that runs both migrations (once) and default clearing (every startup) - Update ContextProxy to use runStartupSettingsMaintenance - Add comprehensive tests for the new functionality This ensures users always benefit from default value improvements. Note: Users cannot 'lock in' a value that matches the default.
1 parent 88e9777 commit 6c36806

3 files changed

Lines changed: 207 additions & 9 deletions

File tree

src/core/config/ContextProxy.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { TelemetryService } from "@roo-code/telemetry"
2121

2222
import { logger } from "../../utils/logging"
2323
import { supportPrompt } from "../../shared/support-prompt"
24-
import { runSettingsMigrations } from "../../utils/settingsMigrations"
24+
import { runStartupSettingsMaintenance } from "../../utils/settingsMigrations"
2525

2626
type GlobalStateKey = keyof GlobalState
2727
type SecretStateKey = keyof SecretState
@@ -97,8 +97,8 @@ export class ContextProxy {
9797
// Migration: Move legacy customCondensingPrompt to customSupportPrompts
9898
await this.migrateLegacyCondensingPrompt()
9999

100-
// Migration: Clear hardcoded defaults so users can benefit from future default changes
101-
await runSettingsMigrations(this)
100+
// Settings maintenance: Run migrations and clear settings that match defaults
101+
await runStartupSettingsMaintenance(this)
102102

103103
this._isInitialized = true
104104
}

src/utils/__tests__/settingsMigrations.spec.ts

Lines changed: 140 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
import { runSettingsMigrations, migrations, CURRENT_MIGRATION_VERSION } from "../settingsMigrations"
1+
import {
2+
runSettingsMigrations,
3+
migrations,
4+
CURRENT_MIGRATION_VERSION,
5+
clearDefaultSettings,
6+
runStartupSettingsMaintenance,
7+
} from "../settingsMigrations"
28
import type { ContextProxy } from "../../core/config/ContextProxy"
39
import type { GlobalState } from "@roo-code/types"
10+
import { settingDefaults } from "@roo-code/types"
411

512
// Mock the logger
613
vi.mock("../logging", () => ({
@@ -296,4 +303,136 @@ describe("settingsMigrations", () => {
296303
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("codebaseIndexConfig", undefined)
297304
})
298305
})
306+
307+
describe("clearDefaultSettings", () => {
308+
it("should clear settings that match current defaults", async () => {
309+
// Setup: user has settings that match current defaults
310+
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
311+
if (key === "browserToolEnabled") return settingDefaults.browserToolEnabled
312+
if (key === "soundVolume") return settingDefaults.soundVolume
313+
if (key === "maxWorkspaceFiles") return settingDefaults.maxWorkspaceFiles
314+
return undefined
315+
})
316+
317+
const clearedCount = await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
318+
319+
// All matching defaults should be cleared
320+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("browserToolEnabled", undefined)
321+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("soundVolume", undefined)
322+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("maxWorkspaceFiles", undefined)
323+
expect(clearedCount).toBe(3)
324+
})
325+
326+
it("should preserve custom values that don't match defaults", async () => {
327+
// Setup: user has custom values that don't match defaults
328+
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
329+
if (key === "browserToolEnabled") return false // default is true
330+
if (key === "soundVolume") return 0.8 // default is 0.5
331+
if (key === "maxWorkspaceFiles") return 500 // default is 200
332+
return undefined
333+
})
334+
335+
const clearedCount = await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
336+
337+
// No settings should be cleared
338+
expect(mockContextProxy.updateGlobalState).not.toHaveBeenCalled()
339+
expect(clearedCount).toBe(0)
340+
})
341+
342+
it("should not clear already undefined values", async () => {
343+
// Setup: all settings are undefined
344+
mockContextProxy.getGlobalState.mockReturnValue(undefined)
345+
346+
const clearedCount = await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
347+
348+
// No settings should be cleared (already undefined)
349+
expect(mockContextProxy.updateGlobalState).not.toHaveBeenCalled()
350+
expect(clearedCount).toBe(0)
351+
})
352+
353+
it("should only clear settings in settingDefaults", async () => {
354+
// Setup: user has settings - some in defaults, some not
355+
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
356+
if (key === "browserToolEnabled") return settingDefaults.browserToolEnabled
357+
if (key === "customInstructions") return "my instructions" // not in settingDefaults
358+
return undefined
359+
})
360+
361+
await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
362+
363+
// browserToolEnabled should be cleared
364+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("browserToolEnabled", undefined)
365+
366+
// customInstructions should NOT be touched (not in settingDefaults)
367+
expect(mockContextProxy.updateGlobalState).not.toHaveBeenCalledWith("customInstructions", undefined)
368+
})
369+
370+
it("should handle string settings correctly", async () => {
371+
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
372+
if (key === "enterBehavior") return "send" // matches default
373+
if (key === "language") return "en" // matches default
374+
return undefined
375+
})
376+
377+
await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
378+
379+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("enterBehavior", undefined)
380+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("language", undefined)
381+
})
382+
383+
it("should return the count of cleared settings", async () => {
384+
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
385+
if (key === "browserToolEnabled") return true // matches default
386+
if (key === "soundEnabled") return true // matches default
387+
if (key === "soundVolume") return 0.8 // does NOT match default (0.5)
388+
return undefined
389+
})
390+
391+
const clearedCount = await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
392+
393+
expect(clearedCount).toBe(2) // Only browserToolEnabled and soundEnabled match
394+
})
395+
})
396+
397+
describe("runStartupSettingsMaintenance", () => {
398+
it("should run both migrations and default clearing", async () => {
399+
// Setup: migration not run, and has a setting matching default
400+
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
401+
if (key === "settingsMigrationVersion") return 0
402+
if (key === "browserToolEnabled") return true // matches both historical and current default
403+
return undefined
404+
})
405+
406+
await runStartupSettingsMaintenance(mockContextProxy as unknown as ContextProxy)
407+
408+
// Should have updated migration version
409+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith(
410+
"settingsMigrationVersion",
411+
CURRENT_MIGRATION_VERSION,
412+
)
413+
414+
// browserToolEnabled should be cleared (by migration or clearDefaults)
415+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("browserToolEnabled", undefined)
416+
})
417+
418+
it("should run clearDefaultSettings even after migrations are complete", async () => {
419+
// Setup: migrations already complete, but has setting matching default
420+
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
421+
if (key === "settingsMigrationVersion") return CURRENT_MIGRATION_VERSION
422+
if (key === "soundVolume") return 0.5 // matches current default
423+
return undefined
424+
})
425+
426+
await runStartupSettingsMaintenance(mockContextProxy as unknown as ContextProxy)
427+
428+
// Migration version should NOT be updated (already current)
429+
expect(mockContextProxy.updateGlobalState).not.toHaveBeenCalledWith(
430+
"settingsMigrationVersion",
431+
expect.anything(),
432+
)
433+
434+
// soundVolume should be cleared by clearDefaultSettings
435+
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("soundVolume", undefined)
436+
})
437+
})
299438
})

src/utils/settingsMigrations.ts

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
11
/**
2-
* Settings migrations for version-gated migration of hardcoded defaults.
2+
* Settings migrations and defaults cleanup.
33
*
4-
* This module tracks which migrations have been applied and runs any pending
5-
* migrations when the extension starts. Each migration targets specific
6-
* historical default values that were being hardcoded in storage before
7-
* the "reset to default" pattern fix.
4+
* This module provides two mechanisms for managing settings:
5+
*
6+
* 1. **Version-gated migrations**: Run once per version to handle specific
7+
* migration scenarios (e.g., flattening nested configs).
8+
*
9+
* 2. **Every-startup defaults clearing**: Clears settings that exactly match
10+
* their current default values on every startup. This ensures users always
11+
* benefit from default value improvements.
812
*
913
* See plans/reset-to-default-ideal-pattern.md for the full design.
1014
*/
1115

1216
import type { ContextProxy } from "../core/config/ContextProxy"
1317
import type { GlobalState, CodebaseIndexConfig } from "@roo-code/types"
18+
import { settingDefaults, type SettingWithDefault } from "@roo-code/types"
1419

1520
import { logger } from "./logging"
1621

@@ -175,3 +180,57 @@ export async function runSettingsMigrations(contextProxy: ContextProxy): Promise
175180
await contextProxy.updateGlobalState("settingsMigrationVersion", CURRENT_MIGRATION_VERSION)
176181
logger.info(`Settings migration complete. Now at version ${CURRENT_MIGRATION_VERSION}`)
177182
}
183+
184+
/**
185+
* Clears settings that exactly match their current default values.
186+
*
187+
* This function runs on every startup to ensure users always benefit from
188+
* default value improvements. When a setting's stored value exactly matches
189+
* the current default, it's cleared (set to undefined) so the default is
190+
* applied at read time.
191+
*
192+
* Note: This approach means users cannot "lock in" a value that happens to
193+
* match the default. If they explicitly set browserToolEnabled=true (the default),
194+
* it will be cleared and they'll use whatever the default is in the future.
195+
*
196+
* @param contextProxy - The ContextProxy instance for reading/writing state
197+
* @returns The number of settings that were cleared
198+
*/
199+
export async function clearDefaultSettings(contextProxy: ContextProxy): Promise<number> {
200+
let clearedCount = 0
201+
202+
for (const key of Object.keys(settingDefaults) as SettingWithDefault[]) {
203+
const storedValue = contextProxy.getGlobalState(key as keyof GlobalState)
204+
const defaultValue = settingDefaults[key]
205+
206+
// Only clear if stored value exactly matches the current default
207+
// undefined values are already "default" so skip them
208+
if (storedValue !== undefined && storedValue === defaultValue) {
209+
await contextProxy.updateGlobalState(key as keyof GlobalState, undefined)
210+
logger.info(`Cleared default setting: ${key} (was ${JSON.stringify(storedValue)})`)
211+
clearedCount++
212+
}
213+
}
214+
215+
if (clearedCount > 0) {
216+
logger.info(`Cleared ${clearedCount} settings that matched their defaults`)
217+
}
218+
219+
return clearedCount
220+
}
221+
222+
/**
223+
* Runs all startup settings maintenance tasks.
224+
*
225+
* This is the main entry point that should be called on extension startup.
226+
* It runs both migrations (once per version) and defaults clearing (every startup).
227+
*
228+
* @param contextProxy - The ContextProxy instance for reading/writing state
229+
*/
230+
export async function runStartupSettingsMaintenance(contextProxy: ContextProxy): Promise<void> {
231+
// First run any pending migrations
232+
await runSettingsMigrations(contextProxy)
233+
234+
// Then clear any settings that match current defaults
235+
await clearDefaultSettings(contextProxy)
236+
}

0 commit comments

Comments
 (0)