takt: override-persona-provider (#171)
This commit is contained in:
parent
cc63f4769d
commit
39432db10a
204
src/__tests__/engine-persona-providers.test.ts
Normal file
204
src/__tests__/engine-persona-providers.test.ts
Normal file
@ -0,0 +1,204 @@
|
|||||||
|
/**
|
||||||
|
* Tests for persona_providers config-level provider override.
|
||||||
|
*
|
||||||
|
* Verifies the provider resolution priority:
|
||||||
|
* 1. Movement YAML provider (highest)
|
||||||
|
* 2. persona_providers[personaDisplayName]
|
||||||
|
* 3. CLI/global provider (lowest)
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, it, expect, beforeEach, vi } from 'vitest';
|
||||||
|
|
||||||
|
vi.mock('../agents/runner.js', () => ({
|
||||||
|
runAgent: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../core/piece/evaluation/index.js', () => ({
|
||||||
|
detectMatchedRule: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../core/piece/phase-runner.js', () => ({
|
||||||
|
needsStatusJudgmentPhase: vi.fn(),
|
||||||
|
runReportPhase: vi.fn(),
|
||||||
|
runStatusJudgmentPhase: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../shared/utils/index.js', async (importOriginal) => ({
|
||||||
|
...(await importOriginal<Record<string, unknown>>()),
|
||||||
|
generateReportDir: vi.fn().mockReturnValue('test-report-dir'),
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { PieceEngine } from '../core/piece/index.js';
|
||||||
|
import { runAgent } from '../agents/runner.js';
|
||||||
|
import type { PieceConfig } from '../core/models/index.js';
|
||||||
|
import {
|
||||||
|
makeResponse,
|
||||||
|
makeRule,
|
||||||
|
makeMovement,
|
||||||
|
mockRunAgentSequence,
|
||||||
|
mockDetectMatchedRuleSequence,
|
||||||
|
applyDefaultMocks,
|
||||||
|
} from './engine-test-helpers.js';
|
||||||
|
|
||||||
|
describe('PieceEngine persona_providers override', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.resetAllMocks();
|
||||||
|
applyDefaultMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should use persona_providers when movement has no provider and persona matches', async () => {
|
||||||
|
const movement = makeMovement('implement', {
|
||||||
|
personaDisplayName: 'coder',
|
||||||
|
rules: [makeRule('done', 'COMPLETE')],
|
||||||
|
});
|
||||||
|
const config: PieceConfig = {
|
||||||
|
name: 'persona-provider-test',
|
||||||
|
movements: [movement],
|
||||||
|
initialMovement: 'implement',
|
||||||
|
maxIterations: 1,
|
||||||
|
};
|
||||||
|
|
||||||
|
mockRunAgentSequence([
|
||||||
|
makeResponse({ persona: movement.persona, content: 'done' }),
|
||||||
|
]);
|
||||||
|
mockDetectMatchedRuleSequence([{ index: 0, method: 'phase1_tag' }]);
|
||||||
|
|
||||||
|
const engine = new PieceEngine(config, '/tmp/project', 'test task', {
|
||||||
|
projectCwd: '/tmp/project',
|
||||||
|
provider: 'claude',
|
||||||
|
personaProviders: { coder: 'codex' },
|
||||||
|
});
|
||||||
|
|
||||||
|
await engine.run();
|
||||||
|
|
||||||
|
const options = vi.mocked(runAgent).mock.calls[0][2];
|
||||||
|
expect(options.provider).toBe('codex');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should use global provider when persona is not in persona_providers', async () => {
|
||||||
|
const movement = makeMovement('plan', {
|
||||||
|
personaDisplayName: 'planner',
|
||||||
|
rules: [makeRule('done', 'COMPLETE')],
|
||||||
|
});
|
||||||
|
const config: PieceConfig = {
|
||||||
|
name: 'persona-provider-nomatch',
|
||||||
|
movements: [movement],
|
||||||
|
initialMovement: 'plan',
|
||||||
|
maxIterations: 1,
|
||||||
|
};
|
||||||
|
|
||||||
|
mockRunAgentSequence([
|
||||||
|
makeResponse({ persona: movement.persona, content: 'done' }),
|
||||||
|
]);
|
||||||
|
mockDetectMatchedRuleSequence([{ index: 0, method: 'phase1_tag' }]);
|
||||||
|
|
||||||
|
const engine = new PieceEngine(config, '/tmp/project', 'test task', {
|
||||||
|
projectCwd: '/tmp/project',
|
||||||
|
provider: 'claude',
|
||||||
|
personaProviders: { coder: 'codex' },
|
||||||
|
});
|
||||||
|
|
||||||
|
await engine.run();
|
||||||
|
|
||||||
|
const options = vi.mocked(runAgent).mock.calls[0][2];
|
||||||
|
expect(options.provider).toBe('claude');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should prioritize movement provider over persona_providers', async () => {
|
||||||
|
const movement = makeMovement('implement', {
|
||||||
|
personaDisplayName: 'coder',
|
||||||
|
provider: 'claude',
|
||||||
|
rules: [makeRule('done', 'COMPLETE')],
|
||||||
|
});
|
||||||
|
const config: PieceConfig = {
|
||||||
|
name: 'movement-over-persona',
|
||||||
|
movements: [movement],
|
||||||
|
initialMovement: 'implement',
|
||||||
|
maxIterations: 1,
|
||||||
|
};
|
||||||
|
|
||||||
|
mockRunAgentSequence([
|
||||||
|
makeResponse({ persona: movement.persona, content: 'done' }),
|
||||||
|
]);
|
||||||
|
mockDetectMatchedRuleSequence([{ index: 0, method: 'phase1_tag' }]);
|
||||||
|
|
||||||
|
const engine = new PieceEngine(config, '/tmp/project', 'test task', {
|
||||||
|
projectCwd: '/tmp/project',
|
||||||
|
provider: 'mock',
|
||||||
|
personaProviders: { coder: 'codex' },
|
||||||
|
});
|
||||||
|
|
||||||
|
await engine.run();
|
||||||
|
|
||||||
|
const options = vi.mocked(runAgent).mock.calls[0][2];
|
||||||
|
expect(options.provider).toBe('claude');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should work without persona_providers (undefined)', async () => {
|
||||||
|
const movement = makeMovement('plan', {
|
||||||
|
personaDisplayName: 'planner',
|
||||||
|
rules: [makeRule('done', 'COMPLETE')],
|
||||||
|
});
|
||||||
|
const config: PieceConfig = {
|
||||||
|
name: 'no-persona-providers',
|
||||||
|
movements: [movement],
|
||||||
|
initialMovement: 'plan',
|
||||||
|
maxIterations: 1,
|
||||||
|
};
|
||||||
|
|
||||||
|
mockRunAgentSequence([
|
||||||
|
makeResponse({ persona: movement.persona, content: 'done' }),
|
||||||
|
]);
|
||||||
|
mockDetectMatchedRuleSequence([{ index: 0, method: 'phase1_tag' }]);
|
||||||
|
|
||||||
|
const engine = new PieceEngine(config, '/tmp/project', 'test task', {
|
||||||
|
projectCwd: '/tmp/project',
|
||||||
|
provider: 'claude',
|
||||||
|
});
|
||||||
|
|
||||||
|
await engine.run();
|
||||||
|
|
||||||
|
const options = vi.mocked(runAgent).mock.calls[0][2];
|
||||||
|
expect(options.provider).toBe('claude');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should apply different providers to different personas in a multi-movement piece', async () => {
|
||||||
|
const planMovement = makeMovement('plan', {
|
||||||
|
personaDisplayName: 'planner',
|
||||||
|
rules: [makeRule('done', 'implement')],
|
||||||
|
});
|
||||||
|
const implementMovement = makeMovement('implement', {
|
||||||
|
personaDisplayName: 'coder',
|
||||||
|
rules: [makeRule('done', 'COMPLETE')],
|
||||||
|
});
|
||||||
|
const config: PieceConfig = {
|
||||||
|
name: 'multi-persona-providers',
|
||||||
|
movements: [planMovement, implementMovement],
|
||||||
|
initialMovement: 'plan',
|
||||||
|
maxIterations: 3,
|
||||||
|
};
|
||||||
|
|
||||||
|
mockRunAgentSequence([
|
||||||
|
makeResponse({ persona: planMovement.persona, content: 'done' }),
|
||||||
|
makeResponse({ persona: implementMovement.persona, content: 'done' }),
|
||||||
|
]);
|
||||||
|
mockDetectMatchedRuleSequence([
|
||||||
|
{ index: 0, method: 'phase1_tag' },
|
||||||
|
{ index: 0, method: 'phase1_tag' },
|
||||||
|
]);
|
||||||
|
|
||||||
|
const engine = new PieceEngine(config, '/tmp/project', 'test task', {
|
||||||
|
projectCwd: '/tmp/project',
|
||||||
|
provider: 'claude',
|
||||||
|
personaProviders: { coder: 'codex' },
|
||||||
|
});
|
||||||
|
|
||||||
|
await engine.run();
|
||||||
|
|
||||||
|
const calls = vi.mocked(runAgent).mock.calls;
|
||||||
|
// Plan movement: planner not in persona_providers → claude
|
||||||
|
expect(calls[0][2].provider).toBe('claude');
|
||||||
|
// Implement movement: coder in persona_providers → codex
|
||||||
|
expect(calls[1][2].provider).toBe('codex');
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -336,6 +336,63 @@ describe('loadGlobalConfig', () => {
|
|||||||
expect(config.interactivePreviewMovements).toBe(0);
|
expect(config.interactivePreviewMovements).toBe(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('persona_providers', () => {
|
||||||
|
it('should load persona_providers from config.yaml', () => {
|
||||||
|
const taktDir = join(testHomeDir, '.takt');
|
||||||
|
mkdirSync(taktDir, { recursive: true });
|
||||||
|
writeFileSync(
|
||||||
|
getGlobalConfigPath(),
|
||||||
|
[
|
||||||
|
'language: en',
|
||||||
|
'persona_providers:',
|
||||||
|
' coder: codex',
|
||||||
|
' reviewer: claude',
|
||||||
|
].join('\n'),
|
||||||
|
'utf-8',
|
||||||
|
);
|
||||||
|
|
||||||
|
const config = loadGlobalConfig();
|
||||||
|
|
||||||
|
expect(config.personaProviders).toEqual({
|
||||||
|
coder: 'codex',
|
||||||
|
reviewer: 'claude',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should save and reload persona_providers', () => {
|
||||||
|
const taktDir = join(testHomeDir, '.takt');
|
||||||
|
mkdirSync(taktDir, { recursive: true });
|
||||||
|
writeFileSync(getGlobalConfigPath(), 'language: en\n', 'utf-8');
|
||||||
|
|
||||||
|
const config = loadGlobalConfig();
|
||||||
|
config.personaProviders = { coder: 'codex' };
|
||||||
|
saveGlobalConfig(config);
|
||||||
|
invalidateGlobalConfigCache();
|
||||||
|
|
||||||
|
const reloaded = loadGlobalConfig();
|
||||||
|
expect(reloaded.personaProviders).toEqual({ coder: 'codex' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should have undefined personaProviders by default', () => {
|
||||||
|
const config = loadGlobalConfig();
|
||||||
|
expect(config.personaProviders).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not save persona_providers when empty', () => {
|
||||||
|
const taktDir = join(testHomeDir, '.takt');
|
||||||
|
mkdirSync(taktDir, { recursive: true });
|
||||||
|
writeFileSync(getGlobalConfigPath(), 'language: en\n', 'utf-8');
|
||||||
|
|
||||||
|
const config = loadGlobalConfig();
|
||||||
|
config.personaProviders = {};
|
||||||
|
saveGlobalConfig(config);
|
||||||
|
invalidateGlobalConfigCache();
|
||||||
|
|
||||||
|
const reloaded = loadGlobalConfig();
|
||||||
|
expect(reloaded.personaProviders).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('provider/model compatibility validation', () => {
|
describe('provider/model compatibility validation', () => {
|
||||||
it('should throw when provider is codex but model is a Claude alias (opus)', () => {
|
it('should throw when provider is codex but model is a Claude alias (opus)', () => {
|
||||||
const taktDir = join(testHomeDir, '.takt');
|
const taktDir = join(testHomeDir, '.takt');
|
||||||
|
|||||||
@ -61,6 +61,8 @@ export interface GlobalConfig {
|
|||||||
bookmarksFile?: string;
|
bookmarksFile?: string;
|
||||||
/** Path to piece categories file (default: ~/.takt/preferences/piece-categories.yaml) */
|
/** Path to piece categories file (default: ~/.takt/preferences/piece-categories.yaml) */
|
||||||
pieceCategoriesFile?: string;
|
pieceCategoriesFile?: string;
|
||||||
|
/** Per-persona provider overrides (e.g., { coder: 'codex' }) */
|
||||||
|
personaProviders?: Record<string, 'claude' | 'codex' | 'mock'>;
|
||||||
/** Branch name generation strategy: 'romaji' (fast, default) or 'ai' (slow) */
|
/** Branch name generation strategy: 'romaji' (fast, default) or 'ai' (slow) */
|
||||||
branchNameStrategy?: 'romaji' | 'ai';
|
branchNameStrategy?: 'romaji' | 'ai';
|
||||||
/** Prevent macOS idle sleep during takt execution using caffeinate (default: false) */
|
/** Prevent macOS idle sleep during takt execution using caffeinate (default: false) */
|
||||||
|
|||||||
@ -318,6 +318,8 @@ export const GlobalConfigSchema = z.object({
|
|||||||
bookmarks_file: z.string().optional(),
|
bookmarks_file: z.string().optional(),
|
||||||
/** Path to piece categories file (default: ~/.takt/preferences/piece-categories.yaml) */
|
/** Path to piece categories file (default: ~/.takt/preferences/piece-categories.yaml) */
|
||||||
piece_categories_file: z.string().optional(),
|
piece_categories_file: z.string().optional(),
|
||||||
|
/** Per-persona provider overrides (e.g., { coder: 'codex' }) */
|
||||||
|
persona_providers: z.record(z.string(), z.enum(['claude', 'codex', 'mock'])).optional(),
|
||||||
/** Branch name generation strategy: 'romaji' (fast, default) or 'ai' (slow) */
|
/** Branch name generation strategy: 'romaji' (fast, default) or 'ai' (slow) */
|
||||||
branch_name_strategy: z.enum(['romaji', 'ai']).optional(),
|
branch_name_strategy: z.enum(['romaji', 'ai']).optional(),
|
||||||
/** Prevent macOS idle sleep during takt execution using caffeinate (default: false) */
|
/** Prevent macOS idle sleep during takt execution using caffeinate (default: false) */
|
||||||
|
|||||||
@ -34,7 +34,7 @@ export class OptionsBuilder {
|
|||||||
return {
|
return {
|
||||||
cwd: this.getCwd(),
|
cwd: this.getCwd(),
|
||||||
personaPath: step.personaPath,
|
personaPath: step.personaPath,
|
||||||
provider: step.provider ?? this.engineOptions.provider,
|
provider: step.provider ?? this.engineOptions.personaProviders?.[step.personaDisplayName] ?? this.engineOptions.provider,
|
||||||
model: step.model ?? this.engineOptions.model,
|
model: step.model ?? this.engineOptions.model,
|
||||||
permissionMode: step.permissionMode,
|
permissionMode: step.permissionMode,
|
||||||
language: this.getLanguage(),
|
language: this.getLanguage(),
|
||||||
|
|||||||
@ -177,6 +177,8 @@ export interface PieceEngineOptions {
|
|||||||
language?: Language;
|
language?: Language;
|
||||||
provider?: ProviderType;
|
provider?: ProviderType;
|
||||||
model?: string;
|
model?: string;
|
||||||
|
/** Per-persona provider overrides (e.g., { coder: 'codex' }) */
|
||||||
|
personaProviders?: Record<string, ProviderType>;
|
||||||
/** Enable interactive-only rules and user-input transitions */
|
/** Enable interactive-only rules and user-input transitions */
|
||||||
interactive?: boolean;
|
interactive?: boolean;
|
||||||
/** Rule tag index detector (required for rules evaluation) */
|
/** Rule tag index detector (required for rules evaluation) */
|
||||||
|
|||||||
@ -328,6 +328,7 @@ export async function executePiece(
|
|||||||
language: options.language,
|
language: options.language,
|
||||||
provider: options.provider,
|
provider: options.provider,
|
||||||
model: options.model,
|
model: options.model,
|
||||||
|
personaProviders: options.personaProviders,
|
||||||
interactive: interactiveUserInput,
|
interactive: interactiveUserInput,
|
||||||
detectRuleIndex,
|
detectRuleIndex,
|
||||||
callAiJudge,
|
callAiJudge,
|
||||||
|
|||||||
@ -77,6 +77,7 @@ export async function executeTask(options: ExecuteTaskOptions): Promise<boolean>
|
|||||||
language: globalConfig.language,
|
language: globalConfig.language,
|
||||||
provider: agentOverrides?.provider,
|
provider: agentOverrides?.provider,
|
||||||
model: agentOverrides?.model,
|
model: agentOverrides?.model,
|
||||||
|
personaProviders: globalConfig.personaProviders,
|
||||||
interactiveUserInput,
|
interactiveUserInput,
|
||||||
interactiveMetadata,
|
interactiveMetadata,
|
||||||
startMovement,
|
startMovement,
|
||||||
|
|||||||
@ -30,6 +30,8 @@ export interface PieceExecutionOptions {
|
|||||||
language?: Language;
|
language?: Language;
|
||||||
provider?: ProviderType;
|
provider?: ProviderType;
|
||||||
model?: string;
|
model?: string;
|
||||||
|
/** Per-persona provider overrides (e.g., { coder: 'codex' }) */
|
||||||
|
personaProviders?: Record<string, ProviderType>;
|
||||||
/** Enable interactive user input during step transitions */
|
/** Enable interactive user input during step transitions */
|
||||||
interactiveUserInput?: boolean;
|
interactiveUserInput?: boolean;
|
||||||
/** Interactive mode result metadata for NDJSON logging */
|
/** Interactive mode result metadata for NDJSON logging */
|
||||||
|
|||||||
@ -106,6 +106,7 @@ export class GlobalConfigManager {
|
|||||||
minimalOutput: parsed.minimal_output,
|
minimalOutput: parsed.minimal_output,
|
||||||
bookmarksFile: parsed.bookmarks_file,
|
bookmarksFile: parsed.bookmarks_file,
|
||||||
pieceCategoriesFile: parsed.piece_categories_file,
|
pieceCategoriesFile: parsed.piece_categories_file,
|
||||||
|
personaProviders: parsed.persona_providers,
|
||||||
branchNameStrategy: parsed.branch_name_strategy,
|
branchNameStrategy: parsed.branch_name_strategy,
|
||||||
preventSleep: parsed.prevent_sleep,
|
preventSleep: parsed.prevent_sleep,
|
||||||
notificationSound: parsed.notification_sound,
|
notificationSound: parsed.notification_sound,
|
||||||
@ -172,6 +173,9 @@ export class GlobalConfigManager {
|
|||||||
if (config.pieceCategoriesFile) {
|
if (config.pieceCategoriesFile) {
|
||||||
raw.piece_categories_file = config.pieceCategoriesFile;
|
raw.piece_categories_file = config.pieceCategoriesFile;
|
||||||
}
|
}
|
||||||
|
if (config.personaProviders && Object.keys(config.personaProviders).length > 0) {
|
||||||
|
raw.persona_providers = config.personaProviders;
|
||||||
|
}
|
||||||
if (config.branchNameStrategy) {
|
if (config.branchNameStrategy) {
|
||||||
raw.branch_name_strategy = config.branchNameStrategy;
|
raw.branch_name_strategy = config.branchNameStrategy;
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user