feat: builtinワークフローをパラレルレビュー対応に変更し、エージェントに仕様準拠チェックを追加 (#31)

- default.yamlのreview/security_reviewを統合しparallelステップ(reviewers)に変更
- improve/security_fixステップを統合fixステップに集約
- parallelサブステップのrulesでnextをoptionalに(スキーマ・型定義)
- planner/architecture-reviewer/supervisorに仕様準拠の確認指示を追加(ja/en)
- parallelレビュー構造の検証テストを追加
This commit is contained in:
nrslib 2026-01-30 20:42:54 +09:00
parent 44cb6b14f4
commit 748f5afb29
13 changed files with 471 additions and 354 deletions

View File

@ -327,7 +327,44 @@ function createUser(data: UserData) {
**Always point these out.** Temporary fixes become permanent.
### 8. Quality Attributes
### 8. Spec Compliance Verification
**Verify that changes comply with the project's documented specifications.**
**Verification targets:**
| Target | What to Check |
|--------|---------------|
| CLAUDE.md / README.md | Conforms to schema definitions, design principles, constraints |
| Type definitions / Zod schemas | New fields reflected in schemas |
| YAML/JSON config files | Follows documented format |
| Existing patterns | Consistent with similar files |
**Specific checks:**
1. When config files (YAML, etc.) are modified or added:
- Cross-reference with schema definitions in CLAUDE.md, etc.
- No ignored or invalid fields present
- No required fields missing
2. When type definitions or interfaces are modified:
- Documentation schema descriptions are updated
- Existing config files are compatible with new schema
3. When workflow definitions are modified:
- Correct fields used for step type (normal vs. parallel)
- No unnecessary fields remaining (e.g., `next` on parallel sub-steps)
**REJECT when these patterns are found:**
| Pattern | Problem |
|---------|---------|
| Fields not in the spec | Ignored or unexpected behavior |
| Invalid values per spec | Runtime error or silently ignored |
| Violation of documented constraints | Against design intent |
| Step type / field mismatch | Sign of copy-paste error |
### 9. Quality Attributes
| Attribute | Review Point |
|-----------|--------------|
@ -335,7 +372,7 @@ function createUser(data: UserData) {
| Maintainability | Easy to modify and fix |
| Observability | Logging and monitoring enabled |
### 9. Big Picture
### 10. Big Picture
**Caution**: Don't get lost in minor "clean code" nitpicks.
@ -346,7 +383,7 @@ Verify:
- Does it align with business requirements
- Is naming consistent with the domain
### 10. Change Scope Assessment
### 11. Change Scope Assessment
**Check change scope and include in report (non-blocking).**
@ -365,7 +402,7 @@ Verify:
**Include as suggestions (non-blocking):**
- If splittable, present splitting proposal
### 11. Circular Review Detection
### 12. Circular Review Detection
When review count is provided (e.g., "Review count: 3rd"), adjust judgment accordingly.

View File

@ -46,13 +46,27 @@ Always verify information used in your analysis against the source of truth:
**Don't guess.** Always verify names, values, and behaviors against actual code.
### 4. Implementation Approach
### 4. Spec & Constraint Verification
**Always** verify specifications related to the change target:
| What to Check | How to Check |
|---------------|-------------|
| Project specs (CLAUDE.md, etc.) | Read the file to understand constraints and schemas |
| Type definitions / schemas | Check related type definition files |
| Config file specifications | Check YAML/JSON schemas and existing config examples |
| Existing patterns / conventions | Check how similar files are written |
**Don't plan against the specs.** If specs are unclear, explicitly state so.
### 5. Implementation Approach
Determine the implementation direction:
- What steps to follow
- Points to be careful about
- Items requiring confirmation
- **Spec constraints** (schemas, formats, ignored fields, etc.)
## Important

View File

@ -81,7 +81,18 @@ You are the **human proxy** in the automated workflow. Before approval, verify t
| Production ready | No mock/stub/TODO remaining? |
| Operation | Actually works as expected? |
### 6. Workflow Overall Review
### 6. Spec Compliance Final Check
**Final verification that changes comply with the project's documented specifications.**
Check:
- Changed files are consistent with schemas and constraints documented in CLAUDE.md, etc.
- Config files (YAML, etc.) follow the documented format
- Type definition changes are reflected in documentation
**REJECT if spec violations are found.** Don't assume "probably correct"—actually read and cross-reference the specs.
### 7. Workflow Overall Review
**Check all reports in the report directory and verify overall workflow consistency.**
@ -98,7 +109,7 @@ Check:
| Deviation from original purpose | REJECT - Request return to objective |
| Scope creep | Record only - Address in next task |
### 7. Improvement Suggestion Check
### 8. Improvement Suggestion Check
**Check review reports for unaddressed improvement suggestions.**

View File

@ -1,5 +1,5 @@
# Default TAKT Workflow
# Plan -> Coder -> AI Review -> Architect Review -> Security Review -> Supervisor Approval
# Plan -> Coder -> AI Review -> Reviewers (parallel: Architect + Security) -> Supervisor Approval
#
# Boilerplate sections (Workflow Context, User Request, Previous Response,
# Additional User Inputs, Instructions heading) are auto-injected by buildInstruction().
@ -182,7 +182,7 @@ steps:
- WebFetch
rules:
- condition: No AI-specific issues
next: review
next: reviewers
- condition: AI-specific issues found
next: ai_fix
instruction_template: |
@ -207,7 +207,7 @@ steps:
permission_mode: acceptEdits
rules:
- condition: AI issues fixed
next: review
next: reviewers
- condition: Cannot proceed, insufficient info
next: plan
instruction_template: |
@ -222,7 +222,9 @@ steps:
- Removing scope creep
pass_previous_response: true
- name: review
- name: reviewers
parallel:
- name: arch-review
edit: false
agent: ../agents/default/architecture-reviewer.md
report:
@ -262,77 +264,14 @@ steps:
- WebSearch
- WebFetch
rules:
- condition: No issues found
next: security_review
- condition: Minor improvements needed
next: improve
- condition: Structural fix required
next: fix
- condition: approved
- condition: needs_fix
instruction_template: |
Focus on **architecture and design** review. Do NOT review AI-specific issues (that's the ai_review step).
Review the changes and provide feedback.
- name: improve
edit: true
agent: ../agents/default/coder.md
allowed_tools:
- Read
- Glob
- Grep
- Edit
- Write
- Bash
- WebSearch
- WebFetch
permission_mode: acceptEdits
rules:
- condition: Improvements complete
next: review
- condition: Cannot proceed, insufficient info
next: plan
instruction_template: |
## Architect Feedback (This is the latest instruction - prioritize this)
{previous_response}
**Important**: Address the Architect's improvement suggestions.
These are minor improvements, not major design issues.
Make improvements such as:
- Naming improvements
- Small refactoring
- Adding/fixing comments
- Code organization
pass_previous_response: true
- name: fix
edit: true
agent: ../agents/default/coder.md
allowed_tools:
- Read
- Glob
- Grep
- Edit
- Write
- Bash
- WebSearch
- WebFetch
permission_mode: acceptEdits
rules:
- condition: Architect feedback addressed
next: review
- condition: Cannot proceed, insufficient info
next: plan
instruction_template: |
## Architect Feedback (This is the latest instruction - prioritize this)
{previous_response}
**Important**: Address the Architect's feedback.
The "Original User Request" is reference information, not the latest instruction.
Review the session conversation history and fix the issues raised by the Architect.
pass_previous_response: true
- name: security_review
- name: security-review
edit: false
agent: ../agents/default/security-reviewer.md
report:
@ -374,18 +313,21 @@ steps:
- WebSearch
- WebFetch
rules:
- condition: No security issues
next: supervise
- condition: Vulnerabilities require fix
next: security_fix
- condition: approved
- condition: needs_fix
instruction_template: |
Perform security review on the changes. Check for vulnerabilities including:
- Injection attacks (SQL, Command, XSS)
- Authentication/Authorization issues
- Data exposure risks
- Cryptographic weaknesses
rules:
- condition: all("approved")
next: supervise
- condition: any("needs_fix")
next: fix
- name: security_fix
- name: fix
edit: true
agent: ../agents/default/coder.md
allowed_tools:
@ -399,16 +341,17 @@ steps:
- WebFetch
permission_mode: acceptEdits
rules:
- condition: Security fix complete
next: security_review
- condition: Fix complete
next: reviewers
- condition: Cannot proceed, insufficient info
next: plan
instruction_template: |
## Security Review Feedback (This is the latest instruction - prioritize this)
## Review Feedback (This is the latest instruction - prioritize this)
{previous_response}
**Important**: Fix the vulnerabilities identified in the security review.
Security issues require highest priority.
**Important**: Address the feedback from the reviewers.
The "Original User Request" is reference information, not the latest instruction.
Review the session conversation history and fix the issues raised by the reviewers.
pass_previous_response: true
- name: supervise

View File

@ -447,7 +447,44 @@ function createOrder(data: OrderData) {
- 3回の重複 → 即抽出
- ドメインが異なる重複 → 抽象化しない(例: 顧客用バリデーションと管理者用バリデーションは別物)
### 8. 呼び出しチェーン検証
### 8. 仕様準拠の検証
**変更が、プロジェクトの文書化された仕様に準拠しているか検証する。**
**検証対象:**
| 対象 | 確認内容 |
|------|---------|
| CLAUDE.md / README.md | スキーマ定義、設計原則、制約に従っているか |
| 型定義・Zodスキーマ | 新しいフィールドがスキーマに反映されているか |
| YAML/JSON設定ファイル | 文書化されたフォーマットに従っているか |
| 既存パターン | 同種のファイルと一貫性があるか |
**具体的なチェック:**
1. 設定ファイルYAML等を変更・追加した場合:
- CLAUDE.md等に記載されたスキーマ定義と突合する
- 無視されるフィールドや無効なフィールドが含まれていないか
- 必須フィールドが欠落していないか
2. 型定義やインターフェースを変更した場合:
- ドキュメントのスキーマ説明が更新されているか
- 既存の設定ファイルが新しいスキーマと整合するか
3. ワークフロー定義を変更した場合:
- ステップ種別(通常/parallelに応じた正しいフィールドが使われているか
- 不要なフィールドparallelサブステップのnext等が残っていないか
**このパターンを見つけたら REJECT:**
| パターン | 問題 |
|---------|------|
| 仕様に存在しないフィールドの使用 | 無視されるか予期しない動作 |
| 仕様上無効な値の設定 | 実行時エラーまたは無視される |
| 文書化された制約への違反 | 設計意図に反する |
| ステップ種別とフィールドの不整合 | コピペミスの兆候 |
### 9. 呼び出しチェーン検証
**新しいパラメータ・フィールドが追加された場合、変更ファイル内だけでなく呼び出し元も検証する。**
@ -480,7 +517,7 @@ export async function executeWorkflow(config, cwd, task, options?) {
**このパターンを見つけたら REJECT。** 個々のファイルが正しくても、結合されていなければ機能しない。
### 9. 品質特性
### 10. 品質特性
| 特性 | 確認観点 |
|------|---------|
@ -488,7 +525,7 @@ export async function executeWorkflow(config, cwd, task, options?) {
| Maintainability | 変更・修正が容易か |
| Observability | ログ・監視が可能な設計か |
### 10. 大局観
### 11. 大局観
**注意**: 細かい「クリーンコード」の指摘に終始しない。
@ -499,7 +536,7 @@ export async function executeWorkflow(config, cwd, task, options?) {
- ビジネス要件と整合しているか
- 命名がドメインと一貫しているか
### 11. 変更スコープの評価
### 12. 変更スコープの評価
**変更スコープを確認し、レポートに記載する(ブロッキングではない)。**
@ -518,7 +555,7 @@ export async function executeWorkflow(config, cwd, task, options?) {
**提案として記載すること(ブロッキングではない):**
- 分割可能な場合は分割案を提示
### 12. 堂々巡りの検出
### 13. 堂々巡りの検出
レビュー回数が渡される場合(例: 「レビュー回数: 3回目」、回数に応じて判断を変える。

View File

@ -46,13 +46,27 @@
**推測で書かない。** 名前・値・振る舞いは必ずコードで確認する。
### 4. 実装アプローチ
### 4. 仕様・制約の確認
変更対象に関連する仕様を**必ず**確認する:
| 確認すべきもの | 確認方法 |
|--------------|---------|
| プロジェクト仕様CLAUDE.md等 | ファイルを読んで制約・スキーマを把握 |
| 型定義・スキーマ | 関連する型定義ファイルを確認 |
| 設定ファイルの仕様 | YAML/JSONスキーマや既存の設定例を確認 |
| 既存のパターン・規約 | 同種のファイルがどう書かれているか確認 |
**仕様に反する計画は立てない。** 仕様が不明確な場合はその旨を明記する。
### 5. 実装アプローチ
実装の方向性を決める:
- どのような手順で進めるか
- 注意すべき点
- 確認が必要な点
- **仕様上の制約**(スキーマ、フォーマット、無視されるフィールド等)
## 重要

View File

@ -81,7 +81,18 @@ Architectが「正しく作られているかVerification」を確認す
| 本番Ready | モック・スタブ・TODO が残っていないか |
| 動作 | 実際に期待通り動くか |
### 6. ワークフロー全体の見直し
### 6. 仕様準拠の最終確認
**変更が、プロジェクトの文書化された仕様に準拠しているか最終確認する。**
確認すること:
- CLAUDE.md等に記載されたスキーマ・制約と、変更されたファイルが整合しているか
- 設定ファイルYAML等が文書化されたフォーマットに従っているか
- 型定義の変更がドキュメントにも反映されているか
**仕様違反を見つけたら REJECT。** 仕様は「たぶん合ってる」ではなく、実際に読んで突合する。
### 7. ワークフロー全体の見直し
**レポートディレクトリ内の全レポートを確認し、ワークフロー全体の整合性をチェックする。**
@ -98,7 +109,7 @@ Architectが「正しく作られているかVerification」を確認す
| 本来の目的から逸脱 | REJECT - 目的に立ち返るよう指示 |
| スコープクリープ | 記録のみ - 次回タスクで対応 |
### 7. 改善提案の確認
### 8. 改善提案の確認
**レビューレポートを確認し、未対応の改善提案がないかチェックする。**

View File

@ -1,5 +1,5 @@
# Default TAKT Workflow
# Plan -> Implement -> AI Review -> Architect Review -> Security Review -> Supervisor Approval
# Plan -> Implement -> AI Review -> Reviewers (parallel: Architect + Security) -> Supervisor Approval
#
# Template Variables (auto-injected by buildInstruction):
# {iteration} - Workflow-wide turn count (total steps executed across all agents)
@ -178,7 +178,7 @@ steps:
- WebFetch
rules:
- condition: AI特有の問題なし
next: review
next: reviewers
- condition: AI特有の問題あり
next: ai_fix
instruction_template: |
@ -203,7 +203,7 @@ steps:
permission_mode: acceptEdits
rules:
- condition: AI問題の修正完了
next: review
next: reviewers
- condition: 判断できない、情報不足
next: plan
instruction_template: |
@ -218,7 +218,9 @@ steps:
- スコープクリープの除去
pass_previous_response: true
- name: review
- name: reviewers
parallel:
- name: arch-review
edit: false
agent: ../agents/default/architecture-reviewer.md
report:
@ -261,14 +263,10 @@ steps:
- WebSearch
- WebFetch
rules:
- condition: 問題なし
next: security_review
- condition: 改善すべき点がある(軽微)
next: improve
- condition: 構造的な修正が必要
next: fix
- condition: approved
- condition: needs_fix
instruction_template: |
**アーキテクチャと設計**のレビューに集中してください。AI特有の問題はレビューしないでください次のステップで行います)。
**アーキテクチャと設計**のレビューに集中してください。AI特有の問題はレビューしないでくださいai_reviewステップで行います
変更をレビューしてフィードバックを提供してください。
@ -280,65 +278,7 @@ steps:
- デッドコード
- 呼び出しチェーン検証
- name: improve
edit: true
agent: ../agents/default/coder.md
allowed_tools:
- Read
- Glob
- Grep
- Edit
- Write
- Bash
- WebSearch
- WebFetch
permission_mode: acceptEdits
rules:
- condition: 改善完了
next: review
- condition: 判断できない、情報不足
next: plan
instruction_template: |
## Architect Feedback (これが最新の指示です - 優先して対応してください)
{previous_response}
**重要**: Architectの改善提案に対応してください。
これらは軽微な改善であり、設計上の大きな問題ではありません。
以下のような改善を行ってください:
- 命名の改善
- 小さなリファクタリング
- コメントの追加・修正
- コードの整理
pass_previous_response: true
- name: fix
edit: true
agent: ../agents/default/coder.md
allowed_tools:
- Read
- Glob
- Grep
- Edit
- Write
- Bash
- WebSearch
- WebFetch
permission_mode: acceptEdits
rules:
- condition: Architectの指摘を修正完了
next: review
- condition: 判断できない、情報不足
next: plan
instruction_template: |
## Architect Feedback (これが最新の指示です - 優先して対応してください)
{previous_response}
**重要**: Architectのフィードバックに対応してください。
セッションの会話履歴を確認し、Architectの指摘事項を修正してください。
pass_previous_response: true
- name: security_review
- name: security-review
edit: false
agent: ../agents/default/security-reviewer.md
report:
@ -380,18 +320,21 @@ steps:
- WebSearch
- WebFetch
rules:
- condition: セキュリティ問題なし
next: supervise
- condition: 脆弱性があり修正が必要
next: security_fix
- condition: approved
- condition: needs_fix
instruction_template: |
変更に対してセキュリティレビューを行ってください。以下の脆弱性を確認してください:
- インジェクション攻撃SQL, コマンド, XSS
- 認証・認可の問題
- データ露出リスク
- 暗号化の弱点
rules:
- condition: all("approved")
next: supervise
- condition: any("needs_fix")
next: fix
- name: security_fix
- name: fix
edit: true
agent: ../agents/default/coder.md
allowed_tools:
@ -405,16 +348,16 @@ steps:
- WebFetch
permission_mode: acceptEdits
rules:
- condition: セキュリティ修正完了
next: security_review
- condition: 修正完了
next: reviewers
- condition: 判断できない、情報不足
next: plan
instruction_template: |
## Security Review Feedback (これが最新の指示です - 優先して対応してください)
## Review Feedback (これが最新の指示です - 優先して対応してください)
{previous_response}
**重要**: セキュリティレビューで指摘された脆弱性を修正してください。
キュリティの問題は最優先で対応が必要です
**重要**: レビュアーのフィードバックに対応してください。
ッションの会話履歴を確認し、レビュアーの指摘事項を修正してください
pass_previous_response: true
- name: supervise

View File

@ -48,6 +48,113 @@ describe('getBuiltinWorkflow', () => {
});
});
describe('default workflow parallel reviewers step', () => {
it('should have a reviewers step with parallel sub-steps', () => {
const workflow = getBuiltinWorkflow('default');
expect(workflow).not.toBeNull();
const reviewersStep = workflow!.steps.find((s) => s.name === 'reviewers');
expect(reviewersStep).toBeDefined();
expect(reviewersStep!.parallel).toBeDefined();
expect(reviewersStep!.parallel).toHaveLength(2);
});
it('should have arch-review and security-review as parallel sub-steps', () => {
const workflow = getBuiltinWorkflow('default');
const reviewersStep = workflow!.steps.find((s) => s.name === 'reviewers')!;
const subStepNames = reviewersStep.parallel!.map((s) => s.name);
expect(subStepNames).toContain('arch-review');
expect(subStepNames).toContain('security-review');
});
it('should have aggregate conditions on the reviewers parent step', () => {
const workflow = getBuiltinWorkflow('default');
const reviewersStep = workflow!.steps.find((s) => s.name === 'reviewers')!;
expect(reviewersStep.rules).toBeDefined();
expect(reviewersStep.rules).toHaveLength(2);
const allRule = reviewersStep.rules!.find((r) => r.isAggregateCondition && r.aggregateType === 'all');
expect(allRule).toBeDefined();
expect(allRule!.aggregateConditionText).toBe('approved');
expect(allRule!.next).toBe('supervise');
const anyRule = reviewersStep.rules!.find((r) => r.isAggregateCondition && r.aggregateType === 'any');
expect(anyRule).toBeDefined();
expect(anyRule!.aggregateConditionText).toBe('needs_fix');
expect(anyRule!.next).toBe('fix');
});
it('should have matching conditions on sub-steps for aggregation', () => {
const workflow = getBuiltinWorkflow('default');
const reviewersStep = workflow!.steps.find((s) => s.name === 'reviewers')!;
for (const subStep of reviewersStep.parallel!) {
expect(subStep.rules).toBeDefined();
const conditions = subStep.rules!.map((r) => r.condition);
expect(conditions).toContain('approved');
expect(conditions).toContain('needs_fix');
}
});
it('should have ai_review transitioning to reviewers step', () => {
const workflow = getBuiltinWorkflow('default');
const aiReviewStep = workflow!.steps.find((s) => s.name === 'ai_review')!;
const approveRule = aiReviewStep.rules!.find((r) => r.next === 'reviewers');
expect(approveRule).toBeDefined();
});
it('should have ai_fix transitioning to reviewers step', () => {
const workflow = getBuiltinWorkflow('default');
const aiFixStep = workflow!.steps.find((s) => s.name === 'ai_fix')!;
const fixedRule = aiFixStep.rules!.find((r) => r.next === 'reviewers');
expect(fixedRule).toBeDefined();
});
it('should have fix step transitioning back to reviewers', () => {
const workflow = getBuiltinWorkflow('default');
const fixStep = workflow!.steps.find((s) => s.name === 'fix')!;
const fixedRule = fixStep.rules!.find((r) => r.next === 'reviewers');
expect(fixedRule).toBeDefined();
});
it('should not have old separate review/security_review/improve steps', () => {
const workflow = getBuiltinWorkflow('default');
const stepNames = workflow!.steps.map((s) => s.name);
expect(stepNames).not.toContain('review');
expect(stepNames).not.toContain('security_review');
expect(stepNames).not.toContain('improve');
expect(stepNames).not.toContain('security_fix');
});
it('should have sub-steps with correct agents', () => {
const workflow = getBuiltinWorkflow('default');
const reviewersStep = workflow!.steps.find((s) => s.name === 'reviewers')!;
const archReview = reviewersStep.parallel!.find((s) => s.name === 'arch-review')!;
expect(archReview.agent).toContain('architecture-reviewer');
const secReview = reviewersStep.parallel!.find((s) => s.name === 'security-review')!;
expect(secReview.agent).toContain('security-reviewer');
});
it('should have reports configured on sub-steps', () => {
const workflow = getBuiltinWorkflow('default');
const reviewersStep = workflow!.steps.find((s) => s.name === 'reviewers')!;
const archReview = reviewersStep.parallel!.find((s) => s.name === 'arch-review')!;
expect(archReview.report).toBeDefined();
const secReview = reviewersStep.parallel!.find((s) => s.name === 'security-review')!;
expect(secReview.report).toBeDefined();
});
});
describe('loadAllWorkflows', () => {
let testDir: string;

View File

@ -74,8 +74,8 @@ export const ReportFieldSchema = z.union([
export const WorkflowRuleSchema = z.object({
/** Human-readable condition text */
condition: z.string().min(1),
/** Next step name (e.g., implement, COMPLETE, ABORT) */
next: z.string().min(1),
/** Next step name (e.g., implement, COMPLETE, ABORT). Optional for parallel sub-steps (parent handles routing). */
next: z.string().min(1).optional(),
/** Template for additional AI output */
appendix: z.string().optional(),
});

View File

@ -53,8 +53,8 @@ export interface SessionState {
export interface WorkflowRule {
/** Human-readable condition text */
condition: string;
/** Next step name (e.g., implement, COMPLETE, ABORT) */
next: string;
/** Next step name (e.g., implement, COMPLETE, ABORT). Optional for parallel sub-steps. */
next?: string;
/** Template for additional AI output */
appendix?: string;
/** Whether this condition uses ai() expression (set by loader) */

View File

@ -110,7 +110,7 @@ export class WorkflowEngine extends EventEmitter {
for (const step of this.config.steps) {
if (step.rules) {
for (const rule of step.rules) {
if (!stepNames.has(rule.next)) {
if (rule.next && !stepNames.has(rule.next)) {
throw new Error(
`Invalid rule in step "${step.name}": target step "${rule.next}" does not exist`
);

View File

@ -20,7 +20,7 @@ export function determineNextStepByRules(
if (!rule) {
return null;
}
return rule.next;
return rule.next ?? null;
}
/**