takt: #391 resolveConfigValue の無意味な defaultValue を撲滅する (#392)

This commit is contained in:
nrs 2026-02-28 12:29:24 +09:00 committed by GitHub
parent 6d50221dd5
commit ae74c0d595
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 148 additions and 13 deletions

View File

@ -42,7 +42,7 @@ describe('loadGlobalConfig', () => {
expect(config.logLevel).toBe('info'); expect(config.logLevel).toBe('info');
expect(config.provider).toBe('claude'); expect(config.provider).toBe('claude');
expect(config.model).toBeUndefined(); expect(config.model).toBeUndefined();
expect(config.verbose).toBeUndefined(); expect(config.verbose).toBe(false);
expect(config.pipeline).toBeUndefined(); expect(config.pipeline).toBeUndefined();
}); });

View File

@ -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<string, unknown>;
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);
});
});
});

View File

@ -125,13 +125,13 @@ export interface PersistedGlobalConfig {
/** Number of movement previews to inject into interactive mode (0 to disable, max 10) */ /** Number of movement previews to inject into interactive mode (0 to disable, max 10) */
interactivePreviewMovements?: number; interactivePreviewMovements?: number;
/** Verbose output mode */ /** Verbose output mode */
verbose?: boolean; verbose: boolean;
/** Number of tasks to run concurrently in takt run (default: 1 = sequential) */ /** Number of tasks to run concurrently in takt run (default: 1 = sequential) */
concurrency: number; concurrency: number;
/** Polling interval in ms for picking up new tasks during takt run (default: 500, range: 100-5000) */ /** Polling interval in ms for picking up new tasks during takt run (default: 500, range: 100-5000) */
taskPollIntervalMs: number; taskPollIntervalMs: number;
/** Opt-in: fetch remote before cloning to keep clones up-to-date (default: false) */ /** 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) */ /** Base branch to clone from (default: current branch) */
baseBranch?: string; baseBranch?: string;
} }

View File

@ -489,7 +489,7 @@ export const GlobalConfigSchema = z.object({
/** Number of movement previews to inject into interactive mode (0 to disable, max 10) */ /** 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), interactive_preview_movements: z.number().int().min(0).max(10).optional().default(3),
/** Verbose output mode */ /** 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) */ /** 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), 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) */ /** Polling interval in ms for picking up new tasks during takt run (default: 500, range: 100-5000) */

View File

@ -356,7 +356,7 @@ export class GlobalConfigManager {
if (config.interactivePreviewMovements !== undefined) { if (config.interactivePreviewMovements !== undefined) {
raw.interactive_preview_movements = config.interactivePreviewMovements; raw.interactive_preview_movements = config.interactivePreviewMovements;
} }
if (config.verbose !== undefined) { if (config.verbose) {
raw.verbose = config.verbose; raw.verbose = config.verbose;
} }
if (config.concurrency !== undefined && config.concurrency > 1) { if (config.concurrency !== undefined && config.concurrency > 1) {
@ -365,7 +365,7 @@ export class GlobalConfigManager {
if (config.taskPollIntervalMs !== undefined && config.taskPollIntervalMs !== 500) { if (config.taskPollIntervalMs !== undefined && config.taskPollIntervalMs !== 500) {
raw.task_poll_interval_ms = config.taskPollIntervalMs; raw.task_poll_interval_ms = config.taskPollIntervalMs;
} }
if (config.autoFetch !== undefined) { if (config.autoFetch) {
raw.auto_fetch = config.autoFetch; raw.auto_fetch = config.autoFetch;
} }
if (config.baseBranch) { if (config.baseBranch) {

View File

@ -33,7 +33,6 @@ export interface ResolvedConfigValue<K extends ConfigParameterKey> {
type ResolutionLayer = 'local' | 'piece' | 'global'; type ResolutionLayer = 'local' | 'piece' | 'global';
interface ResolutionRule<K extends ConfigParameterKey> { interface ResolutionRule<K extends ConfigParameterKey> {
layers: readonly ResolutionLayer[]; layers: readonly ResolutionLayer[];
defaultValue?: LoadedConfig[K];
mergeMode?: 'analytics'; mergeMode?: 'analytics';
pieceValue?: (pieceContext: PieceContext | undefined) => LoadedConfig[K] | undefined; pieceValue?: (pieceContext: PieceContext | undefined) => LoadedConfig[K] | undefined;
} }
@ -61,7 +60,7 @@ const PROVIDER_OPTIONS_ENV_PATHS = [
] as const; ] as const;
const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule<K> }> = { const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule<K> }> = {
piece: { layers: ['local', 'global'], defaultValue: 'default' }, piece: { layers: ['local', 'global'] },
provider: { provider: {
layers: ['local', 'piece', 'global'], layers: ['local', 'piece', 'global'],
pieceValue: (pieceContext) => pieceContext?.provider, pieceValue: (pieceContext) => pieceContext?.provider,
@ -77,8 +76,8 @@ const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule<K
autoPr: { layers: ['local', 'global'] }, autoPr: { layers: ['local', 'global'] },
draftPr: { layers: ['local', 'global'] }, draftPr: { layers: ['local', 'global'] },
analytics: { layers: ['local', 'global'], mergeMode: 'analytics' }, analytics: { layers: ['local', 'global'], mergeMode: 'analytics' },
verbose: { layers: ['local', 'global'], defaultValue: false }, verbose: { layers: ['local', 'global'] },
autoFetch: { layers: ['global'], defaultValue: false }, autoFetch: { layers: ['global'] },
baseBranch: { layers: ['local', 'global'] }, baseBranch: { layers: ['local', 'global'] },
}; };
@ -159,7 +158,7 @@ function resolveByRegistry<K extends ConfigParameterKey>(
} }
} }
return { value: rule.defaultValue as LoadedConfig[K], source: 'default' }; return { value: undefined as LoadedConfig[K], source: 'default' };
} }
function hasProviderOptionsEnvOverride(): boolean { function hasProviderOptionsEnvOverride(): boolean {

View File

@ -1,8 +1,7 @@
import type { PersistedGlobalConfig } from '../../core/models/persisted-global-config.js'; import type { PersistedGlobalConfig } from '../../core/models/persisted-global-config.js';
export interface LoadedConfig extends Omit<PersistedGlobalConfig, 'verbose'> { export interface LoadedConfig extends PersistedGlobalConfig {
piece: string; piece: string;
verbose: boolean;
} }
export type ConfigParameterKey = keyof LoadedConfig; export type ConfigParameterKey = keyof LoadedConfig;