diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index e87cbca3b4..657f9d677f 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -86,6 +86,8 @@ export interface SettingDefinition { options?: readonly SettingEnumOption[]; /** Schema for array items when type is 'array' */ items?: SettingItemDefinition; + /** Minimum value for number-type settings. */ + minimum?: number; /** * Primitive shapes a field accepted before it was expanded to its current * type. The exported JSON Schema wraps the field in `anyOf` so values from @@ -493,6 +495,7 @@ const SETTINGS_SCHEMA = { // surprised when a mid-session edit doesn't take effect immediately. requiresRestart: true, default: 30, + minimum: 0, description: 'Number of days to retain ~/.qwen/file-history/ session backups used by /rewind and background subagent transcripts under /subagents/. Data older than this is removed by a background housekeeping pass that runs at most once per day. Set to 0 for minimum retention (~1 hour) — protects sessions touched in the last hour, plus the currently active session.', showInDialog: true, diff --git a/packages/cli/src/serve/routes/workspace-settings.test.ts b/packages/cli/src/serve/routes/workspace-settings.test.ts new file mode 100644 index 0000000000..612b2cd46a --- /dev/null +++ b/packages/cli/src/serve/routes/workspace-settings.test.ts @@ -0,0 +1,83 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi } from 'vitest'; +import express from 'express'; +import request from 'supertest'; +import { registerWorkspaceSettingsRoutes } from './workspace-settings.js'; + +function makeApp() { + const app = express(); + app.use(express.json()); + + const persistSetting = vi.fn(async () => {}); + const broadcastSettingsChanged = vi.fn(); + + registerWorkspaceSettingsRoutes(app, { + boundWorkspace: '/workspace', + mutate: () => (_req, _res, next) => next(), + safeBody: (req) => + req.body && typeof req.body === 'object' ? req.body : {}, + persistSetting, + broadcastSettingsChanged, + parseAndValidateClientId: () => undefined, + }); + + return { app, persistSetting, broadcastSettingsChanged }; +} + +describe('POST /workspace/settings', () => { + it('rejects negative general.cleanupPeriodDays values', async () => { + const { app, persistSetting, broadcastSettingsChanged } = makeApp(); + + for (const value of [-1, -5]) { + const res = await request(app).post('/workspace/settings').send({ + scope: 'workspace', + key: 'general.cleanupPeriodDays', + value, + }); + + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ + code: 'invalid_value', + error: 'Value must be >= 0', + }); + } + + expect(persistSetting).not.toHaveBeenCalled(); + expect(broadcastSettingsChanged).not.toHaveBeenCalled(); + }); + + it.each([0, 30])('accepts general.cleanupPeriodDays=%s', async (value) => { + const { app, persistSetting, broadcastSettingsChanged } = makeApp(); + + const res = await request(app).post('/workspace/settings').send({ + scope: 'workspace', + key: 'general.cleanupPeriodDays', + value, + }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + key: 'general.cleanupPeriodDays', + scope: 'workspace', + value, + requiresRestart: true, + }); + expect(persistSetting).toHaveBeenCalledWith( + '/workspace', + expect.any(String), + 'general.cleanupPeriodDays', + value, + ); + expect(broadcastSettingsChanged).toHaveBeenCalledWith( + 'general.cleanupPeriodDays', + value, + 'workspace', + undefined, + ); + }); +}); diff --git a/packages/cli/src/serve/routes/workspace-settings.ts b/packages/cli/src/serve/routes/workspace-settings.ts index c053cf0ef7..fb20f11625 100644 --- a/packages/cli/src/serve/routes/workspace-settings.ts +++ b/packages/cli/src/serve/routes/workspace-settings.ts @@ -7,7 +7,6 @@ import type { Application, Request, Response } from 'express'; import { loadSettings, SettingScope } from '../../config/settings.js'; import type { - SettingDefinition, SettingEnumOption, SettingsType, SettingsValue, @@ -16,6 +15,7 @@ import { getDialogSettingKeys, getNestedProperty, getSettingDefinition, + validateSettingValue, } from '../../utils/settingsUtils.js'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; @@ -40,8 +40,6 @@ const WEB_SHELL_SETTINGS = new Set(['ui.compactMode', 'voiceModel']); const VALID_WRITE_SCOPES = new Set(['workspace']); -const MAX_STRING_VALUE_LENGTH = 1024; - interface SettingDescriptor { key: string; type: SettingsType; @@ -139,35 +137,6 @@ function buildSettingsResponse( }; } -function validateSettingValue( - def: SettingDefinition, - value: unknown, -): string | undefined { - switch (def.type) { - case 'boolean': - if (typeof value !== 'boolean') return 'Value must be a boolean'; - break; - case 'number': - if (typeof value !== 'number' || !Number.isFinite(value)) - return 'Value must be a finite number'; - break; - case 'string': - if (typeof value !== 'string') return 'Value must be a string'; - if (value.length > MAX_STRING_VALUE_LENGTH) - return `Value exceeds ${MAX_STRING_VALUE_LENGTH}-character limit`; - break; - case 'enum': - if (!def.options?.some((opt) => opt.value === value)) { - const allowed = def.options?.map((o) => o.value).join(', ') ?? ''; - return `Value must be one of: ${allowed}`; - } - break; - default: - return `Settings of type '${def.type}' cannot be modified via this API`; - } - return undefined; -} - const SCOPE_MAP: Record = { workspace: SettingScope.Workspace, }; diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx index 3729697ecc..3e9f42f48b 100644 --- a/packages/cli/src/ui/components/SettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx @@ -171,6 +171,7 @@ vi.mock('../../utils/languageUtils.js', async () => { return { ...actual, updateOutputLanguageFile: vi.fn(), + writeOutputLanguageAndRegisterPath: vi.fn(), }; }); @@ -524,6 +525,56 @@ describe('SettingsDialog', () => { unmount(); }); + it('should not save number settings below their configured minimum', async () => { + vi.mocked(saveModifiedSettings).mockClear(); + + const settings = createMockSettings(); + const onSelect = vi.fn(); + const component = ( + + + + ); + + const { stdin, unmount, lastFrame } = render(component); + + await waitFor(() => { + expect(lastFrame()).toContain('Settings'); + }); + + const cleanupPeriodIndex = getDialogSettingKeys().indexOf( + 'general.cleanupPeriodDays', + ); + expect(cleanupPeriodIndex).toBeGreaterThanOrEqual(0); + + const press = async (key: string) => { + act(() => { + stdin.write(key); + }); + await wait(); + }; + + for (let i = 0; i < cleanupPeriodIndex; i++) { + await press(TerminalKeys.DOWN_ARROW as string); + } + + await press(TerminalKeys.ENTER as string); + await press('-'); + await press('1'); + await press(TerminalKeys.ENTER as string); + + await wait(); + + const cleanupPeriodCall = vi + .mocked(saveModifiedSettings) + .mock.calls.find((call) => + (call[0] as Set).has('general.cleanupPeriodDays'), + ); + expect(cleanupPeriodCall).toBeUndefined(); + + unmount(); + }); + describe('enum values', () => { it('toggles enum values with the enter key', async () => { vi.mocked(saveModifiedSettings).mockClear(); diff --git a/packages/cli/src/ui/components/SettingsDialog.tsx b/packages/cli/src/ui/components/SettingsDialog.tsx index f6c1566180..2e2e54a603 100644 --- a/packages/cli/src/ui/components/SettingsDialog.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.tsx @@ -26,6 +26,7 @@ import { setPendingSettingValueAny, getNestedValue, getEffectiveValue, + validateSettingValue, } from '../../utils/settingsUtils.js'; import { writeOutputLanguageAndRegisterPath } from '../../utils/languageUtils.js'; import { @@ -325,6 +326,16 @@ export function SettingsDialog({ } } + if (definition) { + const validationError = validateSettingValue(definition, parsed); + if (validationError) { + setEditingKey(null); + setEditBuffer(''); + setEditCursorPos(0); + return; + } + } + // Update pending setPendingSettings((prev) => parsed === undefined diff --git a/packages/cli/src/utils/settingsUtils.test.ts b/packages/cli/src/utils/settingsUtils.test.ts index 03c769d8e1..99da778a2b 100644 --- a/packages/cli/src/utils/settingsUtils.test.ts +++ b/packages/cli/src/utils/settingsUtils.test.ts @@ -36,6 +36,7 @@ import { getEffectiveDisplayValue, setNestedPropertySafe, setNestedPropertyForce, + validateSettingValue, } from './settingsUtils.js'; import { getSettingsSchema, @@ -79,6 +80,16 @@ describe('SettingsUtils', () => { description: 'A test field', showInDialog: true, }, + numberWithMinimum: { + type: 'number', + label: 'Number With Minimum', + category: 'Basic', + requiresRestart: false, + default: 0, + minimum: 0, + description: 'A number field with a minimum.', + showInDialog: true, + }, advanced: { type: 'object', label: 'Advanced', @@ -211,6 +222,25 @@ describe('SettingsUtils', () => { }); }); + describe('validateSettingValue', () => { + it('accepts finite numbers at the configured minimum', () => { + const definition = getSettingDefinition('numberWithMinimum'); + expect(definition).toBeDefined(); + + expect(validateSettingValue(definition!, 0)).toBeUndefined(); + expect(validateSettingValue(definition!, 1)).toBeUndefined(); + }); + + it('rejects numbers below the configured minimum', () => { + const definition = getSettingDefinition('numberWithMinimum'); + expect(definition).toBeDefined(); + + expect(validateSettingValue(definition!, -1)).toBe( + 'Value must be >= 0', + ); + }); + }); + describe('requiresRestart', () => { it('should return true for settings that require restart', () => { expect(requiresRestart('ui.requiresRestart')).toBe(true); diff --git a/packages/cli/src/utils/settingsUtils.ts b/packages/cli/src/utils/settingsUtils.ts index c85d83379d..ee249fa8de 100644 --- a/packages/cli/src/utils/settingsUtils.ts +++ b/packages/cli/src/utils/settingsUtils.ts @@ -309,6 +309,39 @@ const SETTINGS_DIALOG_ORDER: readonly string[] = [ 'privacy.usageStatisticsEnabled', ] as const; +export const MAX_SETTING_STRING_VALUE_LENGTH = 1024; + +export function validateSettingValue( + def: SettingDefinition, + value: unknown, +): string | undefined { + switch (def.type) { + case 'boolean': + if (typeof value !== 'boolean') return 'Value must be a boolean'; + break; + case 'number': + if (typeof value !== 'number' || !Number.isFinite(value)) + return 'Value must be a finite number'; + if (def.minimum !== undefined && value < def.minimum) + return `Value must be >= ${def.minimum}`; + break; + case 'string': + if (typeof value !== 'string') return 'Value must be a string'; + if (value.length > MAX_SETTING_STRING_VALUE_LENGTH) + return `Value exceeds ${MAX_SETTING_STRING_VALUE_LENGTH}-character limit`; + break; + case 'enum': + if (!def.options?.some((opt) => opt.value === value)) { + const allowed = def.options?.map((o) => o.value).join(', ') ?? ''; + return `Value must be one of: ${allowed}`; + } + break; + default: + return `Settings of type '${def.type}' cannot be modified via this API`; + } + return undefined; +} + /** * Get all setting keys that should be shown in the dialog, sorted by display order */ diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index bb474fcc5f..e8421b7a40 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -101,7 +101,8 @@ "cleanupPeriodDays": { "description": "Number of days to retain ~/.qwen/file-history/ session backups used by /rewind and background subagent transcripts under /subagents/. Data older than this is removed by a background housekeeping pass that runs at most once per day. Set to 0 for minimum retention (~1 hour) — protects sessions touched in the last hour, plus the currently active session.", "type": "number", - "default": 30 + "default": 30, + "minimum": 0 }, "gitCoAuthor": { "description": "Attribution added to git commits and pull requests created through Qwen Code.", diff --git a/scripts/generate-settings-schema.ts b/scripts/generate-settings-schema.ts index 281aea99ff..28d7f7a47d 100644 --- a/scripts/generate-settings-schema.ts +++ b/scripts/generate-settings-schema.ts @@ -38,6 +38,7 @@ interface JsonSchemaProperty { items?: JsonSchemaProperty; enum?: (string | number)[]; default?: unknown; + minimum?: number; additionalProperties?: boolean | JsonSchemaProperty; required?: string[]; oneOf?: JsonSchemaProperty[]; @@ -184,6 +185,10 @@ function convertSettingToJsonSchema( } } + if (setting.type === 'number' && setting.minimum !== undefined) { + schema.minimum = setting.minimum; + } + // If the field accepts a legacy primitive shape (e.g. a boolean that was // later expanded into an object), wrap with `anyOf` so existing values // in users' settings.json don't trip the IDE schema validator while