diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index a848388fa..ec121e7cf 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -84,7 +84,8 @@ Settings are organized into categories. All settings should be placed within the | `general.enableAutoUpdate` | boolean | Enable automatic update checks and installations on startup. | `true` | | `general.showSessionRecap` | boolean | Auto-show a one-line "where you left off" recap when returning to the terminal after being away. Off by default. Use `/recap` to trigger manually regardless of this setting. | `false` | | `general.sessionRecapAwayThresholdMinutes` | number | Minutes the terminal must be blurred before an auto-recap fires on focus-in. Only used when `showSessionRecap` is enabled. | `5` | -| `general.gitCoAuthor` | boolean | Automatically add a Co-authored-by trailer to git commit messages when commits are made through Qwen Code. | `true` | +| `general.gitCoAuthor.commit` | boolean | Automatically add a Co-authored-by trailer to git commit messages when commits are made through Qwen Code. | `true` | +| `general.gitCoAuthor.pr` | boolean | Append a Qwen Code attribution line to pull request descriptions when running `gh pr create`. | `true` | | `general.checkpointing.enabled` | boolean | Enable session checkpointing for recovery. | `false` | | `general.defaultFileEncoding` | string | Default encoding for new files. Use `"utf-8"` (default) for UTF-8 without BOM, or `"utf-8-bom"` for UTF-8 with BOM. Only change this if your project specifically requires BOM. | `"utf-8"` | diff --git a/integration-tests/cli/settings-migration.test.ts b/integration-tests/cli/settings-migration.test.ts index 181dcb2ad..a68a01ec1 100644 --- a/integration-tests/cli/settings-migration.test.ts +++ b/integration-tests/cli/settings-migration.test.ts @@ -95,7 +95,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Verify migration to V3 - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); expect(migratedSettings['ui']).toEqual({ theme: 'dark', hideTips: false, @@ -137,7 +137,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Expected output based on stable test output - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); expect(migratedSettings['tools']).toEqual({ autoAccept: false }); expect(migratedSettings['context']).toEqual({ includeDirectories: [] }); expect(migratedSettings['model']).toEqual({ name: ['gemini', 'claude'] }); @@ -162,7 +162,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Should be migrated to V3 - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); // Legacy string values for ui/general should be preserved as-is (user data) expect(migratedSettings['ui']).toBe('legacy-ui-string'); expect(migratedSettings['general']).toBe('legacy-general-string'); @@ -189,7 +189,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Expected output based on stable test output - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); expect(migratedSettings['model']).toEqual({ name: 'qwen-plus' }); expect(migratedSettings['ui']).toEqual({ hideWindowTitle: true, @@ -226,7 +226,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Verify migration to V3 - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); // Verify disable* -> enable* conversion with inversion expect( @@ -303,7 +303,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Should be updated to V3 version - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); // Other settings should remain unchanged expect(migratedSettings['ui']).toEqual({ theme: 'dark' }); expect(migratedSettings['model']).toEqual({ name: 'gemini' }); @@ -330,7 +330,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Version metadata should still be normalized to current version - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); // Existing user content should be preserved expect(migratedSettings['customOnlyKey']).toBe('value'); }); @@ -372,7 +372,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Coercible strings are migrated; invalid disable* values are removed. - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); expect(migratedSettings['general']).toEqual({ enableAutoUpdate: false, }); @@ -437,7 +437,7 @@ describe('settings-migration', () => { const migratedSettings = readSettingsFile(rig); // Expected output based on stable test output - expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['$version']).toBe(4); // Migration converts disable* to enable* by inverting the value // disableAutoUpdate: false -> enableAutoUpdate: true (inverted) // But disableUpdateNag: true may affect the consolidation @@ -501,11 +501,10 @@ describe('settings-migration', () => { // Read settings const finalSettings = readSettingsFile(rig); - // Should remain V3 - expect(finalSettings['$version']).toBe(3); - // Note: V3 settings with legacy disable* keys are left as-is - // Migration only runs when version < current version - // Since this is already V3, no migration logic is applied + // V3 → V4 migration bumps the version; V3→V4 only touches + // general.gitCoAuthor, so unrelated legacy disable* keys remain as-is + // (V2→V3 ran on original V3 load, not re-applied here). + expect(finalSettings['$version']).toBe(4); expect( (finalSettings['general'] as Record)?.[ 'disableAutoUpdate' diff --git a/packages/cli/src/config/migration/index.test.ts b/packages/cli/src/config/migration/index.test.ts index 52bae237e..720370660 100644 --- a/packages/cli/src/config/migration/index.test.ts +++ b/packages/cli/src/config/migration/index.test.ts @@ -15,7 +15,7 @@ import { SETTINGS_VERSION } from '../settings.js'; describe('Migration Framework Integration', () => { describe('runMigrations', () => { - it('should migrate V1 settings to V3', () => { + it('should migrate V1 settings all the way to the current version', () => { const v1Settings = { theme: 'dark', model: 'gemini', @@ -25,8 +25,8 @@ describe('Migration Framework Integration', () => { const result = runMigrations(v1Settings, 'user'); - expect(result.finalVersion).toBe(3); - expect(result.executedMigrations).toHaveLength(2); + expect(result.finalVersion).toBe(SETTINGS_VERSION); + expect(result.executedMigrations).toHaveLength(SETTINGS_VERSION - 1); expect(result.executedMigrations[0]).toEqual({ fromVersion: 1, toVersion: 2, @@ -38,7 +38,7 @@ describe('Migration Framework Integration', () => { // Check V2 structure was created const settings = result.settings as Record; - expect(settings['$version']).toBe(3); + expect(settings['$version']).toBe(SETTINGS_VERSION); expect(settings['ui']).toEqual({ theme: 'dark', accessibility: { enableLoadingPhrases: true }, @@ -51,7 +51,7 @@ describe('Migration Framework Integration', () => { ).toBe(false); }); - it('should migrate V2 settings to V3', () => { + it('should migrate V2 settings forward through the chain', () => { const v2Settings = { $version: 2, ui: { theme: 'light' }, @@ -60,15 +60,15 @@ describe('Migration Framework Integration', () => { const result = runMigrations(v2Settings, 'user'); - expect(result.finalVersion).toBe(3); - expect(result.executedMigrations).toHaveLength(1); + expect(result.finalVersion).toBe(SETTINGS_VERSION); + expect(result.executedMigrations).toHaveLength(SETTINGS_VERSION - 2); expect(result.executedMigrations[0]).toEqual({ fromVersion: 2, toVersion: 3, }); const settings = result.settings as Record; - expect(settings['$version']).toBe(3); + expect(settings['$version']).toBe(SETTINGS_VERSION); expect( (settings['general'] as Record)['enableAutoUpdate'], ).toBe(true); @@ -77,18 +77,18 @@ describe('Migration Framework Integration', () => { ).toBeUndefined(); }); - it('should not modify V3 settings', () => { - const v3Settings = { - $version: 3, + it('should not modify settings already at the current version', () => { + const current = { + $version: SETTINGS_VERSION, ui: { theme: 'dark' }, general: { enableAutoUpdate: true }, }; - const result = runMigrations(v3Settings, 'user'); + const result = runMigrations(current, 'user'); - expect(result.finalVersion).toBe(3); + expect(result.finalVersion).toBe(SETTINGS_VERSION); expect(result.executedMigrations).toHaveLength(0); - expect(result.settings).toEqual(v3Settings); + expect(result.settings).toEqual(current); }); it('should be idempotent', () => { @@ -100,7 +100,7 @@ describe('Migration Framework Integration', () => { const result1 = runMigrations(v1Settings, 'user'); const result2 = runMigrations(result1.settings, 'user'); - expect(result1.executedMigrations).toHaveLength(2); + expect(result1.executedMigrations).toHaveLength(SETTINGS_VERSION - 1); expect(result2.executedMigrations).toHaveLength(0); expect(result1.finalVersion).toBe(result2.finalVersion); }); @@ -135,13 +135,13 @@ describe('Migration Framework Integration', () => { expect(needsMigration(cleanV2Settings)).toBe(true); }); - it('should return false for V3 settings', () => { - const v3Settings = { - $version: 3, + it('should return false for settings already at the current version', () => { + const current = { + $version: SETTINGS_VERSION, general: { enableAutoUpdate: true }, }; - expect(needsMigration(v3Settings)).toBe(false); + expect(needsMigration(current)).toBe(false); }); it('should return false for legacy numeric version when no migration can execute', () => { @@ -156,13 +156,12 @@ describe('Migration Framework Integration', () => { describe('ALL_MIGRATIONS', () => { it('should contain all migrations in order', () => { - expect(ALL_MIGRATIONS).toHaveLength(2); + expect(ALL_MIGRATIONS).toHaveLength(SETTINGS_VERSION - 1); - expect(ALL_MIGRATIONS[0].fromVersion).toBe(1); - expect(ALL_MIGRATIONS[0].toVersion).toBe(2); - - expect(ALL_MIGRATIONS[1].fromVersion).toBe(2); - expect(ALL_MIGRATIONS[1].toVersion).toBe(3); + for (let i = 0; i < ALL_MIGRATIONS.length; i++) { + expect(ALL_MIGRATIONS[i].fromVersion).toBe(i + 1); + expect(ALL_MIGRATIONS[i].toVersion).toBe(i + 2); + } }); }); @@ -178,10 +177,10 @@ describe('Migration Framework Integration', () => { const result = scheduler.migrate(v1Settings); - expect(result.executedMigrations).toHaveLength(2); + expect(result.executedMigrations).toHaveLength(SETTINGS_VERSION - 1); const settings = result.settings as Record; - expect(settings['$version']).toBe(3); + expect(settings['$version']).toBe(SETTINGS_VERSION); expect((settings['ui'] as Record)['theme']).toBe('dark'); expect( (settings['general'] as Record)['enableAutoUpdate'], @@ -212,16 +211,16 @@ describe('Migration Framework Integration', () => { }); it('needsMigration should return false when runMigrations would execute no migrations', () => { - const v3Settings = { - $version: 3, + const current = { + $version: SETTINGS_VERSION, general: { enableAutoUpdate: true }, }; // needsMigration should report that no migration is needed - expect(needsMigration(v3Settings)).toBe(false); + expect(needsMigration(current)).toBe(false); // runMigrations should execute no migrations - const result = runMigrations(v3Settings, 'user'); + const result = runMigrations(current, 'user'); expect(result.executedMigrations).toHaveLength(0); }); @@ -234,10 +233,10 @@ describe('Migration Framework Integration', () => { // needsMigration should report that migration is needed expect(needsMigration(cleanV2Settings)).toBe(true); - // runMigrations should execute the V2->V3 migration + // runMigrations should execute migrations forward to the current version const result = runMigrations(cleanV2Settings, 'user'); expect(result.executedMigrations.length).toBeGreaterThan(0); - expect(result.finalVersion).toBe(3); + expect(result.finalVersion).toBe(SETTINGS_VERSION); }); }); @@ -364,14 +363,14 @@ describe('Migration Framework Integration', () => { it('should avoid repeated no-op migration loops', () => { // Settings that might cause repeated migrations - const v3Settings = { - $version: 3, + const current = { + $version: SETTINGS_VERSION, general: { enableAutoUpdate: true }, }; // First check - expect(needsMigration(v3Settings)).toBe(false); - const result1 = runMigrations(v3Settings, 'user'); + expect(needsMigration(current)).toBe(false); + const result1 = runMigrations(current, 'user'); expect(result1.executedMigrations).toHaveLength(0); // Second check should be consistent diff --git a/packages/cli/src/config/migration/index.ts b/packages/cli/src/config/migration/index.ts index 40d176cbe..e37b06d8d 100644 --- a/packages/cli/src/config/migration/index.ts +++ b/packages/cli/src/config/migration/index.ts @@ -13,6 +13,7 @@ export { MigrationScheduler } from './scheduler.js'; // Export migrations export { v1ToV2Migration, V1ToV2Migration } from './versions/v1-to-v2.js'; export { v2ToV3Migration, V2ToV3Migration } from './versions/v2-to-v3.js'; +export { v3ToV4Migration, V3ToV4Migration } from './versions/v3-to-v4.js'; // Import settings version from single source of truth import { SETTINGS_VERSION } from '../settings.js'; @@ -22,6 +23,7 @@ import { SETTINGS_VERSION } from '../settings.js'; // Order matters: migrations must be sorted by ascending version import { v1ToV2Migration } from './versions/v1-to-v2.js'; import { v2ToV3Migration } from './versions/v2-to-v3.js'; +import { v3ToV4Migration } from './versions/v3-to-v4.js'; import { MigrationScheduler } from './scheduler.js'; import type { MigrationResult } from './types.js'; @@ -35,7 +37,11 @@ import type { MigrationResult } from './types.js'; * const result = scheduler.migrate(settings); * ``` */ -export const ALL_MIGRATIONS = [v1ToV2Migration, v2ToV3Migration] as const; +export const ALL_MIGRATIONS = [ + v1ToV2Migration, + v2ToV3Migration, + v3ToV4Migration, +] as const; /** * Convenience function that runs all migrations on the given settings. diff --git a/packages/cli/src/config/migration/versions/v3-to-v4.test.ts b/packages/cli/src/config/migration/versions/v3-to-v4.test.ts new file mode 100644 index 000000000..5f37cc592 --- /dev/null +++ b/packages/cli/src/config/migration/versions/v3-to-v4.test.ts @@ -0,0 +1,131 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { V3ToV4Migration } from './v3-to-v4.js'; + +describe('V3ToV4Migration', () => { + const migration = new V3ToV4Migration(); + + describe('shouldMigrate', () => { + it('returns true for V3 settings', () => { + expect( + migration.shouldMigrate({ + $version: 3, + general: { gitCoAuthor: false }, + }), + ).toBe(true); + }); + + it('returns true for V3 settings without gitCoAuthor', () => { + // Even without the relevant key, the version must still bump. + expect(migration.shouldMigrate({ $version: 3 })).toBe(true); + }); + + it('returns false for V4 settings', () => { + expect( + migration.shouldMigrate({ + $version: 4, + general: { gitCoAuthor: { commit: true, pr: true } }, + }), + ).toBe(false); + }); + + it('returns false for non-object input', () => { + expect(migration.shouldMigrate(null)).toBe(false); + expect(migration.shouldMigrate('x')).toBe(false); + expect(migration.shouldMigrate(42)).toBe(false); + }); + }); + + describe('migrate', () => { + it('expands legacy boolean true into { commit: true, pr: true }', () => { + const input = { $version: 3, general: { gitCoAuthor: true } }; + const { settings, warnings } = migration.migrate(input, 'user') as { + settings: Record; + warnings: string[]; + }; + + expect( + (settings['general'] as Record)['gitCoAuthor'], + ).toEqual({ commit: true, pr: true }); + expect(settings['$version']).toBe(4); + expect(warnings).toEqual([]); + }); + + it('expands legacy boolean false into { commit: false, pr: false }', () => { + const input = { $version: 3, general: { gitCoAuthor: false } }; + const { settings } = migration.migrate(input, 'user') as { + settings: Record; + warnings: string[]; + }; + + expect( + (settings['general'] as Record)['gitCoAuthor'], + ).toEqual({ commit: false, pr: false }); + }); + + it('leaves an already-object value untouched', () => { + const input = { + $version: 3, + general: { gitCoAuthor: { commit: false, pr: true } }, + }; + const { settings, warnings } = migration.migrate(input, 'user') as { + settings: Record; + warnings: string[]; + }; + + expect( + (settings['general'] as Record)['gitCoAuthor'], + ).toEqual({ commit: false, pr: true }); + expect(warnings).toEqual([]); + }); + + it('bumps version when gitCoAuthor is absent', () => { + const input = { $version: 3, general: {} }; + const { settings, warnings } = migration.migrate(input, 'user') as { + settings: Record; + warnings: string[]; + }; + + expect(settings['$version']).toBe(4); + expect( + (settings['general'] as Record)['gitCoAuthor'], + ).toBeUndefined(); + expect(warnings).toEqual([]); + }); + + it('drops invalid values and emits a warning', () => { + const input = { $version: 3, general: { gitCoAuthor: 'yes' } }; + const { settings, warnings } = migration.migrate(input, 'user') as { + settings: Record; + warnings: string[]; + }; + + expect( + (settings['general'] as Record)['gitCoAuthor'], + ).toEqual({}); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('gitCoAuthor'); + expect(warnings[0]).toContain('user'); + }); + + it('does not mutate the input settings object', () => { + const input = { $version: 3, general: { gitCoAuthor: false } }; + migration.migrate(input, 'user'); + + expect(input).toEqual({ + $version: 3, + general: { gitCoAuthor: false }, + }); + }); + + it('throws for non-object input', () => { + expect(() => migration.migrate(null, 'user')).toThrow(); + expect(() => migration.migrate('string', 'user')).toThrow(); + }); + }); +}); diff --git a/packages/cli/src/config/migration/versions/v3-to-v4.ts b/packages/cli/src/config/migration/versions/v3-to-v4.ts new file mode 100644 index 000000000..34795303a --- /dev/null +++ b/packages/cli/src/config/migration/versions/v3-to-v4.ts @@ -0,0 +1,83 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { SettingsMigration } from '../types.js'; +import { + getNestedProperty, + setNestedPropertySafe, +} from '../../../utils/settingsUtils.js'; + +const GIT_CO_AUTHOR_PATH = 'general.gitCoAuthor'; + +/** + * V3 -> V4 migration (gitCoAuthor boolean → object expansion). + * + * Before V4, `general.gitCoAuthor` was a single boolean that governed both + * commit message attribution and PR body attribution. V4 splits those into + * two independent sub-toggles so users can disable one without losing the + * other. This migration rewrites any stored boolean into `{ commit: v, + * pr: v }` so the user's prior choice carries over to both new toggles and + * the settings dialog reads the expected object shape. + * + * Compatibility strategy: + * - Boolean values are expanded in place. + * - Object values with `commit`/`pr` keys are left untouched (forward- + * compatible — a user who edited their settings.json by hand to the new + * shape is already on V4-equivalent data). + * - Any other present value (string, number, array, null) is dropped with + * a warning so the caller sees an actionable message. + */ +export class V3ToV4Migration implements SettingsMigration { + readonly fromVersion = 3; + readonly toVersion = 4; + + shouldMigrate(settings: unknown): boolean { + if (typeof settings !== 'object' || settings === null) { + return false; + } + const s = settings as Record; + return s['$version'] === 3; + } + + migrate( + settings: unknown, + scope: string, + ): { settings: unknown; warnings: string[] } { + if (typeof settings !== 'object' || settings === null) { + throw new Error('Settings must be an object'); + } + + const result = structuredClone(settings) as Record; + const warnings: string[] = []; + + const value = getNestedProperty(result, GIT_CO_AUTHOR_PATH); + + if (typeof value === 'boolean') { + // Legacy shape — rewrite as { commit, pr } preserving the prior choice. + setNestedPropertySafe(result, GIT_CO_AUTHOR_PATH, { + commit: value, + pr: value, + }); + } else if ( + value !== undefined && + (typeof value !== 'object' || value === null || Array.isArray(value)) + ) { + // Invalid: can't safely interpret. Drop so the schema default (both + // toggles on) applies on next load. + setNestedPropertySafe(result, GIT_CO_AUTHOR_PATH, {}); + warnings.push( + `Reset '${GIT_CO_AUTHOR_PATH}' in ${scope} settings because the stored value was not a boolean or object.`, + ); + } + // Object values (including the new shape) pass through unchanged. + + result['$version'] = 4; + + return { settings: result, warnings }; + } +} + +export const v3ToV4Migration = new V3ToV4Migration(); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 38e86bc9c..b5a774a41 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -65,7 +65,7 @@ export const USER_SETTINGS_DIR = path.dirname(USER_SETTINGS_PATH); export const DEFAULT_EXCLUDED_ENV_VARS = ['DEBUG', 'DEBUG_MODE']; // Settings version to track migration state -export const SETTINGS_VERSION = 3; +export const SETTINGS_VERSION = 4; export const SETTINGS_VERSION_KEY = '$version'; /** diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index d864a44c0..2c1e18f98 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -62,9 +62,20 @@ "default": 5 }, "gitCoAuthor": { - "description": "Automatically add a Co-authored-by trailer to git commit messages when commits are made through Qwen Code.", - "type": "boolean", - "default": true + "description": "Attribution added to git commits and pull requests created through Qwen Code.", + "type": "object", + "properties": { + "commit": { + "description": "Automatically add a Co-authored-by trailer to git commit messages when commits are made through Qwen Code.", + "type": "boolean", + "default": true + }, + "pr": { + "description": "Append a Qwen Code attribution line to PR descriptions when running `gh pr create`.", + "type": "boolean", + "default": true + } + } }, "checkpointing": { "description": "Session checkpointing settings.",