mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
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.
This commit is contained in:
parent
f02913ba90
commit
28ab2e77f2
3 changed files with 207 additions and 9 deletions
|
|
@ -21,7 +21,7 @@ import { TelemetryService } from "@roo-code/telemetry"
|
|||
|
||||
import { logger } from "../../utils/logging"
|
||||
import { supportPrompt } from "../../shared/support-prompt"
|
||||
import { runSettingsMigrations } from "../../utils/settingsMigrations"
|
||||
import { runStartupSettingsMaintenance } from "../../utils/settingsMigrations"
|
||||
|
||||
type GlobalStateKey = keyof GlobalState
|
||||
type SecretStateKey = keyof SecretState
|
||||
|
|
@ -100,8 +100,8 @@ export class ContextProxy {
|
|||
// Migration: Clear old default condensing prompt so users get the improved v2 default
|
||||
await this.migrateOldDefaultCondensingPrompt()
|
||||
|
||||
// Migration: Clear hardcoded defaults so users can benefit from future default changes
|
||||
await runSettingsMigrations(this)
|
||||
// Settings maintenance: Run migrations and clear settings that match defaults
|
||||
await runStartupSettingsMaintenance(this)
|
||||
|
||||
this._isInitialized = true
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,13 @@
|
|||
import { runSettingsMigrations, migrations, CURRENT_MIGRATION_VERSION } from "../settingsMigrations"
|
||||
import {
|
||||
runSettingsMigrations,
|
||||
migrations,
|
||||
CURRENT_MIGRATION_VERSION,
|
||||
clearDefaultSettings,
|
||||
runStartupSettingsMaintenance,
|
||||
} from "../settingsMigrations"
|
||||
import type { ContextProxy } from "../../core/config/ContextProxy"
|
||||
import type { GlobalState } from "@roo-code/types"
|
||||
import { settingDefaults } from "@roo-code/types"
|
||||
|
||||
// Mock the logger
|
||||
vi.mock("../logging", () => ({
|
||||
|
|
@ -296,4 +303,136 @@ describe("settingsMigrations", () => {
|
|||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("codebaseIndexConfig", undefined)
|
||||
})
|
||||
})
|
||||
|
||||
describe("clearDefaultSettings", () => {
|
||||
it("should clear settings that match current defaults", async () => {
|
||||
// Setup: user has settings that match current defaults
|
||||
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
|
||||
if (key === "browserToolEnabled") return settingDefaults.browserToolEnabled
|
||||
if (key === "soundVolume") return settingDefaults.soundVolume
|
||||
if (key === "maxWorkspaceFiles") return settingDefaults.maxWorkspaceFiles
|
||||
return undefined
|
||||
})
|
||||
|
||||
const clearedCount = await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
|
||||
|
||||
// All matching defaults should be cleared
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("browserToolEnabled", undefined)
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("soundVolume", undefined)
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("maxWorkspaceFiles", undefined)
|
||||
expect(clearedCount).toBe(3)
|
||||
})
|
||||
|
||||
it("should preserve custom values that don't match defaults", async () => {
|
||||
// Setup: user has custom values that don't match defaults
|
||||
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
|
||||
if (key === "browserToolEnabled") return false // default is true
|
||||
if (key === "soundVolume") return 0.8 // default is 0.5
|
||||
if (key === "maxWorkspaceFiles") return 500 // default is 200
|
||||
return undefined
|
||||
})
|
||||
|
||||
const clearedCount = await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
|
||||
|
||||
// No settings should be cleared
|
||||
expect(mockContextProxy.updateGlobalState).not.toHaveBeenCalled()
|
||||
expect(clearedCount).toBe(0)
|
||||
})
|
||||
|
||||
it("should not clear already undefined values", async () => {
|
||||
// Setup: all settings are undefined
|
||||
mockContextProxy.getGlobalState.mockReturnValue(undefined)
|
||||
|
||||
const clearedCount = await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
|
||||
|
||||
// No settings should be cleared (already undefined)
|
||||
expect(mockContextProxy.updateGlobalState).not.toHaveBeenCalled()
|
||||
expect(clearedCount).toBe(0)
|
||||
})
|
||||
|
||||
it("should only clear settings in settingDefaults", async () => {
|
||||
// Setup: user has settings - some in defaults, some not
|
||||
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
|
||||
if (key === "browserToolEnabled") return settingDefaults.browserToolEnabled
|
||||
if (key === "customInstructions") return "my instructions" // not in settingDefaults
|
||||
return undefined
|
||||
})
|
||||
|
||||
await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
|
||||
|
||||
// browserToolEnabled should be cleared
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("browserToolEnabled", undefined)
|
||||
|
||||
// customInstructions should NOT be touched (not in settingDefaults)
|
||||
expect(mockContextProxy.updateGlobalState).not.toHaveBeenCalledWith("customInstructions", undefined)
|
||||
})
|
||||
|
||||
it("should handle string settings correctly", async () => {
|
||||
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
|
||||
if (key === "enterBehavior") return "send" // matches default
|
||||
if (key === "language") return "en" // matches default
|
||||
return undefined
|
||||
})
|
||||
|
||||
await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
|
||||
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("enterBehavior", undefined)
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("language", undefined)
|
||||
})
|
||||
|
||||
it("should return the count of cleared settings", async () => {
|
||||
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
|
||||
if (key === "browserToolEnabled") return true // matches default
|
||||
if (key === "soundEnabled") return true // matches default
|
||||
if (key === "soundVolume") return 0.8 // does NOT match default (0.5)
|
||||
return undefined
|
||||
})
|
||||
|
||||
const clearedCount = await clearDefaultSettings(mockContextProxy as unknown as ContextProxy)
|
||||
|
||||
expect(clearedCount).toBe(2) // Only browserToolEnabled and soundEnabled match
|
||||
})
|
||||
})
|
||||
|
||||
describe("runStartupSettingsMaintenance", () => {
|
||||
it("should run both migrations and default clearing", async () => {
|
||||
// Setup: migration not run, and has a setting matching default
|
||||
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
|
||||
if (key === "settingsMigrationVersion") return 0
|
||||
if (key === "browserToolEnabled") return true // matches both historical and current default
|
||||
return undefined
|
||||
})
|
||||
|
||||
await runStartupSettingsMaintenance(mockContextProxy as unknown as ContextProxy)
|
||||
|
||||
// Should have updated migration version
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith(
|
||||
"settingsMigrationVersion",
|
||||
CURRENT_MIGRATION_VERSION,
|
||||
)
|
||||
|
||||
// browserToolEnabled should be cleared (by migration or clearDefaults)
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("browserToolEnabled", undefined)
|
||||
})
|
||||
|
||||
it("should run clearDefaultSettings even after migrations are complete", async () => {
|
||||
// Setup: migrations already complete, but has setting matching default
|
||||
mockContextProxy.getGlobalState.mockImplementation((key: keyof GlobalState) => {
|
||||
if (key === "settingsMigrationVersion") return CURRENT_MIGRATION_VERSION
|
||||
if (key === "soundVolume") return 0.5 // matches current default
|
||||
return undefined
|
||||
})
|
||||
|
||||
await runStartupSettingsMaintenance(mockContextProxy as unknown as ContextProxy)
|
||||
|
||||
// Migration version should NOT be updated (already current)
|
||||
expect(mockContextProxy.updateGlobalState).not.toHaveBeenCalledWith(
|
||||
"settingsMigrationVersion",
|
||||
expect.anything(),
|
||||
)
|
||||
|
||||
// soundVolume should be cleared by clearDefaultSettings
|
||||
expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("soundVolume", undefined)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,16 +1,21 @@
|
|||
/**
|
||||
* Settings migrations for version-gated migration of hardcoded defaults.
|
||||
* Settings migrations and defaults cleanup.
|
||||
*
|
||||
* This module tracks which migrations have been applied and runs any pending
|
||||
* migrations when the extension starts. Each migration targets specific
|
||||
* historical default values that were being hardcoded in storage before
|
||||
* the "reset to default" pattern fix.
|
||||
* This module provides two mechanisms for managing settings:
|
||||
*
|
||||
* 1. **Version-gated migrations**: Run once per version to handle specific
|
||||
* migration scenarios (e.g., flattening nested configs).
|
||||
*
|
||||
* 2. **Every-startup defaults clearing**: Clears settings that exactly match
|
||||
* their current default values on every startup. This ensures users always
|
||||
* benefit from default value improvements.
|
||||
*
|
||||
* See plans/reset-to-default-ideal-pattern.md for the full design.
|
||||
*/
|
||||
|
||||
import type { ContextProxy } from "../core/config/ContextProxy"
|
||||
import type { GlobalState, CodebaseIndexConfig } from "@roo-code/types"
|
||||
import { settingDefaults, type SettingWithDefault } from "@roo-code/types"
|
||||
|
||||
import { logger } from "./logging"
|
||||
|
||||
|
|
@ -175,3 +180,57 @@ export async function runSettingsMigrations(contextProxy: ContextProxy): Promise
|
|||
await contextProxy.updateGlobalState("settingsMigrationVersion", CURRENT_MIGRATION_VERSION)
|
||||
logger.info(`Settings migration complete. Now at version ${CURRENT_MIGRATION_VERSION}`)
|
||||
}
|
||||
|
||||
/**
|
||||
* Clears settings that exactly match their current default values.
|
||||
*
|
||||
* This function runs on every startup to ensure users always benefit from
|
||||
* default value improvements. When a setting's stored value exactly matches
|
||||
* the current default, it's cleared (set to undefined) so the default is
|
||||
* applied at read time.
|
||||
*
|
||||
* Note: This approach means users cannot "lock in" a value that happens to
|
||||
* match the default. If they explicitly set browserToolEnabled=true (the default),
|
||||
* it will be cleared and they'll use whatever the default is in the future.
|
||||
*
|
||||
* @param contextProxy - The ContextProxy instance for reading/writing state
|
||||
* @returns The number of settings that were cleared
|
||||
*/
|
||||
export async function clearDefaultSettings(contextProxy: ContextProxy): Promise<number> {
|
||||
let clearedCount = 0
|
||||
|
||||
for (const key of Object.keys(settingDefaults) as SettingWithDefault[]) {
|
||||
const storedValue = contextProxy.getGlobalState(key as keyof GlobalState)
|
||||
const defaultValue = settingDefaults[key]
|
||||
|
||||
// Only clear if stored value exactly matches the current default
|
||||
// undefined values are already "default" so skip them
|
||||
if (storedValue !== undefined && storedValue === defaultValue) {
|
||||
await contextProxy.updateGlobalState(key as keyof GlobalState, undefined)
|
||||
logger.info(`Cleared default setting: ${key} (was ${JSON.stringify(storedValue)})`)
|
||||
clearedCount++
|
||||
}
|
||||
}
|
||||
|
||||
if (clearedCount > 0) {
|
||||
logger.info(`Cleared ${clearedCount} settings that matched their defaults`)
|
||||
}
|
||||
|
||||
return clearedCount
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs all startup settings maintenance tasks.
|
||||
*
|
||||
* This is the main entry point that should be called on extension startup.
|
||||
* It runs both migrations (once per version) and defaults clearing (every startup).
|
||||
*
|
||||
* @param contextProxy - The ContextProxy instance for reading/writing state
|
||||
*/
|
||||
export async function runStartupSettingsMaintenance(contextProxy: ContextProxy): Promise<void> {
|
||||
// First run any pending migrations
|
||||
await runSettingsMigrations(contextProxy)
|
||||
|
||||
// Then clear any settings that match current defaults
|
||||
await clearDefaultSettings(contextProxy)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue