fix: Address todo list advanced settings feedback

- Remove unnecessary todoListEnabled migration logic (no backfill needed)
- Fix Advanced Settings text wrapping with proper CSS classes
- Temperature and rate limit controls already under advanced settings
This commit is contained in:
Roo Code 2025-09-09 06:21:46 +00:00
parent 195f4eb245
commit 8e077a28f4
3 changed files with 4 additions and 67 deletions

View file

@ -32,7 +32,6 @@ export const providerProfilesSchema = z.object({
diffSettingsMigrated: z.boolean().optional(),
openAiHeadersMigrated: z.boolean().optional(),
consecutiveMistakeLimitMigrated: z.boolean().optional(),
todoListEnabledMigrated: z.boolean().optional(),
})
.optional(),
})
@ -56,7 +55,6 @@ export class ProviderSettingsManager {
diffSettingsMigrated: true, // Mark as migrated on fresh installs
openAiHeadersMigrated: true, // Mark as migrated on fresh installs
consecutiveMistakeLimitMigrated: true, // Mark as migrated on fresh installs
todoListEnabledMigrated: true, // Mark as migrated on fresh installs
},
}
@ -123,7 +121,6 @@ export class ProviderSettingsManager {
diffSettingsMigrated: false,
openAiHeadersMigrated: false,
consecutiveMistakeLimitMigrated: false,
todoListEnabledMigrated: false,
} // Initialize with default values
isDirty = true
}
@ -152,12 +149,6 @@ export class ProviderSettingsManager {
isDirty = true
}
if (!providerProfiles.migrations.todoListEnabledMigrated) {
await this.migrateTodoListEnabled(providerProfiles)
providerProfiles.migrations.todoListEnabledMigrated = true
isDirty = true
}
if (isDirty) {
await this.store(providerProfiles)
}
@ -263,18 +254,6 @@ export class ProviderSettingsManager {
}
}
private async migrateTodoListEnabled(providerProfiles: ProviderProfiles) {
try {
for (const [_name, apiConfig] of Object.entries(providerProfiles.apiConfigs)) {
if (apiConfig.todoListEnabled === undefined) {
apiConfig.todoListEnabled = true
}
}
} catch (error) {
console.error(`[MigrateTodoListEnabled] Failed to migrate todo list enabled setting:`, error)
}
}
/**
* Clean model ID by removing prefix before "/"
*/

View file

@ -67,7 +67,6 @@ describe("ProviderSettingsManager", () => {
diffSettingsMigrated: true,
openAiHeadersMigrated: true,
consecutiveMistakeLimitMigrated: true,
todoListEnabledMigrated: true,
},
}),
)
@ -187,48 +186,6 @@ describe("ProviderSettingsManager", () => {
expect(storedConfig.migrations.consecutiveMistakeLimitMigrated).toEqual(true)
})
it("should call migrateTodoListEnabled if it has not done so already", async () => {
mockSecrets.get.mockResolvedValue(
JSON.stringify({
currentApiConfigName: "default",
apiConfigs: {
default: {
config: {},
id: "default",
todoListEnabled: undefined,
},
test: {
apiProvider: "anthropic",
todoListEnabled: undefined,
},
existing: {
apiProvider: "anthropic",
// this should not really be possible, unless someone has loaded a hand edited config,
// but we don't overwrite so we'll check that
todoListEnabled: false,
},
},
migrations: {
rateLimitSecondsMigrated: true,
diffSettingsMigrated: true,
openAiHeadersMigrated: true,
consecutiveMistakeLimitMigrated: true,
todoListEnabledMigrated: false,
},
}),
)
await providerSettingsManager.initialize()
// Get the last call to store, which should contain the migrated config
const calls = mockSecrets.store.mock.calls
const storedConfig = JSON.parse(calls[calls.length - 1][1])
expect(storedConfig.apiConfigs.default.todoListEnabled).toEqual(true)
expect(storedConfig.apiConfigs.test.todoListEnabled).toEqual(true)
expect(storedConfig.apiConfigs.existing.todoListEnabled).toEqual(false)
expect(storedConfig.migrations.todoListEnabledMigrated).toEqual(true)
})
it("should throw error if secrets storage fails", async () => {
mockSecrets.get.mockRejectedValue(new Error("Storage failed"))

View file

@ -751,9 +751,10 @@ const ApiOptions = ({
{!fromWelcomeView && (
<Collapsible open={isAdvancedSettingsOpen} onOpenChange={setIsAdvancedSettingsOpen}>
<CollapsibleTrigger className="flex items-center gap-1 w-full cursor-pointer hover:opacity-80 mb-2">
<span className={`codicon codicon-chevron-${isAdvancedSettingsOpen ? "down" : "right"}`}></span>
<span className="font-medium">{t("settings:advancedSettings.title")}</span>
<CollapsibleTrigger className="flex items-center gap-2 w-full cursor-pointer hover:opacity-80 mb-2">
<span
className={`codicon codicon-chevron-${isAdvancedSettingsOpen ? "down" : "right"} flex-shrink-0`}></span>
<span className="font-medium whitespace-nowrap">{t("settings:advancedSettings.title")}</span>
</CollapsibleTrigger>
<CollapsibleContent className="space-y-3">
<TodoListSettingsControl