From f6d8c353d32d6906710d51501419432e6b25cb5a Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Thu, 26 Feb 2026 02:11:49 +0900 Subject: [PATCH] =?UTF-8?q?refactor:=20provider=20=E3=81=AE=E3=83=87?= =?UTF-8?q?=E3=83=95=E3=82=A9=E3=83=AB=E3=83=88=E5=80=A4=20'claude'=20?= =?UTF-8?q?=E3=82=92=E5=BB=83=E6=AD=A2=E3=81=97=E6=98=8E=E7=A4=BA=E8=A8=AD?= =?UTF-8?q?=E5=AE=9A=E3=82=92=E5=BF=85=E9=A0=88=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 暗黙の claude フォールバックを削除し、未設定時は明確なエラーを返すように変更。 permission は未設定時 readonly にフォールバック。テスト・E2E を新挙動に適合。 --- e2e/helpers/test-repo.ts | 1 + e2e/specs/config-priority.e2e.ts | 34 ++++++++++++------- src/__tests__/it-pipeline-modes.test.ts | 1 + src/__tests__/it-pipeline.test.ts | 1 + src/__tests__/options-builder.test.ts | 4 +-- src/agents/runner.ts | 7 ++-- src/core/piece/engine/OptionsBuilder.ts | 2 +- .../piece/permission-profile-resolution.ts | 7 +--- src/features/tasks/execute/pieceExecution.ts | 5 ++- src/features/tasks/list/taskSyncAction.ts | 5 ++- src/infra/config/resolveConfigValue.ts | 1 - src/infra/config/resolvedConfig.ts | 3 +- src/infra/task/summarize.ts | 5 ++- 13 files changed, 46 insertions(+), 30 deletions(-) diff --git a/e2e/helpers/test-repo.ts b/e2e/helpers/test-repo.ts index 35cd4f1..e3d4adf 100644 --- a/e2e/helpers/test-repo.ts +++ b/e2e/helpers/test-repo.ts @@ -43,6 +43,7 @@ export interface CreateTestRepoOptions { function getGitHubUser(): string { const user = execFileSync('gh', ['api', 'user', '--jq', '.login'], { encoding: 'utf-8', + stdio: 'pipe', }).trim(); if (!user) { diff --git a/e2e/specs/config-priority.e2e.ts b/e2e/specs/config-priority.e2e.ts index 672155f..e782e6b 100644 --- a/e2e/specs/config-priority.e2e.ts +++ b/e2e/specs/config-priority.e2e.ts @@ -1,7 +1,8 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { dirname, join, resolve } from 'node:path'; 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 { createTestRepo, type TestRepo } from '../helpers/test-repo'; import { runTakt } from '../helpers/takt-runner'; @@ -9,6 +10,17 @@ import { runTakt } from '../helpers/takt-runner'; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); +function readFirstTask(repoPath: string): Record { + const tasksPath = join(repoPath, '.takt', 'tasks.yaml'); + const raw = readFileSync(tasksPath, 'utf-8'); + const parsed = parseYaml(raw) as { tasks?: Array> } | null; + const first = parsed?.tasks?.[0]; + if (!first) { + throw new Error(`No task record found in ${tasksPath}`); + } + return first; +} + // E2E更新時は docs/testing/e2e.md も更新すること describe('E2E: Config priority (piece / autoPr)', () => { let isolatedEnv: IsolatedEnv; @@ -67,7 +79,7 @@ describe('E2E: Config priority (piece / autoPr)', () => { const piecePath = resolve(__dirname, '../fixtures/pieces/mock-single-step.yaml'); const scenarioPath = resolve(__dirname, '../fixtures/scenarios/execute-done.json'); - const result = runTakt({ + runTakt({ args: [ '--task', 'Auto PR default behavior', '--piece', piecePath, @@ -82,10 +94,8 @@ describe('E2E: Config priority (piece / autoPr)', () => { timeout: 240_000, }); - // auto_pr=true の場合は PR 作成フローに入り、テスト環境では gh 未認証のため失敗する - const output = result.stdout + result.stderr; - expect(result.exitCode).toBe(1); - expect(output).toContain('PR creation failed:'); + const task = readFirstTask(testRepo.path); + expect(task['auto_pr']).toBe(true); }, 240_000); it('should use auto_pr from config when set', () => { @@ -108,9 +118,9 @@ describe('E2E: Config priority (piece / autoPr)', () => { timeout: 240_000, }); - const output = result.stdout + result.stderr; 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); 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'); updateIsolatedConfig(isolatedEnv.taktDir, { auto_pr: false }); - const result = runTakt({ + runTakt({ args: [ '--task', 'Auto PR from env override', '--piece', piecePath, @@ -134,9 +144,7 @@ describe('E2E: Config priority (piece / autoPr)', () => { timeout: 240_000, }); - // env override により auto_pr=true が優先され、PR 作成フローに入る - const output = result.stdout + result.stderr; - expect(result.exitCode).toBe(1); - expect(output).toContain('PR creation failed:'); + const task = readFirstTask(testRepo.path); + expect(task['auto_pr']).toBe(true); }, 240_000); }); diff --git a/src/__tests__/it-pipeline-modes.test.ts b/src/__tests__/it-pipeline-modes.test.ts index 1e3a66b..90212d2 100644 --- a/src/__tests__/it-pipeline-modes.test.ts +++ b/src/__tests__/it-pipeline-modes.test.ts @@ -115,6 +115,7 @@ vi.mock('../infra/config/global/globalConfig.js', async (importOriginal) => { ...original, loadGlobalConfig: vi.fn().mockReturnValue({ language: 'en', + provider: 'mock', enableBuiltinPieces: true, disabledBuiltins: [], }), diff --git a/src/__tests__/it-pipeline.test.ts b/src/__tests__/it-pipeline.test.ts index 264a879..dc89c24 100644 --- a/src/__tests__/it-pipeline.test.ts +++ b/src/__tests__/it-pipeline.test.ts @@ -96,6 +96,7 @@ vi.mock('../infra/config/global/globalConfig.js', async (importOriginal) => { ...original, loadGlobalConfig: vi.fn().mockReturnValue({ language: 'en', + provider: 'mock', enableBuiltinPieces: true, disabledBuiltins: [], }), diff --git a/src/__tests__/options-builder.test.ts b/src/__tests__/options-builder.test.ts index 5f14d37..9acc219 100644 --- a/src/__tests__/options-builder.test.ts +++ b/src/__tests__/options-builder.test.ts @@ -57,7 +57,7 @@ describe('OptionsBuilder.buildBaseOptions', () => { 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 builder = createBuilder(step, { provider: undefined, @@ -65,7 +65,7 @@ describe('OptionsBuilder.buildBaseOptions', () => { }); const options = builder.buildBaseOptions(step); - expect(options.permissionMode).toBe('edit'); + expect(options.permissionMode).toBe('readonly'); }); it('merges provider options with precedence: global < movement < project', () => { diff --git a/src/agents/runner.ts b/src/agents/runner.ts index c62f2c7..1fe6645 100644 --- a/src/agents/runner.ts +++ b/src/agents/runner.ts @@ -34,9 +34,12 @@ export class AgentRunner { { provider: options?.stepProvider }, { provider: config.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 : undefined; const resolvedModel = resolveProviderModelCandidates([ diff --git a/src/core/piece/engine/OptionsBuilder.ts b/src/core/piece/engine/OptionsBuilder.ts index 089d70a..8515e19 100644 --- a/src/core/piece/engine/OptionsBuilder.ts +++ b/src/core/piece/engine/OptionsBuilder.ts @@ -64,7 +64,7 @@ export class OptionsBuilder { model: this.engineOptions.model, 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; return { diff --git a/src/core/piece/permission-profile-resolution.ts b/src/core/piece/permission-profile-resolution.ts index 340626f..a877aef 100644 --- a/src/core/piece/permission-profile-resolution.ts +++ b/src/core/piece/permission-profile-resolution.ts @@ -30,12 +30,7 @@ export const DEFAULT_PROVIDER_PERMISSION_PROFILES: ProviderPermissionProfiles = */ export function resolveMovementPermissionMode(input: ResolvePermissionModeInput): PermissionMode { if (!input.provider) { - if (input.requiredPermissionMode) { - 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.`, - ); + return input.requiredPermissionMode ?? 'readonly'; } const projectProfile = input.projectProviderProfiles?.[input.provider]; diff --git a/src/features/tasks/execute/pieceExecution.ts b/src/features/tasks/execute/pieceExecution.ts index 59331e9..0e8e888 100644 --- a/src/features/tasks/execute/pieceExecution.ts +++ b/src/features/tasks/execute/pieceExecution.ts @@ -340,6 +340,9 @@ export async function executePiece( const shouldNotifyPieceComplete = shouldNotify && notificationSoundEvents?.pieceComplete !== false; const shouldNotifyPieceAbort = shouldNotify && notificationSoundEvents?.pieceAbort !== false; const currentProvider = globalConfig.provider; + if (!currentProvider) { + throw new Error('No provider configured. Set "provider" in ~/.takt/config.yaml'); + } const effectivePieceConfig: PieceConfig = { ...pieceConfig, runtime: resolveRuntimeConfig(globalConfig.runtime, pieceConfig.runtime), @@ -555,7 +558,7 @@ export async function executePiece( model: options.model, personaProviders: options.personaProviders, }); - const movementProvider = resolved.provider ?? 'claude'; + const movementProvider = resolved.provider ?? options.provider ?? currentProvider; const resolvedModel = resolved.model; const movementModel = resolvedModel ?? '(default)'; currentMovementProvider = movementProvider; diff --git a/src/features/tasks/list/taskSyncAction.ts b/src/features/tasks/list/taskSyncAction.ts index 0c0eb0c..4501f03 100644 --- a/src/features/tasks/list/taskSyncAction.ts +++ b/src/features/tasks/list/taskSyncAction.ts @@ -70,7 +70,10 @@ export async function syncBranchWithRoot( const prompt = loadTemplate('sync_conflict_resolver_message', lang, { originalInstruction }); 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 agent = provider.setup({ name: 'conflict-resolver', systemPrompt }); diff --git a/src/infra/config/resolveConfigValue.ts b/src/infra/config/resolveConfigValue.ts index 5125e5c..1b87722 100644 --- a/src/infra/config/resolveConfigValue.ts +++ b/src/infra/config/resolveConfigValue.ts @@ -64,7 +64,6 @@ const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule pieceContext?.provider, }, model: { diff --git a/src/infra/config/resolvedConfig.ts b/src/infra/config/resolvedConfig.ts index 820dc73..0aecf1c 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 Omit { piece: string; - provider: NonNullable; verbose: boolean; } diff --git a/src/infra/task/summarize.ts b/src/infra/task/summarize.ts index 17ba84f..47f5a90 100644 --- a/src/infra/task/summarize.ts +++ b/src/infra/task/summarize.ts @@ -67,7 +67,10 @@ export class TaskSummarizer { log.info('Task name romanized', { original: taskName, slug }); 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 provider = getProvider(providerType);