From a69e9f4fb333ece9b25642cf9a0595d2b6cb2a09 Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Thu, 5 Mar 2026 23:32:32 +0900 Subject: [PATCH] takt: add-persona-quality-gates (#472) --- src/__tests__/config.test.ts | 346 ++++++++++++++++++ src/__tests__/globalConfig.test.ts | 55 +++ src/__tests__/projectConfig.test.ts | 80 ++++ src/__tests__/qualityGateOverrides.test.ts | 139 ++++++- src/core/models/persisted-global-config.ts | 2 + src/core/models/schemas.ts | 2 + src/infra/config/configNormalizers.ts | 40 +- src/infra/config/global/globalConfigCore.ts | 9 +- src/infra/config/loaders/pieceParser.ts | 20 +- .../config/loaders/qualityGateOverrides.ts | 31 +- src/infra/config/project/projectConfig.ts | 15 +- 11 files changed, 708 insertions(+), 31 deletions(-) diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index ef171ef..fac6de0 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -37,6 +37,7 @@ import { isVerboseMode, resolveConfigValue, invalidateGlobalConfigCache, + invalidateAllResolvedConfigCache, } from '../infra/config/index.js'; let isolatedGlobalConfigDir: string; @@ -270,6 +271,351 @@ describe('loadPiece (builtin fallback)', () => { }); }); +describe('loadPiece piece_overrides.personas integration', () => { + let testDir: string; + + beforeEach(() => { + testDir = join(tmpdir(), `takt-test-${randomUUID()}`); + mkdirSync(join(testDir, '.takt', 'pieces'), { recursive: true }); + }); + + afterEach(() => { + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('should apply persona quality gates from global then project configs', () => { + // Given: global/project persona overrides and piece yaml quality gates + writeFileSync( + join(isolatedGlobalConfigDir, 'config.yaml'), + [ + 'language: en', + 'piece_overrides:', + ' personas:', + ' coder:', + ' quality_gates:', + ' - "Global persona gate"', + ].join('\n'), + 'utf-8', + ); + writeFileSync( + join(testDir, '.takt', 'config.yaml'), + [ + 'piece_overrides:', + ' personas:', + ' coder:', + ' quality_gates:', + ' - "Project persona gate"', + ].join('\n'), + 'utf-8', + ); + writeFileSync( + join(testDir, '.takt', 'pieces', 'persona-gates.yaml'), + [ + 'name: persona-gates', + 'description: Persona quality gates integration test', + 'max_movements: 3', + 'initial_movement: implement', + 'movements:', + ' - name: implement', + ' persona: coder', + ' edit: true', + ' quality_gates:', + ' - "YAML gate"', + ' rules:', + ' - condition: Done', + ' next: COMPLETE', + ' instruction: "{task}"', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + + // When: loading the piece through normal config pipeline + const piece = loadPiece('persona-gates', testDir); + + // Then: persona gates are merged in global -> project -> yaml order + const movement = piece?.movements.find((step) => step.name === 'implement'); + expect(movement?.qualityGates).toEqual([ + 'Global persona gate', + 'Project persona gate', + 'YAML gate', + ]); + }); + + it('should apply persona quality gates when movement persona uses personas section alias key', () => { + // Given: piece persona alias key differs from mapped persona filename + writeFileSync( + join(isolatedGlobalConfigDir, 'config.yaml'), + [ + 'language: en', + 'piece_overrides:', + ' personas:', + ' coder:', + ' quality_gates:', + ' - "Alias key gate"', + ].join('\n'), + 'utf-8', + ); + mkdirSync(join(testDir, '.takt', 'pieces', 'personas'), { recursive: true }); + writeFileSync(join(testDir, '.takt', 'pieces', 'personas', 'implementer.md'), 'Implementer persona', 'utf-8'); + writeFileSync( + join(testDir, '.takt', 'pieces', 'persona-alias-key.yaml'), + [ + 'name: persona-alias-key', + 'description: personas alias key should drive override matching', + 'max_movements: 3', + 'initial_movement: implement', + 'personas:', + ' coder: ./personas/implementer.md', + 'movements:', + ' - name: implement', + ' persona: coder', + ' quality_gates:', + ' - "YAML gate"', + ' rules:', + ' - condition: Done', + ' next: COMPLETE', + ' instruction: "{task}"', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + + // When: loading piece with section alias persona reference + const piece = loadPiece('persona-alias-key', testDir); + + // Then: override key is alias key ("coder"), not mapped filename ("implementer") + const movement = piece?.movements.find((step) => step.name === 'implement'); + expect(movement?.qualityGates).toEqual(['Alias key gate', 'YAML gate']); + }); + + it('should apply persona quality gates for path personas using basename key', () => { + // Given: movement persona is a path and override key uses its basename + writeFileSync( + join(isolatedGlobalConfigDir, 'config.yaml'), + [ + 'language: en', + 'piece_overrides:', + ' personas:', + ' implementer:', + ' quality_gates:', + ' - "Path basename gate"', + ].join('\n'), + 'utf-8', + ); + mkdirSync(join(testDir, '.takt', 'pieces', 'personas'), { recursive: true }); + writeFileSync(join(testDir, '.takt', 'pieces', 'personas', 'implementer.md'), 'Implementer persona', 'utf-8'); + writeFileSync( + join(testDir, '.takt', 'pieces', 'persona-path-key.yaml'), + [ + 'name: persona-path-key', + 'description: path personas should match overrides by basename', + 'max_movements: 3', + 'initial_movement: implement', + 'movements:', + ' - name: implement', + ' persona: ./personas/implementer.md', + ' quality_gates:', + ' - "YAML gate"', + ' rules:', + ' - condition: Done', + ' next: COMPLETE', + ' instruction: "{task}"', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + + // When: loading piece with path-like persona reference + const piece = loadPiece('persona-path-key', testDir); + + // Then: override key resolves from path basename ("implementer") + const movement = piece?.movements.find((step) => step.name === 'implement'); + expect(movement?.qualityGates).toEqual(['Path basename gate', 'YAML gate']); + }); + + it('should not apply persona quality gates when persona does not match', () => { + // Given: persona overrides exist only for reviewer + writeFileSync( + join(isolatedGlobalConfigDir, 'config.yaml'), + [ + 'language: en', + 'piece_overrides:', + ' personas:', + ' reviewer:', + ' quality_gates:', + ' - "Reviewer gate"', + ].join('\n'), + 'utf-8', + ); + writeFileSync( + join(testDir, '.takt', 'pieces', 'persona-mismatch.yaml'), + [ + 'name: persona-mismatch', + 'description: Persona mismatch integration test', + 'max_movements: 3', + 'initial_movement: implement', + 'movements:', + ' - name: implement', + ' persona: coder', + ' quality_gates:', + ' - "YAML gate"', + ' rules:', + ' - condition: Done', + ' next: COMPLETE', + ' instruction: "{task}"', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + + // When: loading piece with different persona + const piece = loadPiece('persona-mismatch', testDir); + + // Then: only YAML gates are applied + const movement = piece?.movements.find((step) => step.name === 'implement'); + expect(movement?.qualityGates).toEqual(['YAML gate']); + }); + + it('should not apply persona quality gates when movement has no persona', () => { + writeFileSync( + join(isolatedGlobalConfigDir, 'config.yaml'), + [ + 'language: en', + 'piece_overrides:', + ' personas:', + ' reviewer:', + ' quality_gates:', + ' - "Reviewer gate"', + ].join('\n'), + 'utf-8', + ); + writeFileSync( + join(testDir, '.takt', 'pieces', 'no-persona-reviewer.yaml'), + [ + 'name: no-persona-reviewer', + 'description: No persona movement should not match persona overrides', + 'max_movements: 3', + 'initial_movement: reviewer', + 'movements:', + ' - name: reviewer', + ' quality_gates:', + ' - "YAML gate"', + ' rules:', + ' - condition: Done', + ' next: COMPLETE', + ' instruction: "{task}"', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + + const piece = loadPiece('no-persona-reviewer', testDir); + + const movement = piece?.movements.find((step) => step.name === 'reviewer'); + expect(movement?.qualityGates).toEqual(['YAML gate']); + }); + + it('should not apply persona quality gates from persona_name without persona', () => { + writeFileSync( + join(isolatedGlobalConfigDir, 'config.yaml'), + [ + 'language: en', + 'piece_overrides:', + ' personas:', + ' reviewer:', + ' quality_gates:', + ' - "Reviewer gate"', + ].join('\n'), + 'utf-8', + ); + writeFileSync( + join(testDir, '.takt', 'pieces', 'persona-name-only.yaml'), + [ + 'name: persona-name-only', + 'description: persona_name should be display-only for persona overrides', + 'max_movements: 3', + 'initial_movement: review', + 'movements:', + ' - name: review', + ' persona_name: reviewer', + ' quality_gates:', + ' - "YAML gate"', + ' rules:', + ' - condition: Done', + ' next: COMPLETE', + ' instruction: "{task}"', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + + const piece = loadPiece('persona-name-only', testDir); + + const movement = piece?.movements.find((step) => step.name === 'review'); + expect(movement?.qualityGates).toEqual(['YAML gate']); + }); + + it('should throw when movement persona is an empty string', () => { + writeFileSync( + join(testDir, '.takt', 'pieces', 'empty-persona.yaml'), + [ + 'name: empty-persona', + 'description: Empty persona should fail fast', + 'max_movements: 3', + 'initial_movement: implement', + 'movements:', + ' - name: implement', + ' persona: " "', + ' rules:', + ' - condition: Done', + ' next: COMPLETE', + ' instruction: "{task}"', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + + expect(() => loadPiece('empty-persona', testDir)).toThrow('Movement "implement" has an empty persona value'); + }); + + it('should throw when movement persona_name is an empty string', () => { + writeFileSync( + join(testDir, '.takt', 'pieces', 'empty-persona-name.yaml'), + [ + 'name: empty-persona-name', + 'description: Empty persona_name should fail fast', + 'max_movements: 3', + 'initial_movement: implement', + 'movements:', + ' - name: implement', + ' persona: coder', + ' persona_name: " "', + ' rules:', + ' - condition: Done', + ' next: COMPLETE', + ' instruction: "{task}"', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + invalidateAllResolvedConfigCache(); + + expect(() => loadPiece('empty-persona-name', testDir)).toThrow('Movement "implement" has an empty persona_name value'); + }); +}); + describe('listPieces (builtin fallback)', () => { let testDir: string; diff --git a/src/__tests__/globalConfig.test.ts b/src/__tests__/globalConfig.test.ts index c4107f8..37016f0 100644 --- a/src/__tests__/globalConfig.test.ts +++ b/src/__tests__/globalConfig.test.ts @@ -117,6 +117,61 @@ piece_overrides: expect(reloaded.pieceOverrides?.qualityGates).toEqual(['Test 1', 'Test 2']); }); + + it('should preserve personas quality_gates in save/load cycle', () => { + const configContent = ` +piece_overrides: + personas: + coder: + quality_gates: + - "Global persona gate" +`; + writeFileSync(testConfigPath, configContent, 'utf-8'); + + const manager = GlobalConfigManager.getInstance(); + const loaded = manager.load(); + const loadedPieceOverrides = loaded.pieceOverrides as unknown as { + personas?: Record; + }; + expect(loadedPieceOverrides.personas?.coder?.qualityGates).toEqual(['Global persona gate']); + + manager.save(loaded); + + GlobalConfigManager.resetInstance(); + const reloadedManager = GlobalConfigManager.getInstance(); + const reloaded = reloadedManager.load(); + const reloadedPieceOverrides = reloaded.pieceOverrides as unknown as { + personas?: Record; + }; + expect(reloadedPieceOverrides.personas?.coder?.qualityGates).toEqual(['Global persona gate']); + }); + + it('should preserve empty quality_gates array in personas', () => { + const configContent = ` +piece_overrides: + personas: + coder: + quality_gates: [] +`; + writeFileSync(testConfigPath, configContent, 'utf-8'); + + const manager = GlobalConfigManager.getInstance(); + const loaded = manager.load(); + const loadedPieceOverrides = loaded.pieceOverrides as unknown as { + personas?: Record; + }; + expect(loadedPieceOverrides.personas?.coder?.qualityGates).toEqual([]); + + manager.save(loaded); + + GlobalConfigManager.resetInstance(); + const reloadedManager = GlobalConfigManager.getInstance(); + const reloaded = reloadedManager.load(); + const reloadedPieceOverrides = reloaded.pieceOverrides as unknown as { + personas?: Record; + }; + expect(reloadedPieceOverrides.personas?.coder?.qualityGates).toEqual([]); + }); }); describe('security hardening', () => { diff --git a/src/__tests__/projectConfig.test.ts b/src/__tests__/projectConfig.test.ts index 27a5964..db65610 100644 --- a/src/__tests__/projectConfig.test.ts +++ b/src/__tests__/projectConfig.test.ts @@ -95,6 +95,57 @@ piece_overrides: expect(reloaded.pieceOverrides?.qualityGates).toEqual(['Test 1', 'Test 2']); }); + + it('should preserve personas quality_gates in save/load cycle', () => { + const configPath = join(testDir, '.takt', 'config.yaml'); + const configContent = ` +piece_overrides: + personas: + coder: + quality_gates: + - "Project persona gate" +`; + writeFileSync(configPath, configContent, 'utf-8'); + + const loaded = loadProjectConfig(testDir); + const loadedPieceOverrides = loaded.pieceOverrides as unknown as { + personas?: Record; + }; + expect(loadedPieceOverrides.personas?.coder?.qualityGates).toEqual(['Project persona gate']); + + saveProjectConfig(testDir, loaded); + + const reloaded = loadProjectConfig(testDir); + const reloadedPieceOverrides = reloaded.pieceOverrides as unknown as { + personas?: Record; + }; + expect(reloadedPieceOverrides.personas?.coder?.qualityGates).toEqual(['Project persona gate']); + }); + + it('should preserve empty quality_gates array in personas', () => { + const configPath = join(testDir, '.takt', 'config.yaml'); + const configContent = ` +piece_overrides: + personas: + coder: + quality_gates: [] +`; + writeFileSync(configPath, configContent, 'utf-8'); + + const loaded = loadProjectConfig(testDir); + const loadedPieceOverrides = loaded.pieceOverrides as unknown as { + personas?: Record; + }; + expect(loadedPieceOverrides.personas?.coder?.qualityGates).toEqual([]); + + saveProjectConfig(testDir, loaded); + + const reloaded = loadProjectConfig(testDir); + const reloadedPieceOverrides = reloaded.pieceOverrides as unknown as { + personas?: Record; + }; + expect(reloadedPieceOverrides.personas?.coder?.qualityGates).toEqual([]); + }); }); describe('migrated project-local fields', () => { @@ -170,6 +221,35 @@ piece_overrides: expect(raw).not.toContain('verbose: false'); }); + it('should not persist empty pipeline object on save', () => { + // Given: empty pipeline object + const config = { + pipeline: {}, + } as ProjectLocalConfig; + + // When: project config is saved + saveProjectConfig(testDir, config); + + // Then: pipeline key is not serialized + const raw = readFileSync(join(testDir, '.takt', 'config.yaml'), 'utf-8'); + expect(raw).not.toContain('pipeline:'); + }); + + it('should not persist empty personaProviders object on save', () => { + // Given: empty personaProviders object + const config = { + personaProviders: {}, + } as ProjectLocalConfig; + + // When: project config is saved + saveProjectConfig(testDir, config); + + // Then: persona_providers key is not serialized + const raw = readFileSync(join(testDir, '.takt', 'config.yaml'), 'utf-8'); + expect(raw).not.toContain('persona_providers:'); + expect(raw).not.toContain('personaProviders:'); + }); + it('should not persist schema-injected default values on save', () => { const loaded = loadProjectConfig(testDir); saveProjectConfig(testDir, loaded); diff --git a/src/__tests__/qualityGateOverrides.test.ts b/src/__tests__/qualityGateOverrides.test.ts index 330b304..41e9576 100644 --- a/src/__tests__/qualityGateOverrides.test.ts +++ b/src/__tests__/qualityGateOverrides.test.ts @@ -6,21 +6,34 @@ import { describe, it, expect } from 'vitest'; import { applyQualityGateOverrides } from '../infra/config/loaders/qualityGateOverrides.js'; import type { PieceOverrides } from '../core/models/persisted-global-config.js'; +type ApplyOverridesArgs = [ + string, + string[] | undefined, + boolean | undefined, + string | undefined, + PieceOverrides | undefined, + PieceOverrides | undefined, +]; + +function applyOverrides(...args: ApplyOverridesArgs): string[] | undefined { + return applyQualityGateOverrides(...args); +} + describe('applyQualityGateOverrides', () => { it('returns undefined when no gates are defined', () => { - const result = applyQualityGateOverrides('implement', undefined, true, undefined, undefined); + const result = applyOverrides('implement', undefined, true, undefined, undefined, undefined); expect(result).toBeUndefined(); }); it('returns YAML gates when no overrides are defined', () => { const yamlGates = ['Test passes']; - const result = applyQualityGateOverrides('implement', yamlGates, true, undefined, undefined); + const result = applyOverrides('implement', yamlGates, true, undefined, undefined, undefined); expect(result).toEqual(['Test passes']); }); it('returns empty array when yamlGates is empty array and no overrides', () => { const yamlGates: string[] = []; - const result = applyQualityGateOverrides('implement', yamlGates, true, undefined, undefined); + const result = applyOverrides('implement', yamlGates, true, undefined, undefined, undefined); expect(result).toEqual([]); }); @@ -29,7 +42,7 @@ describe('applyQualityGateOverrides', () => { const globalOverrides: PieceOverrides = { qualityGates: ['E2E tests pass'], }; - const result = applyQualityGateOverrides('implement', yamlGates, true, undefined, globalOverrides); + const result = applyOverrides('implement', yamlGates, true, undefined, undefined, globalOverrides); expect(result).toEqual(['E2E tests pass', 'Unit tests pass']); }); @@ -43,7 +56,7 @@ describe('applyQualityGateOverrides', () => { }, }, }; - const result = applyQualityGateOverrides('implement', yamlGates, true, undefined, globalOverrides); + const result = applyOverrides('implement', yamlGates, true, undefined, undefined, globalOverrides); expect(result).toEqual(['Global gate', 'Movement-specific gate', 'Unit tests pass']); }); @@ -55,7 +68,7 @@ describe('applyQualityGateOverrides', () => { const projectOverrides: PieceOverrides = { qualityGates: ['Project gate'], }; - const result = applyQualityGateOverrides('implement', yamlGates, true, projectOverrides, globalOverrides); + const result = applyOverrides('implement', yamlGates, true, undefined, projectOverrides, globalOverrides); expect(result).toEqual(['Global gate', 'Project gate', 'YAML gate']); }); @@ -68,7 +81,7 @@ describe('applyQualityGateOverrides', () => { }, }, }; - const result = applyQualityGateOverrides('implement', yamlGates, true, projectOverrides, undefined); + const result = applyOverrides('implement', yamlGates, true, undefined, projectOverrides, undefined); expect(result).toEqual(['Project movement gate', 'YAML gate']); }); @@ -78,7 +91,7 @@ describe('applyQualityGateOverrides', () => { qualityGates: ['Global gate'], qualityGatesEditOnly: true, }; - const result = applyQualityGateOverrides('review', yamlGates, false, undefined, globalOverrides); + const result = applyOverrides('review', yamlGates, false, undefined, undefined, globalOverrides); expect(result).toEqual(['YAML gate']); // Global gate excluded because edit=false }); @@ -88,7 +101,7 @@ describe('applyQualityGateOverrides', () => { qualityGates: ['Global gate'], qualityGatesEditOnly: true, }; - const result = applyQualityGateOverrides('implement', yamlGates, true, undefined, globalOverrides); + const result = applyOverrides('implement', yamlGates, true, undefined, undefined, globalOverrides); expect(result).toEqual(['Global gate', 'YAML gate']); }); @@ -98,7 +111,7 @@ describe('applyQualityGateOverrides', () => { qualityGates: ['Project gate'], qualityGatesEditOnly: true, }; - const result = applyQualityGateOverrides('review', yamlGates, false, projectOverrides, undefined); + const result = applyOverrides('review', yamlGates, false, undefined, projectOverrides, undefined); expect(result).toEqual(['YAML gate']); // Project gate excluded because edit=false }); @@ -113,7 +126,7 @@ describe('applyQualityGateOverrides', () => { }, }, }; - const result = applyQualityGateOverrides('review', yamlGates, false, projectOverrides, undefined); + const result = applyOverrides('review', yamlGates, false, undefined, projectOverrides, undefined); // Project global gate excluded (edit=false), but movement-specific gate included expect(result).toEqual(['Review-specific gate', 'YAML gate']); }); @@ -136,7 +149,7 @@ describe('applyQualityGateOverrides', () => { }, }, }; - const result = applyQualityGateOverrides('implement', yamlGates, true, projectOverrides, globalOverrides); + const result = applyOverrides('implement', yamlGates, true, undefined, projectOverrides, globalOverrides); expect(result).toEqual([ 'Global gate', 'Global movement gate', @@ -155,10 +168,104 @@ describe('applyQualityGateOverrides', () => { }, }, }; - const result = applyQualityGateOverrides('implement', yamlGates, true, projectOverrides, undefined); + const result = applyOverrides('implement', yamlGates, true, undefined, projectOverrides, undefined); expect(result).toEqual(['YAML gate']); // No override for 'implement', only for 'review' }); + describe('persona overrides', () => { + it('applies persona-specific gates from global and project configs in order', () => { + // Given: both global and project configs define gates for the same persona + const yamlGates = ['YAML gate']; + const globalOverrides = { + personas: { + coder: { + qualityGates: ['Global persona gate'], + }, + }, + } as PieceOverrides; + const projectOverrides = { + personas: { + coder: { + qualityGates: ['Project persona gate'], + }, + }, + } as PieceOverrides; + + // When: the movement is executed with the matching persona + const result = applyOverrides('implement', yamlGates, true, 'coder', projectOverrides, globalOverrides); + + // Then: gates are additive with global persona gates before project persona gates + expect(result).toEqual(['Global persona gate', 'Project persona gate', 'YAML gate']); + }); + + it('does not apply persona-specific gates when persona does not match', () => { + // Given: config defines gates for reviewer persona only + const yamlGates = ['YAML gate']; + const projectOverrides = { + personas: { + reviewer: { + qualityGates: ['Reviewer persona gate'], + }, + }, + } as PieceOverrides; + + // When: movement persona is coder + const result = applyOverrides('implement', yamlGates, true, 'coder', projectOverrides, undefined); + + // Then: only YAML gates remain + expect(result).toEqual(['YAML gate']); + }); + + it('deduplicates gates across movement, persona, and YAML sources', () => { + // Given: same gate appears in multiple override layers + const yamlGates = ['Shared gate', 'YAML only']; + const globalOverrides = { + movements: { + implement: { + qualityGates: ['Shared gate', 'Global movement only'], + }, + }, + personas: { + coder: { + qualityGates: ['Shared gate', 'Global persona only'], + }, + }, + } as PieceOverrides; + const projectOverrides = { + personas: { + coder: { + qualityGates: ['Shared gate', 'Project persona only'], + }, + }, + } as PieceOverrides; + + // When: overrides are merged for matching movement + persona + const result = applyOverrides('implement', yamlGates, true, 'coder', projectOverrides, globalOverrides); + + // Then: duplicates are removed, first appearance order is preserved + expect(result).toEqual([ + 'Shared gate', + 'Global movement only', + 'Global persona only', + 'Project persona only', + 'YAML only', + ]); + }); + + it('throws when personaName is empty', () => { + const projectOverrides = { + personas: { + coder: { + qualityGates: ['Project persona gate'], + }, + }, + } as PieceOverrides; + expect(() => + applyOverrides('implement', ['YAML gate'], true, ' ', projectOverrides, undefined) + ).toThrow('Invalid persona name for movement "implement": empty value'); + }); + }); + describe('deduplication', () => { it('removes duplicate gates from multiple sources', () => { const yamlGates = ['Test 1', 'Test 2']; @@ -168,7 +275,7 @@ describe('applyQualityGateOverrides', () => { const projectOverrides: PieceOverrides = { qualityGates: ['Test 1', 'Test 4'], }; - const result = applyQualityGateOverrides('implement', yamlGates, true, projectOverrides, globalOverrides); + const result = applyOverrides('implement', yamlGates, true, undefined, projectOverrides, globalOverrides); // Duplicates removed: Test 1, Test 2 appear only once expect(result).toEqual(['Test 2', 'Test 3', 'Test 1', 'Test 4']); }); @@ -177,7 +284,7 @@ describe('applyQualityGateOverrides', () => { const projectOverrides: PieceOverrides = { qualityGates: ['Test 1', 'Test 2', 'Test 1', 'Test 3', 'Test 2'], }; - const result = applyQualityGateOverrides('implement', undefined, true, projectOverrides, undefined); + const result = applyOverrides('implement', undefined, true, undefined, projectOverrides, undefined); expect(result).toEqual(['Test 1', 'Test 2', 'Test 3']); }); @@ -186,7 +293,7 @@ describe('applyQualityGateOverrides', () => { const projectOverrides: PieceOverrides = { qualityGates: ['npm run test', 'npm run build'], }; - const result = applyQualityGateOverrides('implement', yamlGates, true, projectOverrides, undefined); + const result = applyOverrides('implement', yamlGates, true, undefined, projectOverrides, undefined); // 'npm run test' appears only once expect(result).toEqual(['npm run test', 'npm run build', 'npm run lint']); }); diff --git a/src/core/models/persisted-global-config.ts b/src/core/models/persisted-global-config.ts index 899762d..f4bdfdb 100644 --- a/src/core/models/persisted-global-config.ts +++ b/src/core/models/persisted-global-config.ts @@ -23,6 +23,8 @@ export interface PieceOverrides { qualityGatesEditOnly?: boolean; /** Movement-specific quality gates overrides */ movements?: Record; + /** Persona-specific quality gates overrides */ + personas?: Record; } /** Custom agent configuration */ diff --git a/src/core/models/schemas.ts b/src/core/models/schemas.ts index de505d2..edee62b 100644 --- a/src/core/models/schemas.ts +++ b/src/core/models/schemas.ts @@ -211,6 +211,8 @@ export const PieceOverridesSchema = z.object({ quality_gates_edit_only: z.boolean().optional(), /** Movement-specific quality gates overrides */ movements: z.record(z.string(), MovementQualityGatesOverrideSchema).optional(), + /** Persona-specific quality gates overrides */ + personas: z.record(z.string(), MovementQualityGatesOverrideSchema).optional(), }).optional(); /** Rule-based transition schema (new unified format) */ diff --git a/src/infra/config/configNormalizers.ts b/src/infra/config/configNormalizers.ts index ccff60c..213515e 100644 --- a/src/infra/config/configNormalizers.ts +++ b/src/infra/config/configNormalizers.ts @@ -41,7 +41,12 @@ export function denormalizeProviderProfiles( } export function normalizePieceOverrides( - raw: { quality_gates?: string[]; quality_gates_edit_only?: boolean; movements?: Record } | undefined, + raw: { + quality_gates?: string[]; + quality_gates_edit_only?: boolean; + movements?: Record; + personas?: Record; + } | undefined, ): PieceOverrides | undefined { if (!raw) return undefined; return { @@ -55,14 +60,32 @@ export function normalizePieceOverrides( ]) ) : undefined, + personas: raw.personas + ? Object.fromEntries( + Object.entries(raw.personas).map(([name, override]) => [ + name, + { qualityGates: override.quality_gates }, + ]) + ) + : undefined, }; } export function denormalizePieceOverrides( overrides: PieceOverrides | undefined, -): { quality_gates?: string[]; quality_gates_edit_only?: boolean; movements?: Record } | undefined { +): { + quality_gates?: string[]; + quality_gates_edit_only?: boolean; + movements?: Record; + personas?: Record; +} | undefined { if (!overrides) return undefined; - const result: { quality_gates?: string[]; quality_gates_edit_only?: boolean; movements?: Record } = {}; + const result: { + quality_gates?: string[]; + quality_gates_edit_only?: boolean; + movements?: Record; + personas?: Record; + } = {}; if (overrides.qualityGates !== undefined) { result.quality_gates = overrides.qualityGates; } @@ -80,6 +103,17 @@ export function denormalizePieceOverrides( }) ); } + if (overrides.personas) { + result.personas = Object.fromEntries( + Object.entries(overrides.personas).map(([name, override]) => { + const personaOverride: { quality_gates?: string[] } = {}; + if (override.qualityGates !== undefined) { + personaOverride.quality_gates = override.qualityGates; + } + return [name, personaOverride]; + }) + ); + } return Object.keys(result).length > 0 ? result : undefined; } diff --git a/src/infra/config/global/globalConfigCore.ts b/src/infra/config/global/globalConfigCore.ts index dddc813..41f9fc5 100644 --- a/src/infra/config/global/globalConfigCore.ts +++ b/src/infra/config/global/globalConfigCore.ts @@ -140,7 +140,14 @@ export class GlobalConfigManager { } : undefined, autoFetch: parsed.auto_fetch, baseBranch: parsed.base_branch, - pieceOverrides: normalizePieceOverrides(parsed.piece_overrides as { quality_gates?: string[]; quality_gates_edit_only?: boolean; movements?: Record } | undefined), + pieceOverrides: normalizePieceOverrides( + parsed.piece_overrides as { + quality_gates?: string[]; + quality_gates_edit_only?: boolean; + movements?: Record; + personas?: Record; + } | undefined + ), }; validateProviderModelCompatibility(config.provider, config.model); this.cachedConfig = config; diff --git a/src/infra/config/loaders/pieceParser.ts b/src/infra/config/loaders/pieceParser.ts index 6af17e3..a53eb7a 100644 --- a/src/infra/config/loaders/pieceParser.ts +++ b/src/infra/config/loaders/pieceParser.ts @@ -20,6 +20,7 @@ import { resolveRefList, resolveSectionMap, extractPersonaDisplayName, + isResourcePath, resolvePersona, } from './resource-resolver.js'; @@ -244,10 +245,22 @@ function normalizeStepFromRaw( const rules: PieceRule[] | undefined = step.rules?.map(normalizeRule); const rawPersona = (step as Record).persona as string | undefined; + if (rawPersona !== undefined && rawPersona.trim().length === 0) { + throw new Error(`Movement "${step.name}" has an empty persona value`); + } const { personaSpec, personaPath } = resolvePersona(rawPersona, sections, pieceDir, context); - const displayName: string | undefined = (step as Record).persona_name as string - || undefined; + const displayNameRaw = (step as Record).persona_name as string | undefined; + if (displayNameRaw !== undefined && displayNameRaw.trim().length === 0) { + throw new Error(`Movement "${step.name}" has an empty persona_name value`); + } + const displayName = displayNameRaw || undefined; + const derivedPersonaName = personaSpec ? extractPersonaDisplayName(personaSpec) : undefined; + const resolvedPersonaDisplayName = displayName || derivedPersonaName || step.name; + const normalizedRawPersona = rawPersona?.trim(); + const personaOverrideKey = normalizedRawPersona + ? (isResourcePath(normalizedRawPersona) ? extractPersonaDisplayName(normalizedRawPersona) : normalizedRawPersona) + : undefined; const policyRef = (step as Record).policy as string | string[] | undefined; const policyContents = resolveRefList(policyRef, sections.resolvedPolicies, pieceDir, 'policies', context); @@ -265,7 +278,7 @@ function normalizeStepFromRaw( description: step.description, persona: personaSpec, session: step.session, - personaDisplayName: displayName || (personaSpec ? extractPersonaDisplayName(personaSpec) : step.name), + personaDisplayName: resolvedPersonaDisplayName, personaPath, mcpServers: step.mcp_servers, provider: normalizedProvider.provider ?? inheritedProvider, @@ -282,6 +295,7 @@ function normalizeStepFromRaw( step.name, step.quality_gates, step.edit, + personaOverrideKey, projectOverrides, globalOverrides, ), diff --git a/src/infra/config/loaders/qualityGateOverrides.ts b/src/infra/config/loaders/qualityGateOverrides.ts index e8040b2..3972145 100644 --- a/src/infra/config/loaders/qualityGateOverrides.ts +++ b/src/infra/config/loaders/qualityGateOverrides.ts @@ -17,15 +17,18 @@ import type { PieceOverrides } from '../../../core/models/persisted-global-confi * Merge order (gates are added in this sequence): * 1. Global override in global config (filtered by edit flag if qualityGatesEditOnly=true) * 2. Movement-specific override in global config - * 3. Global override in project config (filtered by edit flag if qualityGatesEditOnly=true) - * 4. Movement-specific override in project config - * 5. Piece YAML quality_gates + * 3. Persona-specific override in global config + * 4. Global override in project config (filtered by edit flag if qualityGatesEditOnly=true) + * 5. Movement-specific override in project config + * 6. Persona-specific override in project config + * 7. Piece YAML quality_gates * * Merge strategy: Additive merge (all gates are combined, no overriding) * * @param movementName - Name of the movement * @param yamlGates - Quality gates from piece YAML * @param editFlag - Whether the movement has edit: true + * @param personaName - Persona name used by the movement * @param projectOverrides - Project-level piece_overrides (from .takt/config.yaml) * @param globalOverrides - Global-level piece_overrides (from ~/.takt/config.yaml) * @returns Merged quality gates array @@ -34,9 +37,15 @@ export function applyQualityGateOverrides( movementName: string, yamlGates: string[] | undefined, editFlag: boolean | undefined, + personaName: string | undefined, projectOverrides: PieceOverrides | undefined, globalOverrides: PieceOverrides | undefined, ): string[] | undefined { + if (personaName !== undefined && personaName.trim().length === 0) { + throw new Error(`Invalid persona name for movement "${movementName}": empty value`); + } + const normalizedPersonaName = personaName?.trim(); + // Track whether yamlGates was explicitly defined (even if empty) const hasYamlGates = yamlGates !== undefined; const gates: string[] = []; @@ -54,6 +63,14 @@ export function applyQualityGateOverrides( gates.push(...globalMovementGates); } + // Collect persona-specific gates from global config + const globalPersonaGates = normalizedPersonaName + ? globalOverrides?.personas?.[normalizedPersonaName]?.qualityGates + : undefined; + if (globalPersonaGates) { + gates.push(...globalPersonaGates); + } + // Collect global gates from project config const projectGlobalGates = projectOverrides?.qualityGates; const projectEditOnly = projectOverrides?.qualityGatesEditOnly ?? false; @@ -67,6 +84,14 @@ export function applyQualityGateOverrides( gates.push(...projectMovementGates); } + // Collect persona-specific gates from project config + const projectPersonaGates = normalizedPersonaName + ? projectOverrides?.personas?.[normalizedPersonaName]?.qualityGates + : undefined; + if (projectPersonaGates) { + gates.push(...projectPersonaGates); + } + // Add YAML gates (lowest priority) if (yamlGates) { gates.push(...yamlGates); diff --git a/src/infra/config/project/projectConfig.ts b/src/infra/config/project/projectConfig.ts index b4f29f9..ef9ac10 100644 --- a/src/infra/config/project/projectConfig.ts +++ b/src/infra/config/project/projectConfig.ts @@ -51,7 +51,6 @@ type RawProviderReference = ConfigProviderReference; */ export function loadProjectConfig(projectDir: string): ProjectLocalConfig { const configPath = getProjectConfigPath(projectDir); - const rawConfig: Record = {}; if (existsSync(configPath)) { const content = readFileSync(configPath, 'utf-8'); @@ -110,7 +109,6 @@ export function loadProjectConfig(projectDir: string): ProjectLocalConfig { model as string | undefined, provider_options as Record | undefined, ); - const normalizedSubmodules = normalizeSubmodules(submodules); const normalizedWithSubmodules = normalizeWithSubmodules(with_submodules); const effectiveWithSubmodules = normalizedSubmodules === undefined ? normalizedWithSubmodules : undefined; @@ -142,7 +140,14 @@ export function loadProjectConfig(projectDir: string): ProjectLocalConfig { 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), + pieceOverrides: normalizePieceOverrides( + piece_overrides as { + quality_gates?: string[]; + quality_gates_edit_only?: boolean; + movements?: Record; + personas?: Record; + } | undefined + ), runtime: normalizeRuntime(runtime), }; } @@ -153,11 +158,9 @@ export function loadProjectConfig(projectDir: string): ProjectLocalConfig { export function saveProjectConfig(projectDir: string, config: ProjectLocalConfig): void { const configDir = getProjectConfigDir(projectDir); const configPath = getProjectConfigPath(projectDir); - if (!existsSync(configDir)) { mkdirSync(configDir, { recursive: true }); } - copyProjectResourcesToDir(configDir); const savePayload: Record = { ...config }; @@ -243,6 +246,8 @@ export function saveProjectConfig(projectDir: string, config: ProjectLocalConfig } if (config.personaProviders && Object.keys(config.personaProviders).length > 0) { savePayload.persona_providers = config.personaProviders; + } else { + delete savePayload.persona_providers; } if (normalizedSubmodules !== undefined) { savePayload.submodules = normalizedSubmodules;