diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index f028970..7dea5e8 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -399,6 +399,77 @@ describe('loadProjectConfig provider_options', () => { process.env.TAKT_PROVIDER_OPTIONS_CODEX_NETWORK_ACCESS = original; } }); + + it('should throw when provider block uses claude with network_access', () => { + const projectConfigDir = getProjectConfigDir(testDir); + mkdirSync(projectConfigDir, { recursive: true }); + writeFileSync(join(projectConfigDir, 'config.yaml'), [ + 'piece: default', + 'provider:', + ' type: claude', + ' network_access: true', + ].join('\n')); + + expect(() => loadProjectConfig(testDir)).toThrow(/network_access/); + }); + + it('should normalize project provider block into provider/model/providerOptions', () => { + const projectConfigDir = getProjectConfigDir(testDir); + mkdirSync(projectConfigDir, { recursive: true }); + writeFileSync(join(projectConfigDir, 'config.yaml'), [ + 'piece: default', + 'provider:', + ' type: codex', + ' model: gpt-5.3', + ' network_access: false', + ].join('\n')); + + const config = loadProjectConfig(testDir); + + expect(config.provider).toBe('codex'); + expect(config.model).toBe('gpt-5.3'); + expect(config.providerOptions).toEqual({ + codex: { networkAccess: false }, + }); + }); + + it('should throw when provider block uses codex with sandbox', () => { + const projectConfigDir = getProjectConfigDir(testDir); + mkdirSync(projectConfigDir, { recursive: true }); + writeFileSync(join(projectConfigDir, 'config.yaml'), [ + 'piece: default', + 'provider:', + ' type: codex', + ' sandbox:', + ' allow_unsandboxed_commands: true', + ].join('\n')); + + expect(() => loadProjectConfig(testDir)).toThrow(/sandbox/); + }); + + it('should throw when provider block contains unknown fields', () => { + const projectConfigDir = getProjectConfigDir(testDir); + mkdirSync(projectConfigDir, { recursive: true }); + writeFileSync(join(projectConfigDir, 'config.yaml'), [ + 'piece: default', + 'provider:', + ' type: codex', + ' unknown_option: true', + ].join('\n')); + + expect(() => loadProjectConfig(testDir)).toThrow(/unknown fields|unrecognized key/i); + }); + + it('should throw when project provider has unsupported type', () => { + const projectConfigDir = getProjectConfigDir(testDir); + mkdirSync(projectConfigDir, { recursive: true }); + writeFileSync(join(projectConfigDir, 'config.yaml'), [ + 'piece: default', + 'provider: invalid-provider', + ].join('\n')); + + expect(() => loadProjectConfig(testDir)).toThrow(/provider/); + }); }); describe('analytics config resolution', () => { diff --git a/src/__tests__/engine-provider-options.test.ts b/src/__tests__/engine-provider-options.test.ts index e8d88ba..92e13b5 100644 --- a/src/__tests__/engine-provider-options.test.ts +++ b/src/__tests__/engine-provider-options.test.ts @@ -54,7 +54,7 @@ describe('PieceEngine provider_options resolution', () => { } }); - it('should merge provider_options in order: global < piece/movement < project', async () => { + it('should merge provider_options in order: global/project < piece/movement', async () => { const movement = makeMovement('implement', { providerOptions: { codex: { networkAccess: false }, @@ -90,7 +90,7 @@ describe('PieceEngine provider_options resolution', () => { const options = vi.mocked(runAgent).mock.calls[0]?.[2]; expect(options?.providerOptions).toEqual({ - codex: { networkAccess: true }, + codex: { networkAccess: false }, opencode: { networkAccess: true }, claude: { sandbox: { diff --git a/src/__tests__/globalConfig-defaults.test.ts b/src/__tests__/globalConfig-defaults.test.ts index 7696675..fb6626b 100644 --- a/src/__tests__/globalConfig-defaults.test.ts +++ b/src/__tests__/globalConfig-defaults.test.ts @@ -78,6 +78,53 @@ describe('loadGlobalConfig', () => { expect(config.logLevel).toBe('debug'); }); + it('should load provider block from config.yaml and normalize model/providerOptions', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'provider:', + ' type: codex', + ' model: gpt-5.3', + ' network_access: true', + ].join('\n'), + 'utf-8', + ); + + const config = loadGlobalConfig(); + + expect(config.provider).toBe('codex'); + expect(config.model).toBe('gpt-5.3'); + expect(config.providerOptions).toEqual({ + codex: { networkAccess: true }, + }); + }); + + it('should load persona_providers provider block and normalize to provider/model', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'persona_providers:', + ' coder:', + ' type: opencode', + ' model: openai/gpt-5', + ].join('\n'), + 'utf-8', + ); + + const config = loadGlobalConfig(); + + expect(config.personaProviders).toEqual({ + coder: { + provider: 'opencode', + model: 'openai/gpt-5', + }, + }); + }); + it('should apply env override for nested provider_options key', () => { const original = process.env.TAKT_PROVIDER_OPTIONS_CLAUDE_SANDBOX_ALLOW_UNSANDBOXED_COMMANDS; try { @@ -573,6 +620,24 @@ describe('loadGlobalConfig', () => { expect(() => loadGlobalConfig()).not.toThrow(); }); + + it('should throw when persona provider block includes provider options', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'persona_providers:', + ' coder:', + ' type: codex', + ' network_access: true', + ].join('\n'), + 'utf-8', + ); + + expect(() => loadGlobalConfig()).toThrow(); + }); }); describe('runtime', () => { @@ -611,6 +676,39 @@ describe('loadGlobalConfig', () => { }); describe('provider/model compatibility validation', () => { + it('should throw when provider block uses claude with network_access', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'provider:', + ' type: claude', + ' network_access: true', + ].join('\n'), + 'utf-8', + ); + + expect(() => loadGlobalConfig()).toThrow(/network_access/); + }); + + it('should throw when provider block uses codex with sandbox', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'provider:', + ' type: codex', + ' sandbox:', + ' allow_unsandboxed_commands: true', + ].join('\n'), + 'utf-8', + ); + + expect(() => loadGlobalConfig()).toThrow(/sandbox/); + }); + it('should throw when provider is codex but model is a Claude alias (opus)', () => { const taktDir = join(testHomeDir, '.takt'); mkdirSync(taktDir, { recursive: true }); diff --git a/src/__tests__/it-provider-config-block.test.ts b/src/__tests__/it-provider-config-block.test.ts new file mode 100644 index 0000000..b5d00bb --- /dev/null +++ b/src/__tests__/it-provider-config-block.test.ts @@ -0,0 +1,237 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { randomUUID } from 'node:crypto'; + +vi.mock('../agents/runner.js', () => ({ + runAgent: vi.fn(), +})); + +vi.mock('../agents/ai-judge.js', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + callAiJudge: vi.fn().mockResolvedValue(-1), + }; +}); + +vi.mock('../core/piece/phase-runner.js', () => ({ + needsStatusJudgmentPhase: vi.fn().mockReturnValue(false), + runReportPhase: vi.fn().mockResolvedValue(undefined), + runStatusJudgmentPhase: vi.fn().mockResolvedValue({ tag: '', ruleIndex: 0, method: 'auto_select' }), +})); + +vi.mock('../shared/utils/index.js', async (importOriginal) => ({ + ...(await importOriginal>()), + notifySuccess: vi.fn(), + notifyError: vi.fn(), + playWarningSound: vi.fn(), + generateReportDir: vi.fn().mockReturnValue('test-report-dir'), +})); + +import { runAgent } from '../agents/runner.js'; +import { executeTask } from '../features/tasks/execute/taskExecution.js'; +import { invalidateGlobalConfigCache } from '../infra/config/index.js'; + +interface TestEnv { + projectDir: string; + globalDir: string; +} + +function createEnv(pieceBody: string): TestEnv { + const root = join(tmpdir(), `takt-it-provider-block-${randomUUID()}`); + const projectDir = join(root, 'project'); + const globalDir = join(root, 'global'); + + mkdirSync(projectDir, { recursive: true }); + mkdirSync(join(projectDir, '.takt', 'pieces', 'personas'), { recursive: true }); + mkdirSync(globalDir, { recursive: true }); + + writeFileSync( + join(projectDir, '.takt', 'pieces', 'provider-block-it.yaml'), + pieceBody, + 'utf-8', + ); + writeFileSync(join(projectDir, '.takt', 'pieces', 'personas', 'planner.md'), 'You are planner.', 'utf-8'); + + return { projectDir, globalDir }; +} + +function setGlobalConfig(globalDir: string, body: string): void { + writeFileSync(join(globalDir, 'config.yaml'), body, 'utf-8'); +} + +function setProjectConfig(projectDir: string, body: string): void { + writeFileSync(join(projectDir, '.takt', 'config.yaml'), body, 'utf-8'); +} + +function makeDoneResponse() { + return { + persona: 'planner', + status: 'done', + content: '[PLAN:1]\ndone', + timestamp: new Date(), + sessionId: 'session-it', + }; +} + +describe('IT: provider block reflection', () => { + let env: TestEnv; + let originalConfigDir: string | undefined; + + beforeEach(() => { + vi.clearAllMocks(); + originalConfigDir = process.env.TAKT_CONFIG_DIR; + vi.mocked(runAgent).mockResolvedValue(makeDoneResponse()); + }); + + afterEach(() => { + if (originalConfigDir === undefined) { + delete process.env.TAKT_CONFIG_DIR; + } else { + process.env.TAKT_CONFIG_DIR = originalConfigDir; + } + invalidateGlobalConfigCache(); + if (env) { + rmSync(join(env.projectDir, '..'), { recursive: true, force: true }); + } + }); + + it('movement provider block should be normalized and passed to runAgent options', async () => { + // Given + env = createEnv([ + 'name: provider-block-it', + 'description: movement provider block integration test', + 'max_movements: 3', + 'initial_movement: plan', + 'movements:', + ' - name: plan', + ' persona: ./personas/planner.md', + ' provider:', + ' type: codex', + ' model: gpt-5.3', + ' network_access: false', + ' instruction: "{task}"', + ' rules:', + ' - condition: done', + ' next: COMPLETE', + ].join('\n')); + process.env.TAKT_CONFIG_DIR = env.globalDir; + setGlobalConfig(env.globalDir, [ + 'provider:', + ' type: codex', + ' model: global-model', + ' network_access: true', + ].join('\n')); + setProjectConfig(env.projectDir, [ + 'provider:', + ' type: codex', + ' model: project-model', + ' network_access: true', + ].join('\n')); + invalidateGlobalConfigCache(); + + // When + const ok = await executeTask({ + task: 'test task', + cwd: env.projectDir, + projectCwd: env.projectDir, + pieceIdentifier: 'provider-block-it', + }); + + // Then + expect(ok).toBe(true); + const options = vi.mocked(runAgent).mock.calls[0]?.[2]; + expect(options?.stepProvider).toBe('codex'); + expect(options?.stepModel).toBe('gpt-5.3'); + expect(options?.providerOptions).toEqual({ + codex: { networkAccess: false }, + }); + }); + + it('piece_config provider block should be inherited by movement without provider', async () => { + // Given + env = createEnv([ + 'name: provider-block-it', + 'description: piece_config provider block integration test', + 'max_movements: 3', + 'initial_movement: plan', + 'piece_config:', + ' provider:', + ' type: codex', + ' model: piece-model', + ' network_access: true', + 'movements:', + ' - name: plan', + ' persona: ./personas/planner.md', + ' instruction: "{task}"', + ' rules:', + ' - condition: done', + ' next: COMPLETE', + ].join('\n')); + process.env.TAKT_CONFIG_DIR = env.globalDir; + setGlobalConfig(env.globalDir, 'provider: claude'); + invalidateGlobalConfigCache(); + + // When + const ok = await executeTask({ + task: 'test task', + cwd: env.projectDir, + projectCwd: env.projectDir, + pieceIdentifier: 'provider-block-it', + }); + + // Then + expect(ok).toBe(true); + const options = vi.mocked(runAgent).mock.calls[0]?.[2]; + expect(options?.stepProvider).toBe('codex'); + expect(options?.stepModel).toBe('piece-model'); + expect(options?.providerOptions).toEqual({ + codex: { networkAccess: true }, + }); + }); + + it('project provider block should provide providerOptions when movement and piece_config do not specify provider', async () => { + // Given + env = createEnv([ + 'name: provider-block-it', + 'description: project provider block integration test', + 'max_movements: 3', + 'initial_movement: plan', + 'movements:', + ' - name: plan', + ' persona: ./personas/planner.md', + ' instruction: "{task}"', + ' rules:', + ' - condition: done', + ' next: COMPLETE', + ].join('\n')); + process.env.TAKT_CONFIG_DIR = env.globalDir; + setGlobalConfig(env.globalDir, 'provider: claude'); + setProjectConfig(env.projectDir, [ + 'provider:', + ' type: codex', + ' model: project-model', + ' network_access: false', + ].join('\n')); + invalidateGlobalConfigCache(); + + // When + const ok = await executeTask({ + task: 'test task', + cwd: env.projectDir, + projectCwd: env.projectDir, + pieceIdentifier: 'provider-block-it', + }); + + // Then + expect(ok).toBe(true); + const options = vi.mocked(runAgent).mock.calls[0]?.[2]; + expect(options?.stepProvider).toBeUndefined(); + expect(options?.stepModel).toBeUndefined(); + expect(options?.providerOptions).toEqual({ + codex: { networkAccess: false }, + }); + }); +}); diff --git a/src/__tests__/models.test.ts b/src/__tests__/models.test.ts index 4b99b6e..40b5289 100644 --- a/src/__tests__/models.test.ts +++ b/src/__tests__/models.test.ts @@ -11,6 +11,7 @@ import { McpServerConfigSchema, CustomAgentConfigSchema, GlobalConfigSchema, + ProjectConfigSchema, } from '../core/models/index.js'; describe('AgentTypeSchema', () => { @@ -128,6 +129,114 @@ describe('PieceConfigRawSchema', () => { }); }); + it('should parse movement with provider object block', () => { + const config = { + name: 'test-piece', + movements: [ + { + name: 'implement', + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: true, + }, + instruction: '{task}', + }, + ], + }; + + const result = PieceConfigRawSchema.parse(config as unknown); + const movement = result.movements?.[0] as Record | undefined; + const provider = movement?.provider as Record | undefined; + expect(provider?.type).toBe('codex'); + expect(provider?.model).toBe('gpt-5.3'); + expect(provider?.network_access).toBe(true); + }); + + it('should reject provider block when claude sets network_access', () => { + const config = { + name: 'test-piece', + movements: [ + { + name: 'implement', + provider: { + type: 'claude', + network_access: true, + }, + instruction: '{task}', + }, + ], + }; + + expect(() => PieceConfigRawSchema.parse(config as unknown)).toThrow(/network_access/); + }); + + it('should reject provider block when codex sets sandbox', () => { + const config = { + name: 'test-piece', + movements: [ + { + name: 'implement', + provider: { + type: 'codex', + sandbox: { + allow_unsandboxed_commands: true, + }, + }, + instruction: '{task}', + }, + ], + }; + + expect(() => PieceConfigRawSchema.parse(config as unknown)).toThrow(/sandbox/); + }); + + it('should reject provider block with unknown fields', () => { + const config = { + name: 'test-piece', + movements: [ + { + name: 'implement', + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: true, + unknown_option: true, + }, + instruction: '{task}', + }, + ], + }; + + expect(() => PieceConfigRawSchema.parse(config as unknown)).toThrow(); + }); + + it('should parse piece-level piece_config.provider block', () => { + const config = { + name: 'test-piece', + piece_config: { + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: true, + }, + }, + movements: [ + { + name: 'implement', + instruction: '{task}', + }, + ], + }; + + const result = PieceConfigRawSchema.parse(config as unknown); + const pieceConfig = result.piece_config as Record | undefined; + const provider = pieceConfig?.provider as Record | undefined; + expect(provider?.type).toBe('codex'); + expect(provider?.model).toBe('gpt-5.3'); + expect(provider?.network_access).toBe(true); + }); + it('should parse piece-level piece_config.provider_options', () => { const config = { name: 'test-piece', @@ -485,4 +594,60 @@ describe('GlobalConfigSchema', () => { expect(result.log_level).toBe('debug'); expect(result.observability?.provider_events).toBe(false); }); + + it('should parse global provider object block', () => { + const result = GlobalConfigSchema.parse({ + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: true, + }, + } as unknown); + const provider = (result as Record).provider as Record | undefined; + expect(provider?.type).toBe('codex'); + expect(provider?.model).toBe('gpt-5.3'); + expect(provider?.network_access).toBe(true); + }); + + it('should parse persona_providers entry with provider object block', () => { + const result = GlobalConfigSchema.parse({ + persona_providers: { + coder: { + type: 'opencode', + model: 'openai/gpt-5', + }, + }, + } as unknown); + const personaProviders = (result as Record).persona_providers as Record | undefined; + const coder = personaProviders?.coder as Record | undefined; + expect(coder?.type).toBe('opencode'); + expect(coder?.model).toBe('openai/gpt-5'); + }); + + it('should reject persona_providers provider object block with provider options', () => { + expect(() => GlobalConfigSchema.parse({ + persona_providers: { + coder: { + type: 'codex', + network_access: true, + }, + }, + } as unknown)).toThrow(); + }); +}); + +describe('ProjectConfigSchema', () => { + it('should parse project provider object block', () => { + const result = ProjectConfigSchema.parse({ + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: false, + }, + } as unknown); + const provider = (result as Record).provider as Record | undefined; + expect(provider?.type).toBe('codex'); + expect(provider?.model).toBe('gpt-5.3'); + expect(provider?.network_access).toBe(false); + }); }); diff --git a/src/__tests__/options-builder.test.ts b/src/__tests__/options-builder.test.ts index f41f061..5088e16 100644 --- a/src/__tests__/options-builder.test.ts +++ b/src/__tests__/options-builder.test.ts @@ -85,7 +85,7 @@ describe('OptionsBuilder.buildBaseOptions', () => { }); }); - it('merges provider options with precedence: global < movement < project', () => { + it('merges provider options with precedence: global/project < movement', () => { const step = createMovement({ providerOptions: { codex: { networkAccess: false }, @@ -104,12 +104,12 @@ describe('OptionsBuilder.buildBaseOptions', () => { const options = builder.buildBaseOptions(step); expect(options.providerOptions).toEqual({ - codex: { networkAccess: true }, + codex: { networkAccess: false }, opencode: { networkAccess: true }, claude: { sandbox: { - allowUnsandboxedCommands: true, excludedCommands: ['./gradlew'], + allowUnsandboxedCommands: true, }, }, }); diff --git a/src/__tests__/parallel-and-loader.test.ts b/src/__tests__/parallel-and-loader.test.ts index 706d2ae..56105f4 100644 --- a/src/__tests__/parallel-and-loader.test.ts +++ b/src/__tests__/parallel-and-loader.test.ts @@ -54,6 +54,35 @@ describe('ParallelSubMovementRawSchema', () => { } }); + it('should accept provider block in parallel sub-movement', () => { + const raw = { + name: 'provider-block-sub-step', + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: true, + }, + instruction_template: 'Review', + }; + + const result = ParallelSubMovementRawSchema.safeParse(raw); + expect(result.success).toBe(true); + }); + + it('should reject invalid provider block options in parallel sub-movement', () => { + const raw = { + name: 'invalid-provider-block-sub-step', + provider: { + type: 'claude', + network_access: true, + }, + instruction_template: 'Review', + }; + + const result = ParallelSubMovementRawSchema.safeParse(raw); + expect(result.success).toBe(false); + }); + it('should accept rules on sub-movements', () => { const raw = { name: 'reviewed', @@ -120,6 +149,22 @@ describe('PieceMovementRawSchema with parallel', () => { const result = PieceMovementRawSchema.safeParse(raw); expect(result.success).toBe(true); }); + + it('should accept provider string in parallel sub-movement', () => { + const raw = { + name: 'parallel-provider-string', + parallel: [ + { + name: 'arch-review', + provider: 'codex', + instruction_template: 'Review architecture', + }, + ], + }; + + const result = PieceMovementRawSchema.safeParse(raw); + expect(result.success).toBe(true); + }); }); describe('PieceConfigRawSchema with parallel movements', () => { diff --git a/src/__tests__/provider-options-piece-parser.test.ts b/src/__tests__/provider-options-piece-parser.test.ts index 5972750..9b200f4 100644 --- a/src/__tests__/provider-options-piece-parser.test.ts +++ b/src/__tests__/provider-options-piece-parser.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { normalizePieceConfig, mergeProviderOptions } from '../infra/config/loaders/pieceParser.js'; +import { normalizePieceConfig } from '../infra/config/loaders/pieceParser.js'; +import { mergeProviderOptions } from '../infra/config/providerOptions.js'; describe('normalizePieceConfig provider_options', () => { it('piece-level global を movement に継承し、movement 側で上書きできる', () => { @@ -114,6 +115,192 @@ describe('normalizePieceConfig provider_options', () => { prepare: ['gradle', 'node'], }); }); + + it('movement の provider block を provider/model/providerOptions に正規化する', () => { + const raw = { + name: 'provider-block-movement', + movements: [ + { + name: 'implement', + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: false, + }, + instruction: '{task}', + }, + ], + }; + + const config = normalizePieceConfig(raw, process.cwd()); + + expect(config.movements[0]?.provider).toBe('codex'); + expect(config.movements[0]?.model).toBe('gpt-5.3'); + expect(config.movements[0]?.providerOptions).toEqual({ + codex: { networkAccess: false }, + }); + }); + + it('piece_config の provider block を movement 既定値として継承する', () => { + const raw = { + name: 'provider-block-piece-config', + piece_config: { + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: true, + }, + }, + movements: [ + { + name: 'plan', + instruction: '{task}', + }, + ], + }; + + const config = normalizePieceConfig(raw, process.cwd()); + + expect(config.providerOptions).toEqual({ + codex: { networkAccess: true }, + }); + expect(config.movements[0]?.provider).toBe('codex'); + expect(config.movements[0]?.model).toBe('gpt-5.3'); + expect(config.movements[0]?.providerOptions).toEqual({ + codex: { networkAccess: true }, + }); + }); + + it('provider block で claude に network_access を指定した場合はエラーにする', () => { + const raw = { + name: 'invalid-provider-block', + movements: [ + { + name: 'review', + provider: { + type: 'claude', + network_access: true, + }, + instruction: '{task}', + }, + ], + }; + + expect(() => normalizePieceConfig(raw, process.cwd())).toThrow(/network_access/); + }); + + it('provider block で codex に sandbox を指定した場合はエラーにする', () => { + const raw = { + name: 'invalid-provider-block', + piece_config: { + provider: { + type: 'codex', + sandbox: { + allow_unsandboxed_commands: true, + }, + }, + }, + movements: [ + { + name: 'review', + instruction: '{task}', + }, + ], + }; + + expect(() => normalizePieceConfig(raw, process.cwd())).toThrow(/sandbox/); + }); + + it('parallel サブムーブメントは親ムーブメントの provider block を継承する', () => { + const raw = { + name: 'provider-block-parallel-inherit', + piece_config: { + provider: { + type: 'claude', + model: 'sonnet', + }, + }, + movements: [ + { + name: 'reviewers', + provider: { + type: 'codex', + model: 'gpt-5.3', + network_access: true, + }, + parallel: [ + { + name: 'arch-review', + instruction: '{task}', + }, + ], + instruction: '{task}', + }, + ], + }; + + const config = normalizePieceConfig(raw, process.cwd()); + const parent = config.movements[0]; + const child = parent?.parallel?.[0]; + + expect(parent?.provider).toBe('codex'); + expect(parent?.model).toBe('gpt-5.3'); + expect(child?.provider).toBe('codex'); + expect(child?.model).toBe('gpt-5.3'); + expect(child?.providerOptions).toEqual({ + codex: { networkAccess: true }, + }); + }); + + it('parallel の provider block で claude に network_access 指定時はエラーにする', () => { + const raw = { + name: 'invalid-provider-block-parallel', + movements: [ + { + name: 'review', + parallel: [ + { + name: 'arch-review', + provider: { + type: 'claude', + network_access: true, + }, + instruction: '{task}', + }, + ], + instruction: '{task}', + }, + ], + }; + + expect(() => normalizePieceConfig(raw, process.cwd())).toThrow(/network_access/); + }); + + it('parallel の provider block で codex に sandbox 指定時はエラーにする', () => { + const raw = { + name: 'invalid-provider-block-parallel', + movements: [ + { + name: 'review', + parallel: [ + { + name: 'arch-review', + provider: { + type: 'codex', + sandbox: { + allow_unsandboxed_commands: true, + }, + }, + instruction: '{task}', + }, + ], + instruction: '{task}', + }, + ], + }; + + expect(() => normalizePieceConfig(raw, process.cwd())).toThrow(/sandbox/); + }); }); describe('mergeProviderOptions', () => { diff --git a/src/__tests__/requeueHelpers.test.ts b/src/__tests__/requeueHelpers.test.ts new file mode 100644 index 0000000..0caf472 --- /dev/null +++ b/src/__tests__/requeueHelpers.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; + +const { mockDebug } = vi.hoisted(() => ({ + mockDebug: vi.fn(), +})); + +vi.mock('../shared/utils/index.js', async (importOriginal) => ({ + ...(await importOriginal>()), + createLogger: () => ({ + debug: (...args: unknown[]) => mockDebug(...args), + info: vi.fn(), + error: vi.fn(), + enter: vi.fn(), + exit: vi.fn(), + }), +})); + +import { hasDeprecatedProviderConfig } from '../features/tasks/list/requeueHelpers.js'; + +describe('hasDeprecatedProviderConfig', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('YAML parse エラーを debug 記録しつつ有効な候補で判定を続行する', () => { + const orderContent = [ + '```yaml', + 'movements: [', + '```', + '', + '```yaml', + 'movements:', + ' - name: review', + ' provider_options:', + ' codex:', + ' network_access: true', + '```', + ].join('\n'); + + expect(hasDeprecatedProviderConfig(orderContent)).toBe(true); + expect(mockDebug).toHaveBeenCalledTimes(1); + expect(mockDebug).toHaveBeenCalledWith( + 'Failed to parse YAML candidate for deprecated provider config detection', + expect.objectContaining({ error: expect.any(String) }), + ); + }); + + it('provider block 新記法のみの piece config は deprecated と判定しない', () => { + const orderContent = [ + 'movements:', + ' - name: review', + ' provider:', + ' type: codex', + ' model: gpt-5.3', + ' network_access: true', + ].join('\n'); + + expect(hasDeprecatedProviderConfig(orderContent)).toBe(false); + }); + + it('provider object と同階層 model の旧記法を deprecated と判定する', () => { + const orderContent = [ + 'movements:', + ' - name: review', + ' provider:', + ' type: codex', + ' network_access: true', + ' model: gpt-5.3', + ].join('\n'); + + expect(hasDeprecatedProviderConfig(orderContent)).toBe(true); + }); + + it('循環参照を含む YAML でもスタックオーバーフローせず判定できる', () => { + const orderContent = [ + 'movements:', + ' - &step', + ' name: review', + ' provider:', + ' type: codex', + ' model: gpt-5.3', + ' network_access: true', + ' self: *step', + ].join('\n'); + + expect(hasDeprecatedProviderConfig(orderContent)).toBe(false); + }); +}); diff --git a/src/__tests__/taskInstructionActions.test.ts b/src/__tests__/taskInstructionActions.test.ts index 816ee08..d42b36f 100644 --- a/src/__tests__/taskInstructionActions.test.ts +++ b/src/__tests__/taskInstructionActions.test.ts @@ -14,6 +14,8 @@ const { mockListRecentRuns, mockSelectRun, mockLoadRunSessionContext, + mockFindPreviousOrderContent, + mockWarn, } = vi.hoisted(() => ({ mockExistsSync: vi.fn(() => true), mockStartReExecution: vi.fn(), @@ -28,6 +30,8 @@ const { mockListRecentRuns: vi.fn(() => []), mockSelectRun: vi.fn(() => null), mockLoadRunSessionContext: vi.fn(), + mockFindPreviousOrderContent: vi.fn(() => null), + mockWarn: vi.fn(), })); vi.mock('node:fs', async (importOriginal) => ({ @@ -83,7 +87,7 @@ vi.mock('../features/interactive/index.js', () => ({ selectRun: (...args: unknown[]) => mockSelectRun(...args), loadRunSessionContext: (...args: unknown[]) => mockLoadRunSessionContext(...args), findRunForTask: vi.fn(() => null), - findPreviousOrderContent: vi.fn(() => null), + findPreviousOrderContent: (...args: unknown[]) => mockFindPreviousOrderContent(...args), })); vi.mock('../features/tasks/execute/taskExecution.js', () => ({ @@ -93,6 +97,7 @@ vi.mock('../features/tasks/execute/taskExecution.js', () => ({ vi.mock('../shared/ui/index.js', () => ({ info: vi.fn(), error: vi.fn(), + warn: mockWarn, })); vi.mock('../shared/utils/index.js', async (importOriginal) => ({ @@ -122,6 +127,7 @@ describe('instructBranch direct execution flow', () => { mockResolveLanguage.mockReturnValue('en'); mockListRecentRuns.mockReturnValue([]); mockSelectRun.mockResolvedValue(null); + mockFindPreviousOrderContent.mockReturnValue(null); mockStartReExecution.mockReturnValue({ name: 'done-task', content: 'done', @@ -234,6 +240,94 @@ describe('instructBranch direct execution flow', () => { ); }); + it('should show deprecated config warning when selected run order uses legacy provider fields', async () => { + mockListRecentRuns.mockReturnValue([ + { slug: 'run-1', task: 'fix', piece: 'default', status: 'completed', startTime: '2026-02-18T00:00:00Z' }, + ]); + mockSelectRun.mockResolvedValue('run-1'); + mockLoadRunSessionContext.mockReturnValue({ + task: 'fix', + piece: 'default', + status: 'completed', + movementLogs: [], + reports: [], + }); + mockFindPreviousOrderContent.mockReturnValue([ + 'movements:', + ' - name: review', + ' provider: codex', + ' model: gpt-5.3', + ' provider_options:', + ' codex:', + ' network_access: true', + ].join('\n')); + + await instructBranch('/project', { + kind: 'completed', + name: 'done-task', + createdAt: '2026-02-14T00:00:00.000Z', + filePath: '/project/.takt/tasks.yaml', + content: 'done', + branch: 'takt/done-task', + worktreePath: '/project/.takt/worktrees/done-task', + data: { task: 'done' }, + }); + + expect(mockWarn).toHaveBeenCalledTimes(1); + expect(mockWarn).toHaveBeenCalledWith(expect.stringContaining('deprecated')); + }); + + it('should not warn for markdown explanatory snippets without piece config body', async () => { + mockFindPreviousOrderContent.mockReturnValue([ + '# Deprecated examples', + '', + '```yaml', + 'provider: codex', + 'model: gpt-5.3', + 'provider_options:', + ' codex:', + ' network_access: true', + '```', + ].join('\n')); + + await instructBranch('/project', { + kind: 'completed', + name: 'done-task', + createdAt: '2026-02-14T00:00:00.000Z', + filePath: '/project/.takt/tasks.yaml', + content: 'done', + branch: 'takt/done-task', + worktreePath: '/project/.takt/worktrees/done-task', + data: { task: 'done' }, + }); + + expect(mockWarn).not.toHaveBeenCalled(); + }); + + it('should not warn when selected run order uses provider block format', async () => { + mockFindPreviousOrderContent.mockReturnValue([ + 'movements:', + ' - name: review', + ' provider:', + ' type: codex', + ' model: gpt-5.3', + ' network_access: true', + ].join('\n')); + + await instructBranch('/project', { + kind: 'completed', + name: 'done-task', + createdAt: '2026-02-14T00:00:00.000Z', + filePath: '/project/.takt/tasks.yaml', + content: 'done', + branch: 'takt/done-task', + worktreePath: '/project/.takt/worktrees/done-task', + data: { task: 'done' }, + }); + + expect(mockWarn).not.toHaveBeenCalled(); + }); + it('should return false when worktree does not exist', async () => { mockExistsSync.mockReturnValue(false); diff --git a/src/__tests__/taskRetryActions.test.ts b/src/__tests__/taskRetryActions.test.ts index 5eee5fb..10c960d 100644 --- a/src/__tests__/taskRetryActions.test.ts +++ b/src/__tests__/taskRetryActions.test.ts @@ -9,9 +9,11 @@ const { mockGetPieceDescription, mockRunRetryMode, mockFindRunForTask, + mockFindPreviousOrderContent, mockStartReExecution, mockRequeueTask, mockExecuteAndCompleteTask, + mockWarn, } = vi.hoisted(() => ({ mockExistsSync: vi.fn(() => true), mockSelectPiece: vi.fn(), @@ -26,9 +28,11 @@ const { })), mockRunRetryMode: vi.fn(), mockFindRunForTask: vi.fn(() => null), + mockFindPreviousOrderContent: vi.fn(() => null), mockStartReExecution: vi.fn(), mockRequeueTask: vi.fn(), mockExecuteAndCompleteTask: vi.fn(), + mockWarn: vi.fn(), })); vi.mock('node:fs', async (importOriginal) => ({ @@ -49,6 +53,7 @@ vi.mock('../shared/ui/index.js', () => ({ header: vi.fn(), blankLine: vi.fn(), status: vi.fn(), + warn: (...args: unknown[]) => mockWarn(...args), })); vi.mock('../shared/utils/index.js', async (importOriginal) => ({ @@ -77,7 +82,7 @@ vi.mock('../features/interactive/index.js', () => ({ runReports: '', })), runRetryMode: (...args: unknown[]) => mockRunRetryMode(...args), - findPreviousOrderContent: vi.fn(() => null), + findPreviousOrderContent: (...args: unknown[]) => mockFindPreviousOrderContent(...args), })); vi.mock('../infra/task/index.js', () => ({ @@ -146,6 +151,7 @@ beforeEach(() => { mockLoadPieceByIdentifier.mockReturnValue(defaultPieceConfig); mockSelectOptionWithDefault.mockResolvedValue('plan'); mockRunRetryMode.mockResolvedValue({ action: 'execute', task: '追加指示A' }); + mockFindPreviousOrderContent.mockReturnValue(null); mockStartReExecution.mockReturnValue({ name: 'my-task', content: 'Do something', @@ -224,6 +230,40 @@ describe('retryFailedTask', () => { expect(mockFindRunForTask).toHaveBeenCalledWith('/project/.takt/worktrees/my-task', 'Do something'); }); + it('should show deprecated config warning when selected run order uses legacy provider fields', async () => { + const task = makeFailedTask(); + mockFindPreviousOrderContent.mockReturnValue([ + 'movements:', + ' - name: review', + ' provider: codex', + ' model: gpt-5.3', + ' provider_options:', + ' codex:', + ' network_access: true', + ].join('\n')); + + await retryFailedTask(task, '/project'); + + expect(mockWarn).toHaveBeenCalledTimes(1); + expect(mockWarn).toHaveBeenCalledWith(expect.stringContaining('deprecated')); + }); + + it('should not warn when selected run order uses provider block format', async () => { + const task = makeFailedTask(); + mockFindPreviousOrderContent.mockReturnValue([ + 'movements:', + ' - name: review', + ' provider:', + ' type: codex', + ' model: gpt-5.3', + ' network_access: true', + ].join('\n')); + + await retryFailedTask(task, '/project'); + + expect(mockWarn).not.toHaveBeenCalled(); + }); + it('should throw when worktree path is not set', async () => { const task = makeFailedTask({ worktreePath: undefined }); diff --git a/src/agents/runner.ts b/src/agents/runner.ts index a34ab65..f406163 100644 --- a/src/agents/runner.ts +++ b/src/agents/runner.ts @@ -36,26 +36,25 @@ export class AgentRunner { } { const localConfig = loadProjectConfig(cwd); const globalConfig = loadGlobalConfig(); - - const resolvedProviderModel = resolveAgentProviderModel({ - personaDisplayName, + const resolved = resolveAgentProviderModel({ cliProvider: options?.provider, cliModel: options?.model, + personaProviders: globalConfig.personaProviders, + personaDisplayName, 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; + const resolvedProvider = resolved.provider; if (!resolvedProvider) { throw new Error('No provider configured. Set "provider" in ~/.takt/config.yaml'); } return { provider: resolvedProvider, - model: resolvedProviderModel.model, + model: resolved.model, localConfig, globalConfig, }; diff --git a/src/core/models/schemas.ts b/src/core/models/schemas.ts index fcda1e4..41888df 100644 --- a/src/core/models/schemas.ts +++ b/src/core/models/schemas.ts @@ -80,6 +80,48 @@ export const MovementProviderOptionsSchema = z.object({ /** Provider key schema for profile maps */ export const ProviderProfileNameSchema = z.enum(['claude', 'codex', 'opencode', 'cursor', 'copilot', 'mock']); +export const ProviderTypeSchema = ProviderProfileNameSchema; + +export const ProviderBlockSchema = z.object({ + type: ProviderTypeSchema, + model: z.string().optional(), + network_access: z.boolean().optional(), + sandbox: ClaudeSandboxSchema, +}).strict().superRefine((provider, ctx) => { + const hasNetworkAccess = provider.network_access !== undefined; + const hasSandbox = provider.sandbox !== undefined; + + if (provider.type === 'claude') { + if (hasNetworkAccess) { + ctx.addIssue({ + code: 'custom', + path: ['network_access'], + message: "provider.type 'claude' does not support 'network_access'.", + }); + } + return; + } + + if (provider.type === 'codex' || provider.type === 'opencode') { + if (hasSandbox) { + ctx.addIssue({ + code: 'custom', + path: ['sandbox'], + message: `provider.type '${provider.type}' does not support 'sandbox'.`, + }); + } + return; + } + + if (hasNetworkAccess || hasSandbox) { + ctx.addIssue({ + code: 'custom', + message: `provider.type '${provider.type}' does not support provider-specific options in provider block.`, + }); + } +}); + +export const ProviderReferenceSchema = z.union([ProviderTypeSchema, ProviderBlockSchema]); /** Provider permission profile schema */ export const ProviderPermissionProfileSchema = z.object({ @@ -111,6 +153,7 @@ export const RuntimeConfigSchema = z.object({ /** Piece-level provider options schema */ export const PieceProviderOptionsSchema = z.object({ + provider: ProviderReferenceSchema.optional(), provider_options: MovementProviderOptionsSchema, runtime: RuntimeConfigSchema, }).optional(); @@ -262,7 +305,7 @@ export const ParallelSubMovementRawSchema = z.object({ knowledge: z.union([z.string(), z.array(z.string())]).optional(), allowed_tools: z.array(z.string()).optional(), mcp_servers: McpServersSchema, - provider: z.enum(['claude', 'codex', 'opencode', 'cursor', 'copilot', 'mock']).optional(), + provider: ProviderReferenceSchema.optional(), model: z.string().optional(), /** Deprecated alias */ permission_mode: z.never().optional(), @@ -295,7 +338,7 @@ export const PieceMovementRawSchema = z.object({ knowledge: z.union([z.string(), z.array(z.string())]).optional(), allowed_tools: z.array(z.string()).optional(), mcp_servers: McpServersSchema, - provider: z.enum(['claude', 'codex', 'opencode', 'cursor', 'copilot', 'mock']).optional(), + provider: ProviderReferenceSchema.optional(), model: z.string().optional(), /** Deprecated alias */ permission_mode: z.never().optional(), @@ -386,9 +429,23 @@ export const PieceConfigRawSchema = z.object({ }); export const PersonaProviderEntrySchema = z.object({ - provider: z.enum(['claude', 'codex', 'opencode', 'cursor', 'copilot', 'mock']).optional(), + provider: ProviderTypeSchema.optional(), model: z.string().optional(), -}); +}).strict().refine( + (entry) => entry.provider !== undefined || entry.model !== undefined, + { message: "persona_providers entry must include either 'provider' or 'model'" } +); + +export const PersonaProviderBlockSchema = z.object({ + type: ProviderTypeSchema, + model: z.string().optional(), +}).strict(); + +export const PersonaProviderReferenceSchema = z.union([ + ProviderTypeSchema, + PersonaProviderBlockSchema, + PersonaProviderEntrySchema, +]); /** Custom agent configuration schema */ export const CustomAgentConfigSchema = z.object({ @@ -442,7 +499,7 @@ export const PieceCategoryConfigSchema = z.record(z.string(), PieceCategoryConfi export const GlobalConfigSchema = z.object({ language: LanguageSchema.optional().default(DEFAULT_LANGUAGE), log_level: z.enum(['debug', 'info', 'warn', 'error']).optional().default('info'), - provider: z.enum(['claude', 'codex', 'opencode', 'cursor', 'copilot', 'mock']).optional().default('claude'), + provider: ProviderReferenceSchema.optional().default('claude'), model: z.string().optional(), /** Default piece name for new tasks */ piece: z.string().optional(), @@ -485,10 +542,7 @@ export const GlobalConfigSchema = z.object({ /** Path to piece categories file (default: ~/.takt/preferences/piece-categories.yaml) */ piece_categories_file: z.string().optional(), /** Per-persona provider and model overrides. */ - persona_providers: z.record(z.string(), z.union([ - z.enum(['claude', 'codex', 'opencode', 'cursor', 'copilot', 'mock']), - PersonaProviderEntrySchema, - ])).optional(), + persona_providers: z.record(z.string(), PersonaProviderReferenceSchema).optional(), /** Global provider-specific options (lowest priority) */ provider_options: MovementProviderOptionsSchema, /** Provider-specific permission profiles */ @@ -528,8 +582,14 @@ export const GlobalConfigSchema = z.object({ /** Project config schema */ export const ProjectConfigSchema = z.object({ piece: z.string().optional(), - provider: z.enum(['claude', 'codex', 'opencode', 'cursor', 'copilot', 'mock']).optional(), + verbose: z.boolean().optional(), + provider: ProviderReferenceSchema.optional(), model: z.string().optional(), + analytics: AnalyticsConfigSchema.optional(), + /** Auto-create PR after worktree execution (project override) */ + auto_pr: z.boolean().optional(), + /** Create PR as draft (project override) */ + draft_pr: z.boolean().optional(), provider_options: MovementProviderOptionsSchema, provider_profiles: ProviderPermissionProfilesSchema, /** Number of tasks to run concurrently in takt run (default from global: 1, max: 10) */ @@ -541,10 +601,10 @@ export const ProjectConfigSchema = z.object({ /** Submodule acquisition mode (all or explicit path list) */ submodules: z.union([ z.string().refine((value) => value.trim().toLowerCase() === 'all', { - message: 'submodules string value must be "all"', + message: 'Invalid submodules: string value must be "all"', }), z.array(z.string().min(1)).refine((paths) => paths.every((path) => !path.includes('*')), { - message: 'submodules path entries must not include wildcard "*"', + message: 'Invalid submodules: path entries must not include wildcard "*"', }), ]).optional(), /** Compatibility flag for full submodule acquisition when submodules is unset */ diff --git a/src/core/piece/engine/OptionsBuilder.ts b/src/core/piece/engine/OptionsBuilder.ts index b4ca798..b513a6a 100644 --- a/src/core/piece/engine/OptionsBuilder.ts +++ b/src/core/piece/engine/OptionsBuilder.ts @@ -29,13 +29,10 @@ function mergeProviderOptions( } function resolveMovementProviderOptions( - source: 'env' | 'project' | 'global' | 'default' | undefined, + _source: 'env' | 'project' | 'global' | 'default' | undefined, resolvedConfigOptions: MovementProviderOptions | undefined, movementOptions: MovementProviderOptions | undefined, ): MovementProviderOptions | undefined { - if (source === 'env' || source === 'project') { - return mergeProviderOptions(movementOptions, resolvedConfigOptions); - } return mergeProviderOptions(resolvedConfigOptions, movementOptions); } diff --git a/src/features/tasks/list/requeueHelpers.ts b/src/features/tasks/list/requeueHelpers.ts index c1b045d..fad9074 100644 --- a/src/features/tasks/list/requeueHelpers.ts +++ b/src/features/tasks/list/requeueHelpers.ts @@ -1,5 +1,7 @@ import { confirm } from '../../../shared/prompt/index.js'; import { getLabel } from '../../../shared/i18n/index.js'; +import { createLogger, getErrorMessage } from '../../../shared/utils/index.js'; +import { parse as parseYaml } from 'yaml'; import { selectRun, loadRunSessionContext, @@ -7,6 +9,10 @@ import { type RunSessionContext, } from '../../interactive/index.js'; +const log = createLogger('list-tasks'); +export const DEPRECATED_PROVIDER_CONFIG_WARNING = + 'Detected deprecated provider config in selected run order.md. Please migrate legacy fields to the provider block.'; + export function appendRetryNote(existing: string | undefined, additional: string): string { const trimmedAdditional = additional.trim(); if (trimmedAdditional === '') { @@ -18,6 +24,102 @@ export function appendRetryNote(existing: string | undefined, additional: string return `${existing}\n\n${trimmedAdditional}`; } +function extractYamlCandidates(content: string): string[] { + const blockPattern = /```(?:yaml|yml)\s*\n([\s\S]*?)```/gi; + const candidates: string[] = []; + let match: RegExpExecArray | null; + while ((match = blockPattern.exec(content)) !== null) { + if (match[1]) { + candidates.push(match[1]); + } + } + if (candidates.length > 0) { + return candidates; + } + return [content]; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isPieceConfigLike(value: unknown): value is Record { + return isRecord(value) && Array.isArray(value.movements); +} + +const MAX_PROVIDER_SCAN_NODES = 10000; + +function hasDeprecatedProviderConfigInObject( + value: unknown, + visited: WeakSet, + state: { visitedNodes: number }, +): boolean { + if (isRecord(value)) { + if (visited.has(value)) { + return false; + } + visited.add(value); + } + + state.visitedNodes += 1; + if (state.visitedNodes > MAX_PROVIDER_SCAN_NODES) { + return false; + } + + if (Array.isArray(value)) { + for (const item of value) { + if (hasDeprecatedProviderConfigInObject(item, visited, state)) { + return true; + } + } + return false; + } + if (!isRecord(value)) { + return false; + } + + if ('provider_options' in value) { + return true; + } + if ('provider' in value && typeof value.model === 'string') { + return true; + } + + for (const entry of Object.values(value)) { + if (hasDeprecatedProviderConfigInObject(entry, visited, state)) { + return true; + } + } + + return false; +} + +export function hasDeprecatedProviderConfig(orderContent: string | null): boolean { + if (!orderContent) { + return false; + } + + const yamlCandidates = extractYamlCandidates(orderContent); + for (const candidate of yamlCandidates) { + let parsed: unknown; + try { + parsed = parseYaml(candidate); + } catch (error) { + log.debug('Failed to parse YAML candidate for deprecated provider config detection', { + error: getErrorMessage(error), + }); + continue; + } + if ( + isPieceConfigLike(parsed) + && hasDeprecatedProviderConfigInObject(parsed, new WeakSet(), { visitedNodes: 0 }) + ) { + return true; + } + } + return false; +} + export async function selectRunSessionContext( projectDir: string, lang: 'en' | 'ja', diff --git a/src/features/tasks/list/taskInstructionActions.ts b/src/features/tasks/list/taskInstructionActions.ts index 4f16405..d4445bd 100644 --- a/src/features/tasks/list/taskInstructionActions.ts +++ b/src/features/tasks/list/taskInstructionActions.ts @@ -12,7 +12,7 @@ import { detectDefaultBranch, } from '../../../infra/task/index.js'; import { resolvePieceConfigValues, getPieceDescription } from '../../../infra/config/index.js'; -import { info, error as logError } from '../../../shared/ui/index.js'; +import { info, warn, error as logError } from '../../../shared/ui/index.js'; import { createLogger, getErrorMessage } from '../../../shared/utils/index.js'; import { runInstructMode } from './instructMode.js'; import { selectPiece } from '../../pieceSelection/index.js'; @@ -20,7 +20,12 @@ import { dispatchConversationAction } from '../../interactive/actionDispatcher.j import type { PieceContext } from '../../interactive/interactive.js'; import { resolveLanguage, findRunForTask, findPreviousOrderContent } from '../../interactive/index.js'; import { type BranchActionTarget, resolveTargetBranch } from './taskActionTarget.js'; -import { appendRetryNote, selectRunSessionContext } from './requeueHelpers.js'; +import { + appendRetryNote, + DEPRECATED_PROVIDER_CONFIG_WARNING, + hasDeprecatedProviderConfig, + selectRunSessionContext, +} from './requeueHelpers.js'; import { executeAndCompleteTask } from '../execute/taskExecution.js'; const log = createLogger('list-tasks'); @@ -107,6 +112,9 @@ export async function instructBranch( const runSessionContext = await selectRunSessionContext(worktreePath, lang); const matchedSlug = findRunForTask(worktreePath, target.content); const previousOrderContent = findPreviousOrderContent(worktreePath, matchedSlug); + if (hasDeprecatedProviderConfig(previousOrderContent)) { + warn(DEPRECATED_PROVIDER_CONFIG_WARNING); + } const branchContext = getBranchContext(projectDir, branch); diff --git a/src/features/tasks/list/taskRetryActions.ts b/src/features/tasks/list/taskRetryActions.ts index 565faf6..2ff1d40 100644 --- a/src/features/tasks/list/taskRetryActions.ts +++ b/src/features/tasks/list/taskRetryActions.ts @@ -12,7 +12,7 @@ import { loadPieceByIdentifier, resolvePieceConfigValue, getPieceDescription } f import { selectPiece } from '../../pieceSelection/index.js'; import { selectOptionWithDefault } from '../../../shared/prompt/index.js'; import { getLabel } from '../../../shared/i18n/index.js'; -import { info, header, blankLine, status } from '../../../shared/ui/index.js'; +import { info, header, blankLine, status, warn } from '../../../shared/ui/index.js'; import { createLogger } from '../../../shared/utils/index.js'; import type { PieceConfig } from '../../../core/models/index.js'; import { @@ -27,7 +27,11 @@ import { type RetryRunInfo, } from '../../interactive/index.js'; import { executeAndCompleteTask } from '../execute/taskExecution.js'; -import { appendRetryNote } from './requeueHelpers.js'; +import { + appendRetryNote, + DEPRECATED_PROVIDER_CONFIG_WARNING, + hasDeprecatedProviderConfig, +} from './requeueHelpers.js'; const log = createLogger('list-tasks'); @@ -191,6 +195,9 @@ export async function retryFailedTask( // Runs data lives in the worktree (written during previous execution) const previousOrderContent = findPreviousOrderContent(worktreePath, matchedSlug); + if (hasDeprecatedProviderConfig(previousOrderContent)) { + warn(DEPRECATED_PROVIDER_CONFIG_WARNING); + } blankLine(); const branchName = task.branch ?? task.name; diff --git a/src/infra/config/global/globalConfig.ts b/src/infra/config/global/globalConfig.ts index 4ad58d6..778170b 100644 --- a/src/infra/config/global/globalConfig.ts +++ b/src/infra/config/global/globalConfig.ts @@ -12,13 +12,23 @@ import { GlobalConfigSchema } from '../../../core/models/index.js'; import type { Language } from '../../../core/models/index.js'; import type { PersistedGlobalConfig, PersonaProviderEntry, PieceOverrides } from '../../../core/models/persisted-global-config.js'; import type { ProviderPermissionProfiles } from '../../../core/models/provider-profiles.js'; -import { normalizeProviderOptions } from '../loaders/pieceParser.js'; +import { + normalizeConfigProviderReference, + type ConfigProviderReference, +} from '../providerReference.js'; import { getGlobalConfigPath } from '../paths.js'; import { DEFAULT_LANGUAGE } from '../../../shared/constants.js'; import { parseProviderModel } from '../../../shared/utils/providerModel.js'; import { applyGlobalConfigEnvOverrides, envVarNameFromPath } from '../env/config-env-overrides.js'; import { invalidateAllResolvedConfigCache } from '../resolutionCache.js'; +type ProviderType = NonNullable; +type RawPersonaProviderBlock = { + type: ProviderType; + model?: string; +}; +type RawProviderReference = ConfigProviderReference; + /** Claude-specific model aliases that are not valid for other providers */ const CLAUDE_MODEL_ALIASES = new Set(['opus', 'sonnet', 'haiku']); @@ -83,12 +93,19 @@ function validateProviderModelCompatibility(provider: string | undefined, model: } function normalizePersonaProviders( - raw: Record | PersonaProviderEntry> | undefined, + raw: Record | undefined, ): Record | undefined { if (!raw) return undefined; return Object.fromEntries( Object.entries(raw).map(([persona, entry]) => { - const normalized: PersonaProviderEntry = typeof entry === 'string' ? { provider: entry } : entry; + let normalized: PersonaProviderEntry; + if (typeof entry === 'string') { + normalized = { provider: entry }; + } else if ('type' in entry) { + normalized = { provider: entry.type, model: entry.model }; + } else { + normalized = entry; + } validateProviderModelCompatibility(normalized.provider, normalized.model); return [persona, normalized]; }), @@ -216,11 +233,16 @@ export class GlobalConfigManager { applyGlobalConfigEnvOverrides(rawConfig); const parsed = GlobalConfigSchema.parse(rawConfig); + const normalizedProvider = normalizeConfigProviderReference( + parsed.provider as RawProviderReference, + parsed.model, + parsed.provider_options as Record | undefined, + ); const config: PersistedGlobalConfig = { language: parsed.language, logLevel: parsed.log_level, - provider: parsed.provider, - model: parsed.model, + provider: normalizedProvider.provider, + model: normalizedProvider.model, piece: parsed.piece, observability: parsed.observability ? { providerEvents: parsed.observability.provider_events, @@ -250,8 +272,10 @@ export class GlobalConfigManager { minimalOutput: parsed.minimal_output, bookmarksFile: parsed.bookmarks_file, pieceCategoriesFile: parsed.piece_categories_file, - personaProviders: normalizePersonaProviders(parsed.persona_providers as Record | PersonaProviderEntry> | undefined), - providerOptions: normalizeProviderOptions(parsed.provider_options), + personaProviders: normalizePersonaProviders( + parsed.persona_providers as Record | undefined, + ), + providerOptions: normalizedProvider.providerOptions, providerProfiles: normalizeProviderProfiles(parsed.provider_profiles as Record }> | undefined), runtime: parsed.runtime?.prepare && parsed.runtime.prepare.length > 0 ? { prepare: [...new Set(parsed.runtime.prepare)] } diff --git a/src/infra/config/loaders/pieceParser.ts b/src/infra/config/loaders/pieceParser.ts index a8c866b..6868fbc 100644 --- a/src/infra/config/loaders/pieceParser.ts +++ b/src/infra/config/loaders/pieceParser.ts @@ -32,60 +32,26 @@ import type { PieceOverrides } from '../../../core/models/persisted-global-confi import { applyQualityGateOverrides } from './qualityGateOverrides.js'; import { loadProjectConfig } from '../project/projectConfig.js'; import { loadGlobalConfig } from '../global/globalConfig.js'; +import { normalizeConfigProviderReferenceDetailed, type ConfigProviderReference } from '../providerReference.js'; +import { mergeProviderOptions } from '../providerOptions.js'; -/** Convert raw YAML provider_options (snake_case) to internal format (camelCase). */ -export function normalizeProviderOptions( - raw: RawStep['provider_options'], -): MovementProviderOptions | undefined { - if (!raw) return undefined; +type RawProviderReference = RawStep['provider']; - const result: MovementProviderOptions = {}; - if (raw.codex?.network_access !== undefined) { - result.codex = { networkAccess: raw.codex.network_access }; - } - if (raw.opencode?.network_access !== undefined) { - result.opencode = { networkAccess: raw.opencode.network_access }; - } - if (raw.claude?.sandbox) { - result.claude = { - sandbox: { - ...(raw.claude.sandbox.allow_unsandboxed_commands !== undefined - ? { allowUnsandboxedCommands: raw.claude.sandbox.allow_unsandboxed_commands } - : {}), - ...(raw.claude.sandbox.excluded_commands !== undefined - ? { excludedCommands: raw.claude.sandbox.excluded_commands } - : {}), - }, - }; - } - return Object.keys(result).length > 0 ? result : undefined; -} - -/** - * Deep merge provider options. Later sources override earlier ones. - * Exported for reuse in runner.ts (4-layer resolution). - */ -export function mergeProviderOptions( - ...layers: (MovementProviderOptions | undefined)[] -): MovementProviderOptions | undefined { - const result: MovementProviderOptions = {}; - - for (const layer of layers) { - if (!layer) continue; - if (layer.codex) { - result.codex = { ...result.codex, ...layer.codex }; - } - if (layer.opencode) { - result.opencode = { ...result.opencode, ...layer.opencode }; - } - if (layer.claude?.sandbox) { - result.claude = { - sandbox: { ...result.claude?.sandbox, ...layer.claude.sandbox }, - }; - } - } - - return Object.keys(result).length > 0 ? result : undefined; +function normalizeProviderReference( + provider: RawProviderReference, + model: RawStep['model'], + providerOptions: RawStep['provider_options'], +): { + provider: PieceMovement['provider']; + model: PieceMovement['model']; + providerOptions: MovementProviderOptions | undefined; + providerSpecified: boolean; +} { + return normalizeConfigProviderReferenceDetailed( + provider as ConfigProviderReference>, + model, + providerOptions as Record | undefined, + ); } function normalizeRuntimeConfig(raw: RawPiece['piece_config']): PieceRuntimeConfig | undefined { @@ -280,6 +246,8 @@ function normalizeStepFromRaw( step: RawStep, pieceDir: string, sections: PieceSections, + inheritedProvider?: PieceMovement['provider'], + inheritedModel?: PieceMovement['model'], inheritedProviderOptions?: PieceMovement['providerOptions'], context?: FacetResolutionContext, projectOverrides?: PieceOverrides, @@ -298,6 +266,7 @@ function normalizeStepFromRaw( const knowledgeRef = (step as Record).knowledge as string | string[] | undefined; const knowledgeContents = resolveRefList(knowledgeRef, sections.resolvedKnowledge, pieceDir, 'knowledge', context); + const normalizedProvider = normalizeProviderReference(step.provider, step.model, step.provider_options); const expandedInstruction = step.instruction ? resolveRefToContent(step.instruction, sections.resolvedInstructions, pieceDir, 'instructions', context) @@ -312,10 +281,10 @@ function normalizeStepFromRaw( personaPath, allowedTools: step.allowed_tools, mcpServers: step.mcp_servers, - provider: step.provider, - model: step.model, + provider: normalizedProvider.provider ?? inheritedProvider, + model: normalizedProvider.model ?? (normalizedProvider.providerSpecified ? undefined : inheritedModel), requiredPermissionMode: step.required_permission_mode, - providerOptions: mergeProviderOptions(inheritedProviderOptions, normalizeProviderOptions(step.provider_options)), + providerOptions: mergeProviderOptions(inheritedProviderOptions, normalizedProvider.providerOptions), edit: step.edit, instructionTemplate: (step.instruction_template ? resolveRefToContent(step.instruction_template, sections.resolvedInstructions, pieceDir, 'instructions', context) @@ -336,7 +305,17 @@ function normalizeStepFromRaw( if (step.parallel && step.parallel.length > 0) { result.parallel = step.parallel.map((sub: RawStep) => - normalizeStepFromRaw(sub, pieceDir, sections, inheritedProviderOptions, context, projectOverrides, globalOverrides), + normalizeStepFromRaw( + sub, + pieceDir, + sections, + result.provider, + result.model, + result.providerOptions, + context, + projectOverrides, + globalOverrides, + ), ); } @@ -412,11 +391,18 @@ export function normalizePieceConfig( resolvedReportFormats, }; - const pieceProviderOptions = normalizeProviderOptions(parsed.piece_config?.provider_options as RawStep['provider_options']); + const normalizedPieceProvider = normalizeProviderReference( + parsed.piece_config?.provider as RawProviderReference, + undefined, + parsed.piece_config?.provider_options as RawStep['provider_options'], + ); + const pieceProvider = normalizedPieceProvider.provider; + const pieceModel = normalizedPieceProvider.model; + const pieceProviderOptions = normalizedPieceProvider.providerOptions; const pieceRuntime = normalizeRuntimeConfig(parsed.piece_config); const movements: PieceMovement[] = parsed.movements.map((step) => - normalizeStepFromRaw(step, pieceDir, sections, pieceProviderOptions, context, projectOverrides, globalOverrides), + normalizeStepFromRaw(step, pieceDir, sections, pieceProvider, pieceModel, pieceProviderOptions, context, projectOverrides, globalOverrides), ); // Schema guarantees movements.min(1) diff --git a/src/infra/config/project/projectConfig.ts b/src/infra/config/project/projectConfig.ts index 78a9b39..ce5beed 100644 --- a/src/infra/config/project/projectConfig.ts +++ b/src/infra/config/project/projectConfig.ts @@ -7,12 +7,16 @@ import { existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs'; import { join, resolve } from 'node:path'; import { parse, stringify } from 'yaml'; +import { ProjectConfigSchema } from '../../../core/models/index.js'; import { copyProjectResourcesToDir } from '../../resources/index.js'; import type { ProjectLocalConfig } from '../types.js'; import type { ProviderPermissionProfiles } from '../../../core/models/provider-profiles.js'; import type { AnalyticsConfig, PieceOverrides, SubmoduleSelection } from '../../../core/models/persisted-global-config.js'; import { applyProjectConfigEnvOverrides } from '../env/config-env-overrides.js'; -import { normalizeProviderOptions } from '../loaders/pieceParser.js'; +import { + normalizeConfigProviderReference, + type ConfigProviderReference, +} from '../providerReference.js'; import { invalidateResolvedConfigCache } from '../resolutionCache.js'; export type { ProjectLocalConfig } from '../types.js'; @@ -21,6 +25,8 @@ export type { ProjectLocalConfig } from '../types.js'; const DEFAULT_PROJECT_CONFIG: ProjectLocalConfig = {}; const SUBMODULES_ALL = 'all'; +type ProviderType = NonNullable; +type RawProviderReference = ConfigProviderReference; function normalizeSubmodules(raw: unknown): SubmoduleSelection | undefined { if (raw === undefined) return undefined; @@ -178,20 +184,23 @@ function denormalizePieceOverrides( export function loadProjectConfig(projectDir: string): ProjectLocalConfig { const configPath = getConfigPath(projectDir); - const parsedConfig: Record = {}; + const rawConfig: Record = {}; if (existsSync(configPath)) { try { const content = readFileSync(configPath, 'utf-8'); const parsed = (parse(content) as Record | null) ?? {}; - Object.assign(parsedConfig, parsed); + Object.assign(rawConfig, parsed); } catch { return { ...DEFAULT_PROJECT_CONFIG }; } } - applyProjectConfigEnvOverrides(parsedConfig); + applyProjectConfigEnvOverrides(rawConfig); + const parsedConfig = ProjectConfigSchema.parse(rawConfig); const { + provider, + model, auto_pr, draft_pr, base_branch, @@ -207,6 +216,11 @@ export function loadProjectConfig(projectDir: string): ProjectLocalConfig { copilot_cli_path, ...rest } = parsedConfig; + const normalizedProvider = normalizeConfigProviderReference( + provider as RawProviderReference, + model as string | undefined, + provider_options as Record | undefined, + ); const normalizedSubmodules = normalizeSubmodules(submodules); const normalizedWithSubmodules = normalizeWithSubmodules(with_submodules); @@ -221,16 +235,9 @@ export function loadProjectConfig(projectDir: string): ProjectLocalConfig { submodules: normalizedSubmodules, withSubmodules: effectiveWithSubmodules, analytics: normalizeAnalytics(analytics as Record | undefined), - providerOptions: normalizeProviderOptions(provider_options as { - codex?: { network_access?: boolean }; - opencode?: { network_access?: boolean }; - claude?: { - sandbox?: { - allow_unsandboxed_commands?: boolean; - excluded_commands?: string[]; - }; - }; - } | undefined), + provider: normalizedProvider.provider, + model: normalizedProvider.model, + providerOptions: normalizedProvider.providerOptions, providerProfiles: normalizeProviderProfiles(provider_profiles as Record }> | undefined), pieceOverrides: normalizePieceOverrides(piece_overrides as { quality_gates?: string[]; quality_gates_edit_only?: boolean; movements?: Record } | undefined), claudeCliPath: claude_cli_path as string | undefined, diff --git a/src/infra/config/providerBlockOptions.ts b/src/infra/config/providerBlockOptions.ts new file mode 100644 index 0000000..33918c5 --- /dev/null +++ b/src/infra/config/providerBlockOptions.ts @@ -0,0 +1,37 @@ +import type { MovementProviderOptions } from '../../core/models/piece-types.js'; + +type ProviderBlockSandbox = { + allow_unsandboxed_commands?: boolean; + excluded_commands?: string[]; +}; + +export type ProviderBlockInput = { + type: string; + model?: string; + network_access?: boolean; + sandbox?: ProviderBlockSandbox; +}; + +export function normalizeProviderBlockOptions(provider: ProviderBlockInput): MovementProviderOptions | undefined { + if (provider.type === 'codex' || provider.type === 'opencode') { + if (provider.network_access === undefined) { + return undefined; + } + return { [provider.type]: { networkAccess: provider.network_access } }; + } + if (provider.type === 'claude' && provider.sandbox) { + return { + claude: { + sandbox: { + ...(provider.sandbox.allow_unsandboxed_commands !== undefined + ? { allowUnsandboxedCommands: provider.sandbox.allow_unsandboxed_commands } + : {}), + ...(provider.sandbox.excluded_commands !== undefined + ? { excludedCommands: provider.sandbox.excluded_commands } + : {}), + }, + }, + }; + } + return undefined; +} diff --git a/src/infra/config/providerOptions.ts b/src/infra/config/providerOptions.ts new file mode 100644 index 0000000..3a45506 --- /dev/null +++ b/src/infra/config/providerOptions.ts @@ -0,0 +1,71 @@ +import type { MovementProviderOptions } from '../../core/models/piece-types.js'; + +type RawProviderOptions = { + codex?: { + network_access?: boolean; + }; + opencode?: { + network_access?: boolean; + }; + claude?: { + sandbox?: { + allow_unsandboxed_commands?: boolean; + excluded_commands?: string[]; + }; + }; +}; + +/** Convert raw YAML provider_options (snake_case) to internal format (camelCase). */ +export function normalizeProviderOptions( + raw: RawProviderOptions | Record | undefined, +): MovementProviderOptions | undefined { + if (!raw || typeof raw !== 'object') { + return undefined; + } + + const options = raw as RawProviderOptions; + const result: MovementProviderOptions = {}; + if (options.codex?.network_access !== undefined) { + result.codex = { networkAccess: options.codex.network_access }; + } + if (options.opencode?.network_access !== undefined) { + result.opencode = { networkAccess: options.opencode.network_access }; + } + if (options.claude?.sandbox) { + result.claude = { + sandbox: { + ...(options.claude.sandbox.allow_unsandboxed_commands !== undefined + ? { allowUnsandboxedCommands: options.claude.sandbox.allow_unsandboxed_commands } + : {}), + ...(options.claude.sandbox.excluded_commands !== undefined + ? { excludedCommands: options.claude.sandbox.excluded_commands } + : {}), + }, + }; + } + return Object.keys(result).length > 0 ? result : undefined; +} + +/** Deep merge provider options. Later sources override earlier ones. */ +export function mergeProviderOptions( + ...layers: (MovementProviderOptions | undefined)[] +): MovementProviderOptions | undefined { + const result: MovementProviderOptions = {}; + + for (const layer of layers) { + if (!layer) continue; + if (layer.codex) { + result.codex = { ...result.codex, ...layer.codex }; + } + if (layer.opencode) { + result.opencode = { ...result.opencode, ...layer.opencode }; + } + if (layer.claude?.sandbox) { + result.claude = { + sandbox: { ...result.claude?.sandbox, ...layer.claude.sandbox }, + }; + } + } + + return Object.keys(result).length > 0 ? result : undefined; +} diff --git a/src/infra/config/providerReference.ts b/src/infra/config/providerReference.ts new file mode 100644 index 0000000..2b8423a --- /dev/null +++ b/src/infra/config/providerReference.ts @@ -0,0 +1,67 @@ +import type { MovementProviderOptions } from '../../core/models/piece-types.js'; +import { mergeProviderOptions, normalizeProviderOptions } from './providerOptions.js'; +import { normalizeProviderBlockOptions } from './providerBlockOptions.js'; + +export type ConfigProviderBlock = { + type: ProviderType; + model?: string; + network_access?: boolean; + sandbox?: { + allow_unsandboxed_commands?: boolean; + excluded_commands?: string[]; + }; +}; + +export type ConfigProviderReference = + | ProviderType + | ConfigProviderBlock + | undefined; + +export type NormalizedConfigProviderReference = { + provider: ProviderType | undefined; + model: string | undefined; + providerOptions: MovementProviderOptions | undefined; + providerSpecified: boolean; +}; + +export function normalizeConfigProviderReferenceDetailed( + provider: ConfigProviderReference, + model: string | undefined, + providerOptions: Record | undefined, +): NormalizedConfigProviderReference { + if (typeof provider === 'string' || provider === undefined) { + return { + provider, + model, + providerOptions: normalizeProviderOptions(providerOptions), + providerSpecified: provider !== undefined, + }; + } + + return { + provider: provider.type, + model: provider.model ?? model, + providerOptions: mergeProviderOptions( + normalizeProviderBlockOptions(provider), + normalizeProviderOptions(providerOptions), + ), + providerSpecified: true, + }; +} + +export function normalizeConfigProviderReference( + provider: ConfigProviderReference, + model: string | undefined, + providerOptions: Record | undefined, +): { + provider: ProviderType | undefined; + model: string | undefined; + providerOptions: MovementProviderOptions | undefined; +} { + const normalized = normalizeConfigProviderReferenceDetailed(provider, model, providerOptions); + return { + provider: normalized.provider, + model: normalized.model, + providerOptions: normalized.providerOptions, + }; +}