refactor: provider のデフォルト値 'claude' を廃止し明示設定を必須化
暗黙の claude フォールバックを削除し、未設定時は明確なエラーを返すように変更。 permission は未設定時 readonly にフォールバック。テスト・E2E を新挙動に適合。
This commit is contained in:
parent
6d0bac9d07
commit
f6d8c353d3
@ -43,6 +43,7 @@ export interface CreateTestRepoOptions {
|
|||||||
function getGitHubUser(): string {
|
function getGitHubUser(): string {
|
||||||
const user = execFileSync('gh', ['api', 'user', '--jq', '.login'], {
|
const user = execFileSync('gh', ['api', 'user', '--jq', '.login'], {
|
||||||
encoding: 'utf-8',
|
encoding: 'utf-8',
|
||||||
|
stdio: 'pipe',
|
||||||
}).trim();
|
}).trim();
|
||||||
|
|
||||||
if (!user) {
|
if (!user) {
|
||||||
|
|||||||
@ -1,7 +1,8 @@
|
|||||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||||
import { dirname, join, resolve } from 'node:path';
|
import { dirname, join, resolve } from 'node:path';
|
||||||
import { fileURLToPath } from 'node:url';
|
import { fileURLToPath } from 'node:url';
|
||||||
import { mkdirSync, writeFileSync } from 'node:fs';
|
import { mkdirSync, readFileSync, writeFileSync } from 'node:fs';
|
||||||
|
import { parse as parseYaml } from 'yaml';
|
||||||
import { createIsolatedEnv, updateIsolatedConfig, type IsolatedEnv } from '../helpers/isolated-env';
|
import { createIsolatedEnv, updateIsolatedConfig, type IsolatedEnv } from '../helpers/isolated-env';
|
||||||
import { createTestRepo, type TestRepo } from '../helpers/test-repo';
|
import { createTestRepo, type TestRepo } from '../helpers/test-repo';
|
||||||
import { runTakt } from '../helpers/takt-runner';
|
import { runTakt } from '../helpers/takt-runner';
|
||||||
@ -9,6 +10,17 @@ import { runTakt } from '../helpers/takt-runner';
|
|||||||
const __filename = fileURLToPath(import.meta.url);
|
const __filename = fileURLToPath(import.meta.url);
|
||||||
const __dirname = dirname(__filename);
|
const __dirname = dirname(__filename);
|
||||||
|
|
||||||
|
function readFirstTask(repoPath: string): Record<string, unknown> {
|
||||||
|
const tasksPath = join(repoPath, '.takt', 'tasks.yaml');
|
||||||
|
const raw = readFileSync(tasksPath, 'utf-8');
|
||||||
|
const parsed = parseYaml(raw) as { tasks?: Array<Record<string, unknown>> } | null;
|
||||||
|
const first = parsed?.tasks?.[0];
|
||||||
|
if (!first) {
|
||||||
|
throw new Error(`No task record found in ${tasksPath}`);
|
||||||
|
}
|
||||||
|
return first;
|
||||||
|
}
|
||||||
|
|
||||||
// E2E更新時は docs/testing/e2e.md も更新すること
|
// E2E更新時は docs/testing/e2e.md も更新すること
|
||||||
describe('E2E: Config priority (piece / autoPr)', () => {
|
describe('E2E: Config priority (piece / autoPr)', () => {
|
||||||
let isolatedEnv: IsolatedEnv;
|
let isolatedEnv: IsolatedEnv;
|
||||||
@ -67,7 +79,7 @@ describe('E2E: Config priority (piece / autoPr)', () => {
|
|||||||
const piecePath = resolve(__dirname, '../fixtures/pieces/mock-single-step.yaml');
|
const piecePath = resolve(__dirname, '../fixtures/pieces/mock-single-step.yaml');
|
||||||
const scenarioPath = resolve(__dirname, '../fixtures/scenarios/execute-done.json');
|
const scenarioPath = resolve(__dirname, '../fixtures/scenarios/execute-done.json');
|
||||||
|
|
||||||
const result = runTakt({
|
runTakt({
|
||||||
args: [
|
args: [
|
||||||
'--task', 'Auto PR default behavior',
|
'--task', 'Auto PR default behavior',
|
||||||
'--piece', piecePath,
|
'--piece', piecePath,
|
||||||
@ -82,10 +94,8 @@ describe('E2E: Config priority (piece / autoPr)', () => {
|
|||||||
timeout: 240_000,
|
timeout: 240_000,
|
||||||
});
|
});
|
||||||
|
|
||||||
// auto_pr=true の場合は PR 作成フローに入り、テスト環境では gh 未認証のため失敗する
|
const task = readFirstTask(testRepo.path);
|
||||||
const output = result.stdout + result.stderr;
|
expect(task['auto_pr']).toBe(true);
|
||||||
expect(result.exitCode).toBe(1);
|
|
||||||
expect(output).toContain('PR creation failed:');
|
|
||||||
}, 240_000);
|
}, 240_000);
|
||||||
|
|
||||||
it('should use auto_pr from config when set', () => {
|
it('should use auto_pr from config when set', () => {
|
||||||
@ -108,9 +118,9 @@ describe('E2E: Config priority (piece / autoPr)', () => {
|
|||||||
timeout: 240_000,
|
timeout: 240_000,
|
||||||
});
|
});
|
||||||
|
|
||||||
const output = result.stdout + result.stderr;
|
|
||||||
expect(result.exitCode).toBe(0);
|
expect(result.exitCode).toBe(0);
|
||||||
expect(output).not.toContain('PR creation failed:');
|
const task = readFirstTask(testRepo.path);
|
||||||
|
expect(task['auto_pr']).toBe(false);
|
||||||
}, 240_000);
|
}, 240_000);
|
||||||
|
|
||||||
it('should prioritize env auto_pr over config', () => {
|
it('should prioritize env auto_pr over config', () => {
|
||||||
@ -118,7 +128,7 @@ describe('E2E: Config priority (piece / autoPr)', () => {
|
|||||||
const scenarioPath = resolve(__dirname, '../fixtures/scenarios/execute-done.json');
|
const scenarioPath = resolve(__dirname, '../fixtures/scenarios/execute-done.json');
|
||||||
updateIsolatedConfig(isolatedEnv.taktDir, { auto_pr: false });
|
updateIsolatedConfig(isolatedEnv.taktDir, { auto_pr: false });
|
||||||
|
|
||||||
const result = runTakt({
|
runTakt({
|
||||||
args: [
|
args: [
|
||||||
'--task', 'Auto PR from env override',
|
'--task', 'Auto PR from env override',
|
||||||
'--piece', piecePath,
|
'--piece', piecePath,
|
||||||
@ -134,9 +144,7 @@ describe('E2E: Config priority (piece / autoPr)', () => {
|
|||||||
timeout: 240_000,
|
timeout: 240_000,
|
||||||
});
|
});
|
||||||
|
|
||||||
// env override により auto_pr=true が優先され、PR 作成フローに入る
|
const task = readFirstTask(testRepo.path);
|
||||||
const output = result.stdout + result.stderr;
|
expect(task['auto_pr']).toBe(true);
|
||||||
expect(result.exitCode).toBe(1);
|
|
||||||
expect(output).toContain('PR creation failed:');
|
|
||||||
}, 240_000);
|
}, 240_000);
|
||||||
});
|
});
|
||||||
|
|||||||
@ -115,6 +115,7 @@ vi.mock('../infra/config/global/globalConfig.js', async (importOriginal) => {
|
|||||||
...original,
|
...original,
|
||||||
loadGlobalConfig: vi.fn().mockReturnValue({
|
loadGlobalConfig: vi.fn().mockReturnValue({
|
||||||
language: 'en',
|
language: 'en',
|
||||||
|
provider: 'mock',
|
||||||
enableBuiltinPieces: true,
|
enableBuiltinPieces: true,
|
||||||
disabledBuiltins: [],
|
disabledBuiltins: [],
|
||||||
}),
|
}),
|
||||||
|
|||||||
@ -96,6 +96,7 @@ vi.mock('../infra/config/global/globalConfig.js', async (importOriginal) => {
|
|||||||
...original,
|
...original,
|
||||||
loadGlobalConfig: vi.fn().mockReturnValue({
|
loadGlobalConfig: vi.fn().mockReturnValue({
|
||||||
language: 'en',
|
language: 'en',
|
||||||
|
provider: 'mock',
|
||||||
enableBuiltinPieces: true,
|
enableBuiltinPieces: true,
|
||||||
disabledBuiltins: [],
|
disabledBuiltins: [],
|
||||||
}),
|
}),
|
||||||
|
|||||||
@ -57,7 +57,7 @@ describe('OptionsBuilder.buildBaseOptions', () => {
|
|||||||
expect(options.permissionMode).toBe('full');
|
expect(options.permissionMode).toBe('full');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('uses default profile when provider_profiles are not provided', () => {
|
it('uses readonly when provider is not configured', () => {
|
||||||
const step = createMovement();
|
const step = createMovement();
|
||||||
const builder = createBuilder(step, {
|
const builder = createBuilder(step, {
|
||||||
provider: undefined,
|
provider: undefined,
|
||||||
@ -65,7 +65,7 @@ describe('OptionsBuilder.buildBaseOptions', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const options = builder.buildBaseOptions(step);
|
const options = builder.buildBaseOptions(step);
|
||||||
expect(options.permissionMode).toBe('edit');
|
expect(options.permissionMode).toBe('readonly');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('merges provider options with precedence: global < movement < project', () => {
|
it('merges provider options with precedence: global < movement < project', () => {
|
||||||
|
|||||||
@ -34,9 +34,12 @@ export class AgentRunner {
|
|||||||
{ provider: options?.stepProvider },
|
{ provider: options?.stepProvider },
|
||||||
{ provider: config.provider },
|
{ provider: config.provider },
|
||||||
{ provider: agentConfig?.provider },
|
{ provider: agentConfig?.provider },
|
||||||
]).provider ?? 'claude';
|
]).provider;
|
||||||
|
if (!resolvedProvider) {
|
||||||
|
throw new Error('No provider configured. Set "provider" in ~/.takt/config.yaml');
|
||||||
|
}
|
||||||
|
|
||||||
const configModel = (config.provider ?? 'claude') === resolvedProvider
|
const configModel = config.provider === resolvedProvider
|
||||||
? config.model
|
? config.model
|
||||||
: undefined;
|
: undefined;
|
||||||
const resolvedModel = resolveProviderModelCandidates([
|
const resolvedModel = resolveProviderModelCandidates([
|
||||||
|
|||||||
@ -64,7 +64,7 @@ export class OptionsBuilder {
|
|||||||
model: this.engineOptions.model,
|
model: this.engineOptions.model,
|
||||||
personaProviders: this.engineOptions.personaProviders,
|
personaProviders: this.engineOptions.personaProviders,
|
||||||
});
|
});
|
||||||
const resolvedProvider = resolved.provider ?? this.engineOptions.provider ?? 'claude';
|
const resolvedProvider = resolved.provider ?? this.engineOptions.provider;
|
||||||
const resolvedModel = resolved.model ?? this.engineOptions.model;
|
const resolvedModel = resolved.model ?? this.engineOptions.model;
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
@ -30,12 +30,7 @@ export const DEFAULT_PROVIDER_PERMISSION_PROFILES: ProviderPermissionProfiles =
|
|||||||
*/
|
*/
|
||||||
export function resolveMovementPermissionMode(input: ResolvePermissionModeInput): PermissionMode {
|
export function resolveMovementPermissionMode(input: ResolvePermissionModeInput): PermissionMode {
|
||||||
if (!input.provider) {
|
if (!input.provider) {
|
||||||
if (input.requiredPermissionMode) {
|
return input.requiredPermissionMode ?? 'readonly';
|
||||||
return input.requiredPermissionMode;
|
|
||||||
}
|
|
||||||
throw new Error(
|
|
||||||
`Unable to resolve permission mode for movement "${input.movementName}": provider is required when movement.required_permission_mode is omitted.`,
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const projectProfile = input.projectProviderProfiles?.[input.provider];
|
const projectProfile = input.projectProviderProfiles?.[input.provider];
|
||||||
|
|||||||
@ -340,6 +340,9 @@ export async function executePiece(
|
|||||||
const shouldNotifyPieceComplete = shouldNotify && notificationSoundEvents?.pieceComplete !== false;
|
const shouldNotifyPieceComplete = shouldNotify && notificationSoundEvents?.pieceComplete !== false;
|
||||||
const shouldNotifyPieceAbort = shouldNotify && notificationSoundEvents?.pieceAbort !== false;
|
const shouldNotifyPieceAbort = shouldNotify && notificationSoundEvents?.pieceAbort !== false;
|
||||||
const currentProvider = globalConfig.provider;
|
const currentProvider = globalConfig.provider;
|
||||||
|
if (!currentProvider) {
|
||||||
|
throw new Error('No provider configured. Set "provider" in ~/.takt/config.yaml');
|
||||||
|
}
|
||||||
const effectivePieceConfig: PieceConfig = {
|
const effectivePieceConfig: PieceConfig = {
|
||||||
...pieceConfig,
|
...pieceConfig,
|
||||||
runtime: resolveRuntimeConfig(globalConfig.runtime, pieceConfig.runtime),
|
runtime: resolveRuntimeConfig(globalConfig.runtime, pieceConfig.runtime),
|
||||||
@ -555,7 +558,7 @@ export async function executePiece(
|
|||||||
model: options.model,
|
model: options.model,
|
||||||
personaProviders: options.personaProviders,
|
personaProviders: options.personaProviders,
|
||||||
});
|
});
|
||||||
const movementProvider = resolved.provider ?? 'claude';
|
const movementProvider = resolved.provider ?? options.provider ?? currentProvider;
|
||||||
const resolvedModel = resolved.model;
|
const resolvedModel = resolved.model;
|
||||||
const movementModel = resolvedModel ?? '(default)';
|
const movementModel = resolvedModel ?? '(default)';
|
||||||
currentMovementProvider = movementProvider;
|
currentMovementProvider = movementProvider;
|
||||||
|
|||||||
@ -70,7 +70,10 @@ export async function syncBranchWithRoot(
|
|||||||
const prompt = loadTemplate('sync_conflict_resolver_message', lang, { originalInstruction });
|
const prompt = loadTemplate('sync_conflict_resolver_message', lang, { originalInstruction });
|
||||||
|
|
||||||
const config = resolveConfigValues(projectDir, ['provider', 'model']);
|
const config = resolveConfigValues(projectDir, ['provider', 'model']);
|
||||||
const providerType = (config.provider ?? 'claude') as ProviderType;
|
if (!config.provider) {
|
||||||
|
throw new Error('No provider configured. Set "provider" in ~/.takt/config.yaml');
|
||||||
|
}
|
||||||
|
const providerType = config.provider as ProviderType;
|
||||||
const provider = getProvider(providerType);
|
const provider = getProvider(providerType);
|
||||||
const agent = provider.setup({ name: 'conflict-resolver', systemPrompt });
|
const agent = provider.setup({ name: 'conflict-resolver', systemPrompt });
|
||||||
|
|
||||||
|
|||||||
@ -64,7 +64,6 @@ const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule<K
|
|||||||
piece: { layers: ['local', 'global'], defaultValue: 'default' },
|
piece: { layers: ['local', 'global'], defaultValue: 'default' },
|
||||||
provider: {
|
provider: {
|
||||||
layers: ['local', 'piece', 'global'],
|
layers: ['local', 'piece', 'global'],
|
||||||
defaultValue: 'claude',
|
|
||||||
pieceValue: (pieceContext) => pieceContext?.provider,
|
pieceValue: (pieceContext) => pieceContext?.provider,
|
||||||
},
|
},
|
||||||
model: {
|
model: {
|
||||||
|
|||||||
@ -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, 'provider' | 'verbose'> {
|
export interface LoadedConfig extends Omit<PersistedGlobalConfig, 'verbose'> {
|
||||||
piece: string;
|
piece: string;
|
||||||
provider: NonNullable<PersistedGlobalConfig['provider']>;
|
|
||||||
verbose: boolean;
|
verbose: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -67,7 +67,10 @@ export class TaskSummarizer {
|
|||||||
log.info('Task name romanized', { original: taskName, slug });
|
log.info('Task name romanized', { original: taskName, slug });
|
||||||
return slug || 'task';
|
return slug || 'task';
|
||||||
}
|
}
|
||||||
const providerType = (globalConfig.provider as ProviderType) ?? 'claude';
|
if (!globalConfig.provider) {
|
||||||
|
throw new Error('No provider configured. Set "provider" in ~/.takt/config.yaml');
|
||||||
|
}
|
||||||
|
const providerType = globalConfig.provider as ProviderType;
|
||||||
const model = options.model ?? globalConfig.model;
|
const model = options.model ?? globalConfig.model;
|
||||||
|
|
||||||
const provider = getProvider(providerType);
|
const provider = getProvider(providerType);
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user