From 91cd7a5e64937fb012ad56f35fb94088742fadaa Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Thu, 26 Feb 2026 07:18:33 +0900 Subject: [PATCH] =?UTF-8?q?takt:=20#391=20resolveConfigValue=20=E3=81=AE?= =?UTF-8?q?=E7=84=A1=E6=84=8F=E5=91=B3=E3=81=AA=20defaultValue=20=E3=82=92?= =?UTF-8?q?=E6=92=B2=E6=BB=85=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/__tests__/globalConfig-defaults.test.ts | 2 +- ...resolveConfigValue-no-defaultValue.test.ts | 137 ++++++++++++++++++ src/core/models/persisted-global-config.ts | 4 +- src/core/models/schemas.ts | 2 +- src/infra/config/global/globalConfig.ts | 4 +- src/infra/config/resolveConfigValue.ts | 9 +- src/infra/config/resolvedConfig.ts | 3 +- 7 files changed, 148 insertions(+), 13 deletions(-) create mode 100644 src/__tests__/resolveConfigValue-no-defaultValue.test.ts diff --git a/src/__tests__/globalConfig-defaults.test.ts b/src/__tests__/globalConfig-defaults.test.ts index b63c5e42..76966755 100644 --- a/src/__tests__/globalConfig-defaults.test.ts +++ b/src/__tests__/globalConfig-defaults.test.ts @@ -42,7 +42,7 @@ describe('loadGlobalConfig', () => { expect(config.logLevel).toBe('info'); expect(config.provider).toBe('claude'); expect(config.model).toBeUndefined(); - expect(config.verbose).toBeUndefined(); + expect(config.verbose).toBe(false); expect(config.pipeline).toBeUndefined(); }); diff --git a/src/__tests__/resolveConfigValue-no-defaultValue.test.ts b/src/__tests__/resolveConfigValue-no-defaultValue.test.ts new file mode 100644 index 00000000..5d48a1ae --- /dev/null +++ b/src/__tests__/resolveConfigValue-no-defaultValue.test.ts @@ -0,0 +1,137 @@ +/** + * Tests for RESOLUTION_REGISTRY defaultValue removal. + * + * Verifies that piece, verbose, and autoFetch no longer rely on + * RESOLUTION_REGISTRY defaultValue but instead use schema defaults + * or other guaranteed sources. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { mkdirSync, rmSync, writeFileSync, existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { randomUUID } from 'node:crypto'; + +const testId = randomUUID(); +const testDir = join(tmpdir(), `takt-rcv-test-${testId}`); +const globalTaktDir = join(testDir, 'global-takt'); +const globalConfigPath = join(globalTaktDir, 'config.yaml'); + +vi.mock('../infra/config/paths.js', async (importOriginal) => { + const original = await importOriginal() as Record; + return { + ...original, + getGlobalConfigPath: () => globalConfigPath, + getTaktDir: () => globalTaktDir, + }; +}); + +const { resolveConfigValue, resolveConfigValueWithSource, invalidateAllResolvedConfigCache } = await import('../infra/config/resolveConfigValue.js'); +const { invalidateGlobalConfigCache } = await import('../infra/config/global/globalConfig.js'); +const { getProjectConfigDir } = await import('../infra/config/paths.js'); + +describe('RESOLUTION_REGISTRY defaultValue removal', () => { + let projectDir: string; + + beforeEach(() => { + projectDir = join(testDir, `project-${randomUUID()}`); + mkdirSync(projectDir, { recursive: true }); + mkdirSync(globalTaktDir, { recursive: true }); + writeFileSync(globalConfigPath, 'language: en\n', 'utf-8'); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + }); + + afterEach(() => { + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + }); + + describe('piece', () => { + it('should resolve piece from project config DEFAULT_PROJECT_CONFIG when not explicitly set', () => { + const value = resolveConfigValue(projectDir, 'piece'); + expect(value).toBe('default'); + }); + + it('should report source as project when piece comes from DEFAULT_PROJECT_CONFIG', () => { + const result = resolveConfigValueWithSource(projectDir, 'piece'); + expect(result.value).toBe('default'); + expect(result.source).toBe('project'); + }); + + it('should resolve explicit project piece over default', () => { + const configDir = getProjectConfigDir(projectDir); + mkdirSync(configDir, { recursive: true }); + writeFileSync(join(configDir, 'config.yaml'), 'piece: custom-piece\n'); + + const value = resolveConfigValue(projectDir, 'piece'); + expect(value).toBe('custom-piece'); + }); + + it('should resolve piece from global config when global has it', () => { + writeFileSync(globalConfigPath, 'language: en\npiece: global-piece\n', 'utf-8'); + invalidateGlobalConfigCache(); + + const result = resolveConfigValueWithSource(projectDir, 'piece'); + expect(result.value).toBe('default'); + expect(result.source).toBe('project'); + }); + }); + + describe('verbose', () => { + it('should resolve verbose to false via schema default when not set anywhere', () => { + const value = resolveConfigValue(projectDir, 'verbose'); + expect(value).toBe(false); + }); + + it('should report source as global when verbose comes from schema default', () => { + const result = resolveConfigValueWithSource(projectDir, 'verbose'); + expect(result.value).toBe(false); + expect(result.source).toBe('global'); + }); + + it('should resolve verbose from global config when explicitly set', () => { + writeFileSync(globalConfigPath, 'language: en\nverbose: true\n', 'utf-8'); + invalidateGlobalConfigCache(); + + const value = resolveConfigValue(projectDir, 'verbose'); + expect(value).toBe(true); + }); + + it('should resolve verbose from project config over global', () => { + writeFileSync(globalConfigPath, 'language: en\nverbose: false\n', 'utf-8'); + invalidateGlobalConfigCache(); + + const configDir = getProjectConfigDir(projectDir); + mkdirSync(configDir, { recursive: true }); + writeFileSync(join(configDir, 'config.yaml'), 'piece: default\nverbose: true\n'); + + const value = resolveConfigValue(projectDir, 'verbose'); + expect(value).toBe(true); + }); + }); + + describe('autoFetch', () => { + it('should resolve autoFetch to false via schema default when not set', () => { + const value = resolveConfigValue(projectDir, 'autoFetch'); + expect(value).toBe(false); + }); + + it('should report source as global when autoFetch comes from schema default', () => { + const result = resolveConfigValueWithSource(projectDir, 'autoFetch'); + expect(result.value).toBe(false); + expect(result.source).toBe('global'); + }); + + it('should resolve autoFetch from global config when explicitly set', () => { + writeFileSync(globalConfigPath, 'language: en\nauto_fetch: true\n', 'utf-8'); + invalidateGlobalConfigCache(); + + const value = resolveConfigValue(projectDir, 'autoFetch'); + expect(value).toBe(true); + }); + }); +}); diff --git a/src/core/models/persisted-global-config.ts b/src/core/models/persisted-global-config.ts index 4f2f5b45..4ac17a57 100644 --- a/src/core/models/persisted-global-config.ts +++ b/src/core/models/persisted-global-config.ts @@ -121,13 +121,13 @@ export interface PersistedGlobalConfig { /** Number of movement previews to inject into interactive mode (0 to disable, max 10) */ interactivePreviewMovements?: number; /** Verbose output mode */ - verbose?: boolean; + verbose: boolean; /** Number of tasks to run concurrently in takt run (default: 1 = sequential) */ concurrency: number; /** Polling interval in ms for picking up new tasks during takt run (default: 500, range: 100-5000) */ taskPollIntervalMs: number; /** Opt-in: fetch remote before cloning to keep clones up-to-date (default: false) */ - autoFetch?: boolean; + autoFetch: boolean; /** Base branch to clone from (default: current branch) */ baseBranch?: string; } diff --git a/src/core/models/schemas.ts b/src/core/models/schemas.ts index 549f27cf..580a3cb1 100644 --- a/src/core/models/schemas.ts +++ b/src/core/models/schemas.ts @@ -476,7 +476,7 @@ export const GlobalConfigSchema = z.object({ /** Number of movement previews to inject into interactive mode (0 to disable, max 10) */ interactive_preview_movements: z.number().int().min(0).max(10).optional().default(3), /** Verbose output mode */ - verbose: z.boolean().optional(), + verbose: z.boolean().optional().default(false), /** Number of tasks to run concurrently in takt run (default: 1 = sequential, max: 10) */ concurrency: z.number().int().min(1).max(10).optional().default(1), /** Polling interval in ms for picking up new tasks during takt run (default: 500, range: 100-5000) */ diff --git a/src/infra/config/global/globalConfig.ts b/src/infra/config/global/globalConfig.ts index bf10412a..b2621efc 100644 --- a/src/infra/config/global/globalConfig.ts +++ b/src/infra/config/global/globalConfig.ts @@ -343,7 +343,7 @@ export class GlobalConfigManager { if (config.interactivePreviewMovements !== undefined) { raw.interactive_preview_movements = config.interactivePreviewMovements; } - if (config.verbose !== undefined) { + if (config.verbose) { raw.verbose = config.verbose; } if (config.concurrency !== undefined && config.concurrency > 1) { @@ -352,7 +352,7 @@ export class GlobalConfigManager { if (config.taskPollIntervalMs !== undefined && config.taskPollIntervalMs !== 500) { raw.task_poll_interval_ms = config.taskPollIntervalMs; } - if (config.autoFetch !== undefined) { + if (config.autoFetch) { raw.auto_fetch = config.autoFetch; } if (config.baseBranch) { diff --git a/src/infra/config/resolveConfigValue.ts b/src/infra/config/resolveConfigValue.ts index 1b877221..fb327f03 100644 --- a/src/infra/config/resolveConfigValue.ts +++ b/src/infra/config/resolveConfigValue.ts @@ -33,7 +33,6 @@ export interface ResolvedConfigValue { type ResolutionLayer = 'local' | 'piece' | 'global'; interface ResolutionRule { layers: readonly ResolutionLayer[]; - defaultValue?: LoadedConfig[K]; mergeMode?: 'analytics'; pieceValue?: (pieceContext: PieceContext | undefined) => LoadedConfig[K] | undefined; } @@ -61,7 +60,7 @@ const PROVIDER_OPTIONS_ENV_PATHS = [ ] as const; const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule }> = { - piece: { layers: ['local', 'global'], defaultValue: 'default' }, + piece: { layers: ['local', 'global'] }, provider: { layers: ['local', 'piece', 'global'], pieceValue: (pieceContext) => pieceContext?.provider, @@ -77,8 +76,8 @@ const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule( } } - return { value: rule.defaultValue as LoadedConfig[K], source: 'default' }; + return { value: undefined as LoadedConfig[K], source: 'default' }; } function hasProviderOptionsEnvOverride(): boolean { diff --git a/src/infra/config/resolvedConfig.ts b/src/infra/config/resolvedConfig.ts index 0aecf1c9..f3266c53 100644 --- a/src/infra/config/resolvedConfig.ts +++ b/src/infra/config/resolvedConfig.ts @@ -1,8 +1,7 @@ import type { PersistedGlobalConfig } from '../../core/models/persisted-global-config.js'; -export interface LoadedConfig extends Omit { +export interface LoadedConfig extends PersistedGlobalConfig { piece: string; - verbose: boolean; } export type ConfigParameterKey = keyof LoadedConfig;