diff --git a/docs/agents.md b/docs/agents.md index bf32245..0f252ec 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -78,8 +78,6 @@ agents: - Read - Glob - Grep - provider: claude # Optional: claude, codex, or opencode - model: opus # Optional: model alias or full name ``` ### Agent Configuration Options @@ -90,8 +88,6 @@ agents: | `prompt_file` | Path to Markdown prompt file | | `prompt` | Inline prompt text (alternative to `prompt_file`) | | `allowed_tools` | List of tools the agent can use | -| `provider` | Provider override: `claude`, `codex`, or `opencode` | -| `model` | Model override (alias or full name) | ### Available Tools diff --git a/docs/configuration.ja.md b/docs/configuration.ja.md index b3fdd34..bd157a7 100644 --- a/docs/configuration.ja.md +++ b/docs/configuration.ja.md @@ -223,9 +223,8 @@ codex_cli_path: /usr/local/bin/codex 各 movement で使用されるモデルは、次の優先順位（高い順）で解決されます。 1. **Piece movement の `model`** - piece YAML の movement 定義で指定 -2. **カスタムエージェントの `model`** - `.takt/agents.yaml` のエージェントレベルのモデル -3. **グローバル設定の `model`** - `~/.takt/config.yaml` のデフォルトモデル -4. **Provider デフォルト** - provider のビルトインデフォルトにフォールバック（Claude: `sonnet`、Codex: `codex`、OpenCode: provider デフォルト） +2. **グローバル設定の `model`** - `~/.takt/config.yaml` のデフォルトモデル +3. **Provider デフォルト** - provider のビルトインデフォルトにフォールバック（Claude: `sonnet`、Codex: `codex`、OpenCode: provider デフォルト） ### Provider 固有のモデルに関する注意 diff --git a/docs/configuration.md b/docs/configuration.md index 5975a2f..c976e44 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -223,9 +223,8 @@ The path must be an absolute path to an executable file. `TAKT_CODEX_CLI_PATH` t The model used for each movement is resolved with the following priority order (highest first): 1. **Piece movement `model`** - Specified in the movement definition in piece YAML -2. **Custom agent `model`** - Agent-level model in `.takt/agents.yaml` -3. **Global config `model`** - Default model in `~/.takt/config.yaml` -4. **Provider default** - Falls back to the provider's built-in default (Claude: `sonnet`, Codex: `codex`, OpenCode: provider default) +2. **Global config `model`** - Default model in `~/.takt/config.yaml` +3. **Provider default** - Falls back to the provider's built-in default (Claude: `sonnet`, Codex: `codex`, OpenCode: provider default) ### Provider-specific Model Notes diff --git a/src/__tests__/models.test.ts b/src/__tests__/models.test.ts index e336ba7..0d04ff1 100644 --- a/src/__tests__/models.test.ts +++ b/src/__tests__/models.test.ts @@ -470,17 +470,6 @@ describe('CustomAgentConfigSchema', () => { expect(result.claude_agent).toBe('architect'); }); - it('should accept agent with provider override', () => { - const config = { - name: 'my-agent', - prompt: 'You are a helpful assistant.', - provider: 'codex', - }; - - const result = CustomAgentConfigSchema.parse(config); - expect(result.provider).toBe('codex'); - }); - it('should reject agent without any prompt source', () => { const config = { name: 'my-agent', diff --git a/src/__tests__/opencode-config.test.ts b/src/__tests__/opencode-config.test.ts index a07dcfa..0d215da 100644 --- a/src/__tests__/opencode-config.test.ts +++ b/src/__tests__/opencode-config.test.ts @@ -6,7 +6,6 @@ import { describe, it, expect } from 'vitest'; import { GlobalConfigSchema, ProjectConfigSchema, - CustomAgentConfigSchema, PieceMovementRawSchema, ParallelSubMovementRawSchema, } from '../core/models/index.js'; @@ -64,15 +63,6 @@ describe('Schemas accept opencode provider', () => { expect(() => ProjectConfigSchema.parse({ submodules: 'libs' })).toThrow(); }); - it('should accept opencode in CustomAgentConfigSchema', () => { - const result = CustomAgentConfigSchema.parse({ - name: 'test', - prompt: 'You are a test agent', - provider: 'opencode', - }); - expect(result.provider).toBe('opencode'); - }); - it('should accept opencode in PieceMovementRawSchema', () => { const result = PieceMovementRawSchema.parse({ name: 'test-movement', diff --git a/src/__tests__/option-resolution-order.test.ts b/src/__tests__/option-resolution-order.test.ts index 2ffba87..12987f2 100644 --- a/src/__tests__/option-resolution-order.test.ts +++ b/src/__tests__/option-resolution-order.test.ts @@ -2,9 +2,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; const { getProviderMock, - loadConfigMock, loadCustomAgentsMock, loadAgentPromptMock, + loadProjectConfigMock, + loadGlobalConfigMock, loadTemplateMock, providerSetupMock, providerCallMock, @@ -14,9 +15,10 @@ const { return { getProviderMock: vi.fn(() => ({ setup: providerSetup })), - loadConfigMock: vi.fn(), loadCustomAgentsMock: vi.fn(), loadAgentPromptMock: vi.fn(), + loadProjectConfigMock: vi.fn(), + loadGlobalConfigMock: vi.fn(), loadTemplateMock: vi.fn(), providerSetupMock: providerSetup, providerCallMock: providerCall, @@ -28,21 +30,10 @@ vi.mock('../infra/providers/index.js', () => ({ })); vi.mock('../infra/config/index.js', () => ({ - loadConfig: loadConfigMock, + loadProjectConfig: loadProjectConfigMock, + loadGlobalConfig: loadGlobalConfigMock, loadCustomAgents: loadCustomAgentsMock, loadAgentPrompt: loadAgentPromptMock, - resolveConfigValues: (_projectDir: string, keys: readonly string[]) => { - const loaded = loadConfigMock() as Record; - const global = (loaded.global ?? {}) as Record; - const project = (loaded.project ?? {}) as Record; - const provider = (project.provider ?? global.provider ?? 'claude') as string; - const config: Record = { ...global, ...project, provider, piece: project.piece ?? 'default', verbose: false }; - const result: Record = {}; - for (const key of keys) { - result[key] = config[key]; - } - return result; - }, })); vi.mock('../shared/prompts/index.js', () => ({ @@ -56,120 +47,169 @@ describe('option resolution order', () => { vi.clearAllMocks(); providerCallMock.mockResolvedValue({ content: 'ok' }); - loadConfigMock.mockReturnValue({ global: {}, project: {} }); + loadProjectConfigMock.mockReturnValue({}); + loadGlobalConfigMock.mockReturnValue({ + language: 'en', + concurrency: 1, + taskPollIntervalMs: 500, + }); loadCustomAgentsMock.mockReturnValue(new Map()); loadAgentPromptMock.mockReturnValue('prompt'); loadTemplateMock.mockReturnValue('template'); }); - it('should resolve provider in order: CLI > stepProvider > Config(project??global) > default', async () => { - // Given - loadConfigMock.mockReturnValue({ - project: { provider: 'opencode' }, - global: { provider: 'mock' }, + it('should resolve provider in order: CLI > stepProvider > local config > global config', async () => { + loadProjectConfigMock.mockReturnValue({ provider: 'opencode' }); + loadGlobalConfigMock.mockReturnValue({ + provider: 'mock', + language: 'en', + concurrency: 1, + taskPollIntervalMs: 500, }); - // When: CLI provider が指定される await runAgent(undefined, 'task', { cwd: '/repo', provider: 'codex', stepProvider: 'claude', }); - - // Then expect(getProviderMock).toHaveBeenLastCalledWith('codex'); - // When: CLI 指定なし（stepProvider が優先される） await runAgent(undefined, 'task', { cwd: '/repo', stepProvider: 'claude', }); - - // Then expect(getProviderMock).toHaveBeenLastCalledWith('claude'); - // When: project なし → resolveConfigValues は global.provider を返す（フラットマージ） - loadConfigMock.mockReturnValue({ - project: {}, - global: { provider: 'mock' }, + loadProjectConfigMock.mockReturnValue({}); + loadGlobalConfigMock.mockReturnValue({ + provider: 'mock', + language: 'en', + concurrency: 1, + taskPollIntervalMs: 500, }); await runAgent(undefined, 'task', { cwd: '/repo', stepProvider: 'claude', }); - - // Then: stepProvider が global fallback より優先される expect(getProviderMock).toHaveBeenLastCalledWith('claude'); - // When: stepProvider もなし → 同様に global.provider await runAgent(undefined, 'task', { cwd: '/repo' }); - - // Then expect(getProviderMock).toHaveBeenLastCalledWith('mock'); }); - it('should resolve model in order: CLI > Piece(step) > Global(matching provider)', async () => { - // Given - loadConfigMock.mockReturnValue({ - project: { provider: 'claude' }, - global: { provider: 'claude', model: 'global-model' }, + it('should apply persona provider override before local/global config', async () => { + loadProjectConfigMock.mockReturnValue({ provider: 'opencode' }); + loadGlobalConfigMock.mockReturnValue({ + provider: 'mock', + personaProviders: { + coder: { provider: 'claude' }, + }, + language: 'en', + concurrency: 1, + taskPollIntervalMs: 500, }); - // When: CLI model あり - await runAgent(undefined, 'task', { + await runAgent('coder', 'task', { + cwd: '/repo', + }); + + expect(getProviderMock).toHaveBeenLastCalledWith('claude'); + }); + + it('should resolve model in order: CLI > persona > step > local > global', async () => { + loadProjectConfigMock.mockReturnValue({ + provider: 'claude', + model: 'local-model', + }); + loadGlobalConfigMock.mockReturnValue({ + provider: 'claude', + model: 'global-model', + language: 'en', + concurrency: 1, + taskPollIntervalMs: 500, + personaProviders: { + coder: { model: 'persona-model' }, + }, + }); + + await runAgent('coder', 'task', { cwd: '/repo', model: 'cli-model', stepModel: 'step-model', }); - // Then expect(providerCallMock).toHaveBeenLastCalledWith( 'task', expect.objectContaining({ model: 'cli-model' }), ); - // When: CLI model なし await runAgent(undefined, 'task', { cwd: '/repo', stepModel: 'step-model', + stepProvider: 'claude', }); - // Then expect(providerCallMock).toHaveBeenLastCalledWith( 'task', expect.objectContaining({ model: 'step-model' }), ); - // When: stepModel なし - await runAgent(undefined, 'task', { cwd: '/repo' }); + await runAgent('coder', 'task', { + cwd: '/repo', + stepProvider: 'claude', + }); + expect(providerCallMock).toHaveBeenLastCalledWith( + 'task', + expect.objectContaining({ model: 'persona-model' }), + ); + + loadGlobalConfigMock.mockReturnValue({ + provider: 'codex', + model: 'global-model', + language: 'en', + concurrency: 1, + taskPollIntervalMs: 500, + }); + loadProjectConfigMock.mockReturnValue({ + provider: 'codex', + }); + + await runAgent(undefined, 'task', { + cwd: '/repo', + stepProvider: 'codex', + }); - // Then expect(providerCallMock).toHaveBeenLastCalledWith( 'task', expect.objectContaining({ model: 'global-model' }), ); }); - it('should ignore global model when resolved provider does not match config provider', async () => { - // Given: CLI provider overrides config provider, causing mismatch with config.model - loadConfigMock.mockReturnValue({ - project: {}, - global: { provider: 'claude', model: 'global-model' }, + it('should ignore local/global model if resolved provider is not matching', async () => { + loadProjectConfigMock.mockReturnValue({ + provider: 'claude', + model: 'local-model', + }); + loadGlobalConfigMock.mockReturnValue({ + provider: 'mock', + model: 'global-model', + language: 'en', + concurrency: 1, + taskPollIntervalMs: 500, }); - // When: CLI provider='codex' overrides config provider='claude' - // resolveModel compares config.provider ('claude') with resolvedProvider ('codex') → mismatch → model ignored - await runAgent(undefined, 'task', { cwd: '/repo', provider: 'codex' }); + await runAgent(undefined, 'task', { + cwd: '/repo', + stepProvider: 'opencode', + }); - // Then expect(providerCallMock).toHaveBeenLastCalledWith( 'task', expect.objectContaining({ model: undefined }), ); }); - it('should use providerOptions from piece(step) only', async () => { - // Given + it('should use providerOptions from piece/step only', async () => { const stepProviderOptions = { claude: { sandbox: { @@ -178,55 +218,37 @@ describe('option resolution order', () => { }, }; - loadConfigMock.mockReturnValue({ - project: { - provider: 'claude', - }, - global: { - provider: 'claude', - providerOptions: { - claude: { sandbox: { allowUnsandboxedCommands: true } }, - }, - }, - }); - - // When await runAgent(undefined, 'task', { cwd: '/repo', provider: 'claude', providerOptions: stepProviderOptions, }); - // Then expect(providerCallMock).toHaveBeenLastCalledWith( 'task', expect.objectContaining({ providerOptions: stepProviderOptions }), ); }); - it('should use custom agent model and prompt when higher-priority values are absent', async () => { - // Given: custom agent with provider/model, but no CLI/config override - // Note: resolveConfigValues returns provider='claude' by default (loadConfig merges project ?? global ?? 'claude'), - // so agentConfig.provider is not reached in resolveProvider (config.provider is always truthy). - // However, custom agent model IS used because resolveModel checks agentConfig.model before config. - const customAgents = new Map([ - ['custom', { name: 'custom', prompt: 'agent prompt', provider: 'opencode', model: 'agent-model' }], - ]); - loadCustomAgentsMock.mockReturnValue(customAgents); + it('should ignore custom agent provider/model overrides', async () => { + loadProjectConfigMock.mockReturnValue({ provider: 'claude', model: 'project-model' }); + loadGlobalConfigMock.mockReturnValue({ + provider: 'mock', + language: 'en', + concurrency: 1, + taskPollIntervalMs: 500, + }); + + loadCustomAgentsMock.mockReturnValue(new Map([ + ['custom', { name: 'custom', prompt: 'agent prompt' }], + ])); - // When await runAgent('custom', 'task', { cwd: '/repo' }); - // Then: provider falls back to config default ('claude'), not agentConfig.provider expect(getProviderMock).toHaveBeenLastCalledWith('claude'); - // Agent model is used (resolved before config.model in resolveModel) expect(providerCallMock).toHaveBeenLastCalledWith( 'task', - expect.objectContaining({ model: 'agent-model' }), - ); - // Agent prompt is still used - expect(providerSetupMock).toHaveBeenLastCalledWith( - expect.objectContaining({ systemPrompt: 'prompt' }), + expect.objectContaining({ model: 'project-model' }), ); }); }); diff --git a/src/__tests__/provider-resolution.test.ts b/src/__tests__/provider-resolution.test.ts index 84c8051..2adfc6d 100644 --- a/src/__tests__/provider-resolution.test.ts +++ b/src/__tests__/provider-resolution.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest'; -import { resolveMovementProviderModel, resolveProviderModelCandidates } from '../core/piece/provider-resolution.js'; +import { + resolveAgentProviderModel, + resolveMovementProviderModel, + resolveProviderModelCandidates, +} from '../core/piece/provider-resolution.js'; describe('resolveProviderModelCandidates', () => { it('should resolve first defined provider and model independently', () => { @@ -26,118 +30,313 @@ describe('resolveProviderModelCandidates', () => { describe('resolveMovementProviderModel', () => { it('should prefer personaProviders.provider over step.provider when both are defined', () => { - // Given: step.provider と personaProviders.provider が両方指定されている const result = resolveMovementProviderModel({ step: { provider: 'codex', model: undefined, personaDisplayName: 'coder' }, provider: 'claude', personaProviders: { coder: { provider: 'opencode' } }, }); - // When: provider/model を解決する - // Then: personaProviders.provider が step.provider を上書きする expect(result.provider).toBe('opencode'); }); it('should use personaProviders.provider when step.provider is undefined', () => { - // Given: step.provider が未定義で personaProviders に対応がある const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'reviewer' }, provider: 'claude', personaProviders: { reviewer: { provider: 'opencode' } }, }); - // When: provider/model を解決する - // Then: personaProviders の provider が使われる expect(result.provider).toBe('opencode'); }); it('should fallback to input.provider when persona mapping is missing', () => { - // Given: step.provider 未定義かつ persona マッピングが存在しない const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'unknown' }, provider: 'mock', personaProviders: { reviewer: { provider: 'codex' } }, }); - // When: provider/model を解決する - // Then: input.provider が使われる expect(result.provider).toBe('mock'); }); it('should return undefined provider when all provider candidates are missing', () => { - // Given: provider の候補がすべて未定義 const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'none' }, provider: undefined, personaProviders: undefined, }); - // When: provider/model を解決する - // Then: provider は undefined になる expect(result.provider).toBeUndefined(); }); it('should prefer personaProviders.model over step.model and input.model', () => { - // Given: step.model と personaProviders.model と input.model が指定されている const result = resolveMovementProviderModel({ step: { provider: undefined, model: 'step-model', personaDisplayName: 'coder' }, model: 'input-model', personaProviders: { coder: { provider: 'codex', model: 'persona-model' } }, }); - // When: provider/model を解決する - // Then: personaProviders.model が step.model を上書きする expect(result.model).toBe('persona-model'); }); it('should use personaProviders.model when step.model is undefined', () => { - // Given: step.model が未定義で personaProviders.model が指定されている const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'coder' }, model: 'input-model', personaProviders: { coder: { provider: 'codex', model: 'persona-model' } }, }); - // When: provider/model を解決する - // Then: personaProviders.model が使われる expect(result.model).toBe('persona-model'); }); it('should fallback to input.model when step.model and personaProviders.model are undefined', () => { - // Given: step.model と personaProviders.model が未定義で input.model が指定されている const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'coder' }, model: 'input-model', personaProviders: { coder: { provider: 'codex' } }, }); - // When: provider/model を解決する - // Then: input.model が使われる expect(result.model).toBe('input-model'); }); it('should return undefined model when all model candidates are missing', () => { - // Given: model の候補がすべて未定義 const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'coder' }, model: undefined, personaProviders: { coder: { provider: 'codex' } }, }); - // Then: model は undefined になる expect(result.model).toBeUndefined(); }); it('should resolve provider from personaProviders entry with only model specified', () => { - // Given: personaProviders エントリに provider が指定されていない（model のみ） const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'coder' }, provider: 'claude', personaProviders: { coder: { model: 'o3-mini' } }, }); - // Then: provider は input.provider、model は personaProviders.model になる expect(result.provider).toBe('claude'); expect(result.model).toBe('o3-mini'); }); }); + +describe('resolveAgentProviderModel', () => { + it('should resolve provider in order: CLI > persona > movement > local > global', () => { + const result = resolveAgentProviderModel({ + cliProvider: 'opencode', + stepProvider: 'claude', + localProvider: 'codex', + globalProvider: 'claude', + personaProviders: { coder: { provider: 'mock' } }, + personaDisplayName: 'coder', + }); + + expect(result.provider).toBe('opencode'); + }); + + it('should use persona override when no CLI provider is set', () => { + const result = resolveAgentProviderModel({ + stepProvider: 'claude', + localProvider: 'codex', + globalProvider: 'claude', + personaProviders: { coder: { provider: 'opencode', model: 'persona-model' } }, + personaDisplayName: 'coder', + }); + + expect(result.provider).toBe('opencode'); + expect(result.model).toBe('persona-model'); + }); + + it('should fall back to movement provider when persona override is not configured', () => { + const result = resolveAgentProviderModel({ + stepProvider: 'claude', + localProvider: 'codex', + globalProvider: 'claude', + personaProviders: { reviewer: { provider: 'mock', model: 'o3-mini' } }, + personaDisplayName: 'coder', + }); + + expect(result.provider).toBe('claude'); + }); + + it('should prefer local config provider/model over global config for same provider', () => { + const result = resolveAgentProviderModel({ + localProvider: 'codex', + localModel: 'local-model', + globalProvider: 'codex', + globalModel: 'global-model', + }); + + expect(result.provider).toBe('codex'); + expect(result.model).toBe('local-model'); + }); + + it('should prefer global config when local config is not set', () => { + const result = resolveAgentProviderModel({ + localProvider: undefined, + globalProvider: 'claude', + globalModel: 'global-model', + }); + + expect(result.provider).toBe('claude'); + expect(result.model).toBe('global-model'); + }); + + it('should resolve model order: CLI > persona > movement > config candidate matching provider', () => { + const result = resolveAgentProviderModel({ + cliModel: 'cli-model', + stepModel: 'movement-model', + localProvider: 'claude', + localModel: 'local-model', + globalProvider: 'codex', + globalModel: 'global-model', + cliProvider: 'codex', + personaProviders: { coder: { model: 'persona-model' } }, + personaDisplayName: 'coder', + }); + + expect(result.provider).toBe('codex'); + expect(result.model).toBe('cli-model'); + }); + + it('should use movement model when persona model is absent', () => { + const result = resolveAgentProviderModel({ + stepModel: 'movement-model', + localProvider: 'claude', + localModel: 'local-model', + globalProvider: 'codex', + globalModel: 'global-model', + personaProviders: { coder: { provider: 'opencode' } }, + personaDisplayName: 'coder', + }); + + expect(result.provider).toBe('opencode'); + expect(result.model).toBe('movement-model'); + }); + + it('should apply local/ global model only when provider matches resolved provider', () => { + const result = resolveAgentProviderModel({ + localProvider: 'claude', + localModel: 'local-model', + globalProvider: 'codex', + globalModel: 'global-model', + stepProvider: 'codex', + }); + + expect(result.provider).toBe('codex'); + expect(result.model).toBe('global-model'); + }); + + it('should ignore local and global model when provider does not match', () => { + const result = resolveAgentProviderModel({ + localProvider: 'codex', + localModel: 'local-model', + globalProvider: 'claude', + globalModel: 'global-model', + stepProvider: 'opencode', + }); + + expect(result.provider).toBe('opencode'); + expect(result.model).toBeUndefined(); + }); + + it('should combine persona and movement overrides in one run', () => { + const result = resolveAgentProviderModel({ + cliProvider: 'codex', + stepProvider: 'claude', + stepModel: 'movement-model', + localProvider: 'claude', + localModel: 'local-model', + globalProvider: 'mock', + globalModel: 'global-model', + cliModel: 'cli-model', + personaProviders: { + coder: { + provider: 'mock', + model: 'persona-model', + }, + }, + personaDisplayName: 'coder', + }); + + expect(result.provider).toBe('codex'); + expect(result.model).toBe('cli-model'); + }); + + it('should apply full priority chain when all layers are present', () => { + const result = resolveAgentProviderModel({ + cliProvider: 'codex', + cliModel: 'cli-model', + personaProviders: { + reviewer: { + provider: 'mock', + model: 'persona-model', + }, + }, + personaDisplayName: 'reviewer', + stepProvider: 'claude', + stepModel: 'step-model', + localProvider: 'opencode', + localModel: 'local-model', + globalProvider: 'claude', + globalModel: 'global-model', + }); + + expect(result.provider).toBe('codex'); + expect(result.model).toBe('cli-model'); + }); + + it('should apply full priority chain without cli overrides', () => { + const result = resolveAgentProviderModel({ + personaProviders: { + reviewer: { + provider: 'mock', + model: 'persona-model', + }, + }, + personaDisplayName: 'reviewer', + stepProvider: 'claude', + stepModel: 'step-model', + localProvider: 'opencode', + localModel: 'local-model', + globalProvider: 'claude', + globalModel: 'global-model', + }); + + expect(result.provider).toBe('mock'); + expect(result.model).toBe('persona-model'); + }); + + it('should keep model and provider priorities consistent for fallback path', () => { + const result = resolveAgentProviderModel({ + stepProvider: 'claude', + localProvider: 'codex', + localModel: 'local-model', + globalProvider: 'claude', + globalModel: 'global-model', + }); + + expect(result.provider).toBe('claude'); + expect(result.model).toBe('global-model'); + }); + + it('should keep model fallback after persona-only model when step model is absent', () => { + const result = resolveAgentProviderModel({ + personaProviders: { + reviewer: { + model: 'persona-model', + }, + }, + personaDisplayName: 'reviewer', + stepProvider: 'claude', + localProvider: 'codex', + localModel: 'local-model', + globalProvider: 'codex', + globalModel: 'global-model', + }); + + expect(result.provider).toBe('claude'); + expect(result.model).toBe('persona-model'); + }); +}); diff --git a/src/__tests__/taskExecution.test.ts b/src/__tests__/taskExecution.test.ts index 1139a07..31be3e6 100644 --- a/src/__tests__/taskExecution.test.ts +++ b/src/__tests__/taskExecution.test.ts @@ -67,7 +67,7 @@ vi.mock('../shared/i18n/index.js', () => ({ getLabel: vi.fn((key: string) => key), })); -import { executeAndCompleteTask } from '../features/tasks/execute/taskExecution.js'; +import { executeAndCompleteTask, executeTask } from '../features/tasks/execute/taskExecution.js'; const createTask = (name: string): TaskInfo => ({ name, @@ -151,6 +151,54 @@ describe('executeAndCompleteTask', () => { expect(pieceExecutionOptions?.providerOptionsSource).toBe('project'); }); + it('should not pass config provider/model to executePiece when agent overrides are absent', async () => { + // Given: project config contains provider/model, but overrides are omitted. + const task = createTask('task-with-defaults'); + + // When + await executeTask({ + task: task.content, + cwd: '/project', + projectCwd: '/project', + pieceIdentifier: 'default', + }); + + // Then: piece options should not force provider/model from taskExecution layer + expect(mockExecutePiece).toHaveBeenCalledTimes(1); + const pieceExecutionOptions = mockExecutePiece.mock.calls[0]?.[3] as { + provider?: string; + model?: string; + }; + expect(pieceExecutionOptions?.provider).toBeUndefined(); + expect(pieceExecutionOptions?.model).toBeUndefined(); + }); + + it('should pass agent overrides to executePiece when provided', async () => { + // Given: overrides explicitly specified by caller. + const task = createTask('task-with-overrides'); + + // When + await executeTask({ + task: task.content, + cwd: '/project', + projectCwd: '/project', + pieceIdentifier: 'default', + agentOverrides: { + provider: 'codex', + model: 'gpt-5.3-codex', + }, + }); + + // Then + expect(mockExecutePiece).toHaveBeenCalledTimes(1); + const pieceExecutionOptions = mockExecutePiece.mock.calls[0]?.[3] as { + provider?: string; + model?: string; + }; + expect(pieceExecutionOptions?.provider).toBe('codex'); + expect(pieceExecutionOptions?.model).toBe('gpt-5.3-codex'); + }); + it('should mark task as failed when PR creation fails', async () => { // Given: worktree mode with autoPr enabled, PR creation fails const task = createTask('task-with-pr-failure'); diff --git a/src/agents/runner.ts b/src/agents/runner.ts index 1fe6645..cdb3c2d 100644 --- a/src/agents/runner.ts +++ b/src/agents/runner.ts @@ -4,10 +4,10 @@ import { existsSync, readFileSync } from 'node:fs'; import { basename, dirname } from 'node:path'; -import { loadCustomAgents, loadAgentPrompt, resolveConfigValues } from '../infra/config/index.js'; +import { loadCustomAgents, loadAgentPrompt, loadGlobalConfig, loadProjectConfig } from '../infra/config/index.js'; import { getProvider, type ProviderType, type ProviderCallOptions } from '../infra/providers/index.js'; import type { AgentResponse, CustomAgentConfig } from '../core/models/index.js'; -import { resolveProviderModelCandidates } from '../core/piece/provider-resolution.js'; +import { resolveAgentProviderModel } from '../core/piece/provider-resolution.js'; import { createLogger } from '../shared/utils/index.js'; import { loadTemplate } from '../shared/prompts/index.js'; import type { RunAgentOptions } from './types.js'; @@ -25,33 +25,31 @@ const log = createLogger('runner'); export class AgentRunner { private static resolveProviderAndModel( cwd: string, + personaDisplayName: string | undefined, options?: RunAgentOptions, - agentConfig?: CustomAgentConfig, ): { provider: ProviderType; model: string | undefined } { - const config = resolveConfigValues(cwd, ['provider', 'model']); - const resolvedProvider = resolveProviderModelCandidates([ - { provider: options?.provider }, - { provider: options?.stepProvider }, - { provider: config.provider }, - { provider: agentConfig?.provider }, - ]).provider; + const localConfig = loadProjectConfig(cwd); + const globalConfig = loadGlobalConfig(); + + const resolvedProviderModel = resolveAgentProviderModel({ + personaDisplayName, + cliProvider: options?.provider, + cliModel: options?.model, + stepProvider: options?.stepProvider, + stepModel: options?.stepModel, + personaProviders: globalConfig.personaProviders, + localProvider: localConfig.provider, + localModel: localConfig.model, + globalProvider: globalConfig.provider, + globalModel: globalConfig.model, + }); + const resolvedProvider = resolvedProviderModel.provider; if (!resolvedProvider) { throw new Error('No provider configured. Set "provider" in ~/.takt/config.yaml'); } - - const configModel = config.provider === resolvedProvider - ? config.model - : undefined; - const resolvedModel = resolveProviderModelCandidates([ - { model: options?.model }, - { model: options?.stepModel }, - { model: agentConfig?.model }, - { model: configModel }, - ]).model; - return { provider: resolvedProvider, - model: resolvedModel, + model: resolvedProviderModel.model, }; } @@ -112,7 +110,7 @@ export class AgentRunner { task: string, options: RunAgentOptions, ): Promise { - const resolved = AgentRunner.resolveProviderAndModel(options.cwd, options, agentConfig); + const resolved = AgentRunner.resolveProviderAndModel(options.cwd, agentConfig.name, options); const providerType = resolved.provider; const provider = getProvider(providerType); @@ -145,7 +143,7 @@ export class AgentRunner { permissionMode: options.permissionMode, }); - const resolved = AgentRunner.resolveProviderAndModel(options.cwd, options); + const resolved = AgentRunner.resolveProviderAndModel(options.cwd, personaName, options); const providerType = resolved.provider; const provider = getProvider(providerType); const callOptions = AgentRunner.buildCallOptions(resolved.model, options); diff --git a/src/core/models/persisted-global-config.ts b/src/core/models/persisted-global-config.ts index 4f2f5b4..028ca56 100644 --- a/src/core/models/persisted-global-config.ts +++ b/src/core/models/persisted-global-config.ts @@ -18,8 +18,6 @@ export interface CustomAgentConfig { allowedTools?: string[]; claudeAgent?: string; claudeSkill?: string; - provider?: 'claude' | 'codex' | 'opencode' | 'mock'; - model?: string; } /** Observability configuration for runtime event logs */ diff --git a/src/core/models/schemas.ts b/src/core/models/schemas.ts index a8440a1..8d85541 100644 --- a/src/core/models/schemas.ts +++ b/src/core/models/schemas.ts @@ -380,8 +380,6 @@ export const CustomAgentConfigSchema = z.object({ allowed_tools: z.array(z.string()).optional(), claude_agent: z.string().optional(), claude_skill: z.string().optional(), - provider: z.enum(['claude', 'codex', 'opencode', 'mock']).optional(), - model: z.string().optional(), }).refine( (data) => data.prompt_file || data.prompt || data.claude_agent || data.claude_skill, { message: 'Agent must have prompt_file, prompt, claude_agent, or claude_skill' } diff --git a/src/core/piece/engine/stream-buffer.ts b/src/core/piece/engine/stream-buffer.ts index 5b35826..d40c36d 100644 --- a/src/core/piece/engine/stream-buffer.ts +++ b/src/core/piece/engine/stream-buffer.ts @@ -130,7 +130,31 @@ export class LineTimeSliceBuffer { } private isBoundary(ch: string): boolean { - return /\s|[,.!?;:、。！？；：（）\[\]{}]/u.test(ch); + const boundaryChars = new Set([ + ' ', + '\n', + '\t', + ',', + '.', + '!', + '?', + ';', + ':', + '、', + '。', + '！', + '？', + '；', + '：', + '（', + '）', + '[', + ']', + '{', + '}', + ]); + + return boundaryChars.has(ch); } private clearTimer(key: string): void { diff --git a/src/core/piece/provider-resolution.ts b/src/core/piece/provider-resolution.ts index 60ee330..b6d0966 100644 --- a/src/core/piece/provider-resolution.ts +++ b/src/core/piece/provider-resolution.ts @@ -19,6 +19,11 @@ export interface ProviderModelCandidate { model?: string; } +interface ModelProviderCandidate { + model?: string; + provider?: ProviderType; +} + export function resolveProviderModelCandidates( candidates: readonly ProviderModelCandidate[], ): MovementProviderModelOutput { @@ -40,6 +45,61 @@ export function resolveProviderModelCandidates( return { provider, model }; } +export interface AgentProviderModelInput { + cliProvider?: ProviderType; + cliModel?: string; + personaProviders?: Record; + personaDisplayName?: string; + stepProvider?: ProviderType; + stepModel?: string; + localProvider?: ProviderType; + localModel?: string; + globalProvider?: ProviderType; + globalModel?: string; +} + +export interface AgentProviderModelOutput { + provider?: ProviderType; + model?: string; +} + +function resolveModelFromCandidates( + candidates: readonly ModelProviderCandidate[], + resolvedProvider: ProviderType | undefined, +): string | undefined { + for (const candidate of candidates) { + const { model, provider } = candidate; + if (model === undefined) { + continue; + } + if (provider !== undefined && provider !== resolvedProvider) { + continue; + } + return model; + } + return undefined; +} + +export function resolveAgentProviderModel(input: AgentProviderModelInput): AgentProviderModelOutput { + const personaEntry = input.personaProviders?.[input.personaDisplayName ?? '']; + const provider = resolveProviderModelCandidates([ + { provider: input.cliProvider }, + { provider: personaEntry?.provider }, + { provider: input.stepProvider }, + { provider: input.localProvider }, + { provider: input.globalProvider }, + ]).provider; + const model = resolveModelFromCandidates([ + { model: input.cliModel }, + { model: personaEntry?.model }, + { model: input.stepModel }, + { model: input.localModel, provider: input.localProvider }, + { model: input.globalModel, provider: input.globalProvider }, + ], provider); + + return { provider, model }; +} + export function resolveMovementProviderModel(input: MovementProviderModelInput): MovementProviderModelOutput { const personaEntry = input.personaProviders?.[input.step.personaDisplayName]; const provider = resolveProviderModelCandidates([ diff --git a/src/features/tasks/execute/taskExecution.ts b/src/features/tasks/execute/taskExecution.ts index d21cf90..ace42c1 100644 --- a/src/features/tasks/execute/taskExecution.ts +++ b/src/features/tasks/execute/taskExecution.ts @@ -62,19 +62,13 @@ async function executeTaskWithResult(options: ExecuteTaskOptions): Promise s.name), }); - const config = resolvePieceConfigValues(projectCwd, [ - 'language', - 'provider', - 'model', - 'personaProviders', - 'providerProfiles', - ]); + const config = resolvePieceConfigValues(projectCwd, ['language', 'personaProviders', 'providerProfiles']); const providerOptions = resolveConfigValueWithSource(projectCwd, 'providerOptions'); return await executePiece(pieceConfig, task, cwd, { projectCwd, language: config.language, - provider: agentOverrides?.provider ?? config.provider, - model: agentOverrides?.model ?? config.model, + provider: agentOverrides?.provider, + model: agentOverrides?.model, providerOptions: providerOptions.value, providerOptionsSource: providerOptions.source === 'piece' ? 'global' : providerOptions.source, personaProviders: config.personaProviders, diff --git a/src/infra/config/types.ts b/src/infra/config/types.ts index cb2a122..8058e1f 100644 --- a/src/infra/config/types.ts +++ b/src/infra/config/types.ts @@ -12,6 +12,8 @@ export interface ProjectLocalConfig { piece?: string; /** Provider selection for agent runtime */ provider?: 'claude' | 'codex' | 'opencode' | 'mock'; + /** Model selection for agent runtime */ + model?: string; /** Auto-create PR after worktree execution */ autoPr?: boolean; /** Create PR as draft */