From ccc19e83ff82cba72a8988ee425e0c2028c7e050 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Tue, 27 Jan 2026 11:50:07 +0900 Subject: [PATCH] =?UTF-8?q?mock=E5=AE=9F=E8=A1=8C=E3=82=92=E3=81=A7?= =?UTF-8?q?=E3=81=8D=E3=82=8B=E3=82=88=E3=81=86=E3=81=AB=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../global/en/agents/default/architect.md | 73 +++- resources/global/en/agents/default/coder.md | 156 +++++++- .../global/en/workflows/expert-review.yaml | 2 +- .../global/ja/agents/default/architect.md | 63 +++- resources/global/ja/agents/default/coder.md | 158 +++++++- .../agents/expert-review/cqrs-es-reviewer.md | 336 +++++++++++++++++- .../agents/expert-review/frontend-reviewer.md | 336 ++++++++++++++++-- .../global/ja/workflows/expert-review.yaml | 2 +- src/agents/runner.ts | 66 ++-- src/mock/client.ts | 85 +++++ src/models/schemas.ts | 8 +- src/models/types.ts | 8 +- src/providers/claude.ts | 43 +++ src/providers/codex.ts | 35 ++ src/providers/index.ts | 63 ++++ src/providers/mock.ts | 30 ++ 16 files changed, 1357 insertions(+), 107 deletions(-) create mode 100644 src/mock/client.ts create mode 100644 src/providers/claude.ts create mode 100644 src/providers/codex.ts create mode 100644 src/providers/index.ts create mode 100644 src/providers/mock.ts diff --git a/resources/global/en/agents/default/architect.md b/resources/global/en/agents/default/architect.md index 48399f2..7c97d20 100644 --- a/resources/global/en/agents/default/architect.md +++ b/resources/global/en/agents/default/architect.md @@ -159,7 +159,68 @@ Prohibited patterns: | Hidden Dependencies | Child components implicitly calling APIs etc. | | Non-idiomatic | Custom implementation ignoring language/FW conventions | -### 6. Unnecessary Backward Compatibility Code Detection +### 6. Abstraction Level Evaluation + +**Conditional Branch Proliferation Detection:** + +| Pattern | Judgment | +|---------|----------| +| Same if-else pattern in 3+ places | Abstract with polymorphism → **REJECT** | +| switch/case with 5+ branches | Consider Strategy/Map pattern | +| Flag arguments changing behavior | Split into separate functions → **REJECT** | +| Type-based branching (instanceof/typeof) | Replace with polymorphism → **REJECT** | +| Nested conditionals (3+ levels) | Early return or extract → **REJECT** | + +**Abstraction Level Mismatch Detection:** + +| Pattern | Problem | Fix | +|---------|---------|-----| +| Low-level details in high-level processing | Hard to read | Extract details to functions | +| Mixed abstraction levels in one function | Cognitive load | Align to same granularity | +| DB operations mixed with business logic | Responsibility violation | Separate to Repository layer | +| Config values mixed with processing logic | Hard to change | Externalize configuration | + +**Good Abstraction Examples:** + +```typescript +// ❌ Proliferating conditionals +function process(type: string) { + if (type === 'A') { /* process A */ } + else if (type === 'B') { /* process B */ } + else if (type === 'C') { /* process C */ } + // ...continues +} + +// ✅ Abstract with Map pattern +const processors: Record void> = { + A: processA, + B: processB, + C: processC, +}; +function process(type: string) { + processors[type]?.(); +} +``` + +```typescript +// ❌ Mixed abstraction levels +function createUser(data: UserData) { + // High level: business logic + validateUser(data); + // Low level: DB operation details + const conn = await pool.getConnection(); + await conn.query('INSERT INTO users...'); + conn.release(); +} + +// ✅ Aligned abstraction levels +function createUser(data: UserData) { + validateUser(data); + await userRepository.save(data); // Details hidden +} +``` + +### 7. Unnecessary Backward Compatibility Code Detection **AI tends to leave unnecessary code "for backward compatibility." Don't overlook this.** @@ -188,7 +249,7 @@ Code that should be kept: **Be suspicious when AI says "for backward compatibility."** Verify if it's really needed. -### 7. Workaround Detection +### 8. Workaround Detection **Don't overlook compromises made to "just make it work."** @@ -203,7 +264,7 @@ Code that should be kept: **Always point these out.** Temporary fixes become permanent. -### 8. Quality Attributes +### 9. Quality Attributes | Attribute | Review Point | |-----------|--------------| @@ -211,7 +272,7 @@ Code that should be kept: | 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. @@ -222,7 +283,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).** @@ -241,7 +302,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. diff --git a/resources/global/en/agents/default/coder.md b/resources/global/en/agents/default/coder.md index a905df2..4cb53b8 100644 --- a/resources/global/en/agents/default/coder.md +++ b/resources/global/en/agents/default/coder.md @@ -105,7 +105,45 @@ Perform self-check after implementation. | Boy Scout | Leave touched areas slightly improved | | Fail Fast | Detect errors early. Don't swallow them | -**When in doubt**: Choose Simple. Abstraction can come later. +**When in doubt**: Choose Simple. + +## Abstraction Principles + +**Before adding conditional branches, consider:** +- Does this condition exist elsewhere? → Abstract with a pattern +- Will more branches be added? → Use Strategy/Map pattern +- Branching on type? → Replace with polymorphism + +```typescript +// ❌ Adding more conditionals +if (type === 'A') { ... } +else if (type === 'B') { ... } +else if (type === 'C') { ... } // Yet another one + +// ✅ Abstract with Map +const handlers = { A: handleA, B: handleB, C: handleC }; +handlers[type]?.(); +``` + +**Align abstraction levels:** +- Keep same granularity of operations within one function +- Extract detailed processing to separate functions +- Don't mix "what to do" with "how to do it" + +```typescript +// ❌ Mixed abstraction levels +function processOrder(order) { + validateOrder(order); // High level + const conn = pool.getConnection(); // Low level detail + conn.query('INSERT...'); // Low level detail +} + +// ✅ Aligned abstraction levels +function processOrder(order) { + validateOrder(order); + saveOrder(order); // Details hidden +} +``` **Follow language/framework conventions:** - Be Pythonic in Python, Kotlin-like in Kotlin @@ -134,6 +172,121 @@ Perform self-check after implementation. - Children don't modify state directly (notify parent via events) - State flows in one direction +## Error Handling + +**Principle: Centralize error handling. Don't scatter try-catch everywhere.** + +```typescript +// ❌ Try-catch everywhere +async function createUser(data) { + try { + const user = await userService.create(data) + return user + } catch (e) { + console.error(e) + throw new Error('Failed to create user') + } +} + +// ✅ Centralized handling at upper layer +// Catch at Controller/Handler layer +// Or use @ControllerAdvice / ErrorBoundary +async function createUser(data) { + return await userService.create(data) // Let exceptions propagate +} +``` + +**Error handling placement:** + +| Layer | Responsibility | +|-------|----------------| +| Domain/Service layer | Throw exceptions on business rule violations | +| Controller/Handler layer | Catch exceptions and convert to response | +| Global handler | Handle common exceptions (NotFound, auth errors, etc.) | + +## Transformation Placement + +**Principle: Put conversion methods on DTOs.** + +```typescript +// ✅ Request/Response DTOs have conversion methods +interface CreateUserRequest { + name: string + email: string +} + +function toUseCaseInput(req: CreateUserRequest): CreateUserInput { + return { name: req.name, email: req.email } +} + +// Controller +const input = toUseCaseInput(request) +const output = await useCase.execute(input) +return UserResponse.from(output) +``` + +**Conversion direction:** +``` +Request → toInput() → UseCase/Service → Output → Response.from() +``` + +## Extraction Decisions + +**Rule of Three:** +- 1st time: Write it inline +- 2nd time: Don't extract yet (wait and see) +- 3rd time: Consider extraction + +**Should extract:** +- Same logic in 3+ places +- Same style/UI pattern +- Same validation logic +- Same formatting logic + +**Should NOT extract:** +- Similar but slightly different (forced generalization adds complexity) +- Used in only 1-2 places +- Based on "might use later" predictions + +```typescript +// ❌ Over-generalization +function formatValue(value, type, options) { + if (type === 'currency') { ... } + else if (type === 'date') { ... } + else if (type === 'percentage') { ... } +} + +// ✅ Separate functions by purpose +function formatCurrency(amount: number): string { ... } +function formatDate(date: Date): string { ... } +function formatPercentage(value: number): string { ... } +``` + +## Writing Tests + +**Principle: Structure tests with "Given-When-Then".** + +```typescript +test('returns NotFound error when user does not exist', async () => { + // Given: non-existent user ID + const nonExistentId = 'non-existent-id' + + // When: attempt to get user + const result = await getUser(nonExistentId) + + // Then: NotFound error is returned + expect(result.error).toBe('NOT_FOUND') +}) +``` + +**Test priority:** + +| Priority | Target | +|----------|--------| +| High | Business logic, state transitions | +| Medium | Edge cases, error handling | +| Low | Simple CRUD, UI appearance | + ## Prohibited - **Fallback value overuse** - Don't hide problems with `?? 'unknown'`, `|| 'default'` @@ -143,4 +296,5 @@ Perform self-check after implementation. - **Direct object/array mutation** - Create new with spread operator - **console.log** - Don't leave in production code - **Hardcoded secrets** +- **Scattered try-catch** - Centralize error handling at upper layer diff --git a/resources/global/en/workflows/expert-review.yaml b/resources/global/en/workflows/expert-review.yaml index 12f0c42..1708b62 100644 --- a/resources/global/en/workflows/expert-review.yaml +++ b/resources/global/en/workflows/expert-review.yaml @@ -631,7 +631,7 @@ steps: pass_previous_response: true transitions: - condition: done - next_step: cqrs_es_review + next_step: ai_review - condition: blocked next_step: plan diff --git a/resources/global/ja/agents/default/architect.md b/resources/global/ja/agents/default/architect.md index e057b17..42c6860 100644 --- a/resources/global/ja/agents/default/architect.md +++ b/resources/global/ja/agents/default/architect.md @@ -159,7 +159,68 @@ Vertical Slice の判定基準: | 隠れた依存 | 子コンポーネントが暗黙的にAPIを呼ぶ等 | | 非イディオマティック | 言語・FWの作法を無視した独自実装 | -### 6. 不要な後方互換コードの検出 +### 6. 抽象化レベルの評価 + +**条件分岐の肥大化検出:** + +| パターン | 判定 | +|---------|------| +| 同じif-elseパターンが3箇所以上 | ポリモーフィズムで抽象化 → **REJECT** | +| switch/caseが5分岐以上 | Strategy/Mapパターンを検討 | +| フラグ引数で挙動を変える | 別関数に分割 → **REJECT** | +| 型による分岐（instanceof/typeof） | ポリモーフィズムに置換 → **REJECT** | +| ネストした条件分岐（3段以上） | 早期リターンまたは抽出 → **REJECT** | + +**抽象度の不一致検出:** + +| パターン | 問題 | 修正案 | +|---------|------|--------| +| 高レベル処理の中に低レベル詳細 | 読みにくい | 詳細を関数に抽出 | +| 1関数内で抽象度が混在 | 認知負荷 | 同じ粒度に揃える | +| ビジネスロジックにDB操作が混在 | 責務違反 | Repository層に分離 | +| 設定値と処理ロジックが混在 | 変更困難 | 設定を外部化 | + +**良い抽象化の例:** + +```typescript +// ❌ 条件分岐の肥大化 +function process(type: string) { + if (type === 'A') { /* 処理A */ } + else if (type === 'B') { /* 処理B */ } + else if (type === 'C') { /* 処理C */ } + // ...続く +} + +// ✅ Mapパターンで抽象化 +const processors: Record void> = { + A: processA, + B: processB, + C: processC, +}; +function process(type: string) { + processors[type]?.(); +} +``` + +```typescript +// ❌ 抽象度の混在 +function createUser(data: UserData) { + // 高レベル: ビジネスロジック + validateUser(data); + // 低レベル: DB操作の詳細 + const conn = await pool.getConnection(); + await conn.query('INSERT INTO users...'); + conn.release(); +} + +// ✅ 抽象度を揃える +function createUser(data: UserData) { + validateUser(data); + await userRepository.save(data); // 詳細は隠蔽 +} +``` + +### 7. 不要な後方互換コードの検出 **AIは「後方互換のために」不要なコードを残しがちである。これを見逃さない。** diff --git a/resources/global/ja/agents/default/coder.md b/resources/global/ja/agents/default/coder.md index 56f6fc1..2c9c013 100644 --- a/resources/global/ja/agents/default/coder.md +++ b/resources/global/ja/agents/default/coder.md @@ -105,7 +105,45 @@ | ボーイスカウト | 触った箇所は少し改善して去る | | Fail Fast | エラーは早期に検出。握りつぶさない | -**迷ったら**: Simple を選ぶ。抽象化は後からでもできる。 +**迷ったら**: Simple を選ぶ。 + +## 抽象化の原則 + +**条件分岐を追加する前に考える:** +- 同じ条件が他にもあるか → あればパターンで抽象化 +- 今後も分岐が増えそうか → Strategy/Mapパターンを使う +- 型で分岐しているか → ポリモーフィズムで置換 + +```typescript +// ❌ 条件分岐を増やす +if (type === 'A') { ... } +else if (type === 'B') { ... } +else if (type === 'C') { ... } // また増えた + +// ✅ Mapで抽象化 +const handlers = { A: handleA, B: handleB, C: handleC }; +handlers[type]?.(); +``` + +**抽象度を揃える:** +- 1つの関数内では同じ粒度の処理を並べる +- 詳細な処理は別関数に切り出す +- 「何をするか」と「どうやるか」を混ぜない + +```typescript +// ❌ 抽象度が混在 +function processOrder(order) { + validateOrder(order); // 高レベル + const conn = pool.getConnection(); // 低レベル詳細 + conn.query('INSERT...'); // 低レベル詳細 +} + +// ✅ 抽象度を揃える +function processOrder(order) { + validateOrder(order); + saveOrder(order); // 詳細は隠蔽 +} +``` **言語・フレームワークの作法に従う:** - Pythonなら Pythonic に、KotlinならKotlinらしく @@ -134,6 +172,121 @@ - 子は状態を直接変更しない（イベントを親に通知） - 状態の流れは単方向 +## エラーハンドリング + +**原則: エラーは一元管理する。各所でtry-catchしない。** + +```typescript +// ❌ 各所でtry-catch +async function createUser(data) { + try { + const user = await userService.create(data) + return user + } catch (e) { + console.error(e) + throw new Error('ユーザー作成に失敗しました') + } +} + +// ✅ 上位層で一元処理 +// Controller/Handler層でまとめてキャッチ +// または @ControllerAdvice / ErrorBoundary で処理 +async function createUser(data) { + return await userService.create(data) // 例外はそのまま上に投げる +} +``` + +**エラー処理の配置:** + +| 層 | 責務 | +|----|------| +| ドメイン/サービス層 | ビジネスルール違反時に例外をスロー | +| Controller/Handler層 | 例外をキャッチしてレスポンスに変換 | +| グローバルハンドラ | 共通例外（NotFound, 認証エラー等）を処理 | + +## 変換処理の配置 + +**原則: 変換メソッドはDTO側に持たせる。** + +```typescript +// ✅ Request/Response DTOに変換メソッド +interface CreateUserRequest { + name: string + email: string +} + +function toUseCaseInput(req: CreateUserRequest): CreateUserInput { + return { name: req.name, email: req.email } +} + +// Controller +const input = toUseCaseInput(request) +const output = await useCase.execute(input) +return UserResponse.from(output) +``` + +**変換の方向:** +``` +Request → toInput() → UseCase/Service → Output → Response.from() +``` + +## 共通化の判断 + +**3回ルール:** +- 1回目: そのまま書く +- 2回目: まだ共通化しない（様子見） +- 3回目: 共通化を検討 + +**共通化すべきもの:** +- 同じ処理が3箇所以上 +- 同じスタイル/UIパターン +- 同じバリデーションロジック +- 同じフォーマット処理 + +**共通化すべきでないもの:** +- 似ているが微妙に違うもの（無理に汎用化すると複雑化） +- 1-2箇所しか使わないもの +- 「将来使うかも」という予測に基づくもの + +```typescript +// ❌ 過度な汎用化 +function formatValue(value, type, options) { + if (type === 'currency') { ... } + else if (type === 'date') { ... } + else if (type === 'percentage') { ... } +} + +// ✅ 用途別に関数を分ける +function formatCurrency(amount: number): string { ... } +function formatDate(date: Date): string { ... } +function formatPercentage(value: number): string { ... } +``` + +## テストの書き方 + +**原則: テストは「Given-When-Then」で構造化する。** + +```typescript +test('ユーザーが存在しない場合、NotFoundエラーを返す', async () => { + // Given: 存在しないユーザーID + const nonExistentId = 'non-existent-id' + + // When: ユーザー取得を試みる + const result = await getUser(nonExistentId) + + // Then: NotFoundエラーが返る + expect(result.error).toBe('NOT_FOUND') +}) +``` + +**テストの優先度:** + +| 優先度 | 対象 | +|--------|------| +| 高 | ビジネスロジック、状態遷移 | +| 中 | エッジケース、エラーハンドリング | +| 低 | 単純なCRUD、UIの見た目 | + ## 禁止事項 - **フォールバック値の乱用** - `?? 'unknown'`、`|| 'default'` で問題を隠さない @@ -142,4 +295,5 @@ - **any型** - 型安全を破壊しない - **オブジェクト/配列の直接変更** - スプレッド演算子で新規作成 - **console.log** - 本番コードに残さない -- **機密情報のハードコーディング** \ No newline at end of file +- **機密情報のハードコーディング** +- **各所でのtry-catch** - エラーは上位層で一元処理 \ No newline at end of file diff --git a/resources/global/ja/agents/expert-review/cqrs-es-reviewer.md b/resources/global/ja/agents/expert-review/cqrs-es-reviewer.md index 8181fed..ceb2609 100644 --- a/resources/global/ja/agents/expert-review/cqrs-es-reviewer.md +++ b/resources/global/ja/agents/expert-review/cqrs-es-reviewer.md @@ -32,6 +32,15 @@ ### 1. Aggregate設計 +**原則: Aggregateは判断に必要なフィールドのみ保持する** + +Command Model（Aggregate）の役割は「コマンドを受けて判断し、イベントを発行する」こと。 +クエリ用データはRead Model（Projection）が担当する。 + +**「判断に必要」とは:** +- `if`/`require`の条件分岐に使う +- インスタンスメソッドでイベント発行時にフィールド値を参照する + **必須チェック:** | 基準 | 判定 | @@ -40,12 +49,49 @@ | Aggregate間の直接参照（ID参照でない） | REJECT | | Aggregateが100行を超える | 分割を検討 | | ビジネス不変条件がAggregate外にある | REJECT | +| 判断に使わないフィールドを保持 | REJECT | **良いAggregate:** -- 整合性境界が明確 -- ID参照で他Aggregateを参照 -- コマンドを受け取り、イベントを発行 -- 不変条件を内部で保護 +```kotlin +// ✅ 判断に必要なフィールドのみ +data class Order( + val orderId: String, // イベント発行時に使用 + val status: OrderStatus // 状態チェックに使用 +) { + fun confirm(confirmedBy: String): OrderConfirmedEvent { + require(status == OrderStatus.PENDING) { "確定できる状態ではありません" } + return OrderConfirmedEvent( + orderId = orderId, + confirmedBy = confirmedBy, + confirmedAt = LocalDateTime.now() + ) + } +} + +// ❌ 判断に使わないフィールドを保持 +data class Order( + val orderId: String, + val customerId: String, // 判断に未使用 + val shippingAddress: Address, // 判断に未使用 + val status: OrderStatus +) +``` + +**追加操作がないAggregateはIDのみ:** +```kotlin +// ✅ 作成のみで追加操作がない場合 +data class Notification(val notificationId: String) { + companion object { + fun create(customerId: String, message: String): NotificationCreatedEvent { + return NotificationCreatedEvent( + notificationId = UUID.randomUUID().toString(), + customerId = customerId, + message = message + ) + } + } +} +``` ### 2. イベント設計 @@ -60,7 +106,7 @@ | CRUDスタイルのイベント（Updated, Deleted） | 要検討 | **良いイベント:** -``` +```kotlin // Good: ドメインの意図が明確 OrderPlaced, PaymentReceived, ItemShipped @@ -108,7 +154,57 @@ OrderUpdated, OrderDeleted - イベントから冪等に再構築可能 - Writeモデルから完全に独立 -### 5. 結果整合性 +### 5. Query側の設計 + +**原則: ControllerはQueryGatewayを使う。Repositoryを直接使わない。** + +**レイヤー間の型:** +- `application/query/` - Query結果の型（例: `OrderDetail`） +- `adapter/protocol/` - RESTレスポンスの型（例: `OrderDetailResponse`） +- QueryHandlerはapplication層の型を返し、Controllerがadapter層の型に変換 + +```kotlin +// application/query/OrderDetail.kt +data class OrderDetail( + val orderId: String, + val customerName: String, + val totalAmount: Money +) + +// adapter/protocol/OrderDetailResponse.kt +data class OrderDetailResponse(...) { + companion object { + fun from(detail: OrderDetail) = OrderDetailResponse(...) + } +} + +// QueryHandler - application層の型を返す +@QueryHandler +fun handle(query: GetOrderDetailQuery): OrderDetail? { + val entity = repository.findById(query.id) ?: return null + return OrderDetail(...) +} + +// Controller - adapter層の型に変換 +@GetMapping("/{id}") +fun getById(@PathVariable id: String): ResponseEntity { + val detail = queryGateway.query( + GetOrderDetailQuery(id), + OrderDetail::class.java + ).join() ?: throw NotFoundException("...") + + return ResponseEntity.ok(OrderDetailResponse.from(detail)) +} +``` + +**構成:** +``` +Controller (adapter) → QueryGateway → QueryHandler (application) → Repository + ↓ ↓ +Response.from(detail) OrderDetail +``` + +### 6. 結果整合性 **必須チェック:** @@ -118,7 +214,176 @@ OrderUpdated, OrderDeleted | 整合性遅延が許容範囲を超える | アーキテクチャ再検討 | | 補償トランザクションが未定義 | 障害シナリオの検討を要求 | -### 6. アンチパターン検出 +### 7. Saga vs EventHandler + +**原則: Sagaは「競合が発生する複数アグリゲート間の操作」にのみ使用する** + +**Sagaが必要なケース:** +``` +複数のアクターが同じリソースを取り合う場合 +例: 在庫確保（10人が同時に同じ商品を注文） + +OrderPlacedEvent + ↓ InventoryReservationSaga +ReserveInventoryCommand → Inventory集約（同時実行を直列化） + ↓ +InventoryReservedEvent → ConfirmOrderCommand +InventoryReservationFailedEvent → CancelOrderCommand +``` + +**Sagaが不要なケース:** +``` +競合が発生しない操作 +例: 注文キャンセル時の在庫解放 + +OrderCancelledEvent + ↓ InventoryReleaseHandler（単純なEventHandler） +ReleaseInventoryCommand + ↓ +InventoryReleasedEvent +``` + +**判断基準:** + +| 状況 | Saga | EventHandler | +|------|------|--------------| +| リソースの取り合いがある | ✅ | - | +| 補償トランザクションが必要 | ✅ | - | +| 競合しない単純な連携 | - | ✅ | +| 失敗時は再試行で十分 | - | ✅ | + +**アンチパターン:** +```kotlin +// ❌ ライフサイクル管理のためにSagaを使う +@Saga +class OrderLifecycleSaga { + // 注文の全状態遷移をSagaで追跡 + // PLACED → CONFIRMED → SHIPPED → DELIVERED +} + +// ✅ 結果整合性が必要な操作だけをSagaで処理 +@Saga +class InventoryReservationSaga { + // 在庫確保の同時実行制御のみ +} +``` + +**Sagaはライフサイクル管理ツールではない。** 結果整合性が必要な「操作」単位で作成する。 + +### 8. 例外 vs イベント（失敗時の選択） + +**原則: 監査不要な失敗は例外、監査が必要な失敗はイベント** + +**例外アプローチ（推奨：ほとんどのケース）:** +```kotlin +// ドメインモデル: バリデーション失敗時に例外をスロー +fun reserveInventory(orderId: String, quantity: Int): InventoryReservedEvent { + if (availableQuantity < quantity) { + throw InsufficientInventoryException("在庫が不足しています") + } + return InventoryReservedEvent(productId, orderId, quantity) +} + +// Saga: exceptionally でキャッチして補償アクション +commandGateway.send(command) + .exceptionally { ex -> + commandGateway.send(CancelOrderCommand( + orderId = orderId, + reason = ex.cause?.message ?: "在庫確保に失敗しました" + )) + null + } +``` + +**イベントアプローチ（稀なケース）:** +```kotlin +// 監査が必要な場合のみ +data class PaymentFailedEvent( + val paymentId: String, + val reason: String, + val attemptedAmount: Money +) : PaymentEvent +``` + +**判断基準:** + +| 質問 | 例外 | イベント | +|------|------|----------| +| この失敗を後で確認する必要があるか? | No | Yes | +| 規制やコンプライアンスで記録が必要か? | No | Yes | +| Sagaだけが失敗を気にするか? | Yes | No | +| Event Storeに残すと価値があるか? | No | Yes | + +**デフォルトは例外アプローチ。** 監査要件がある場合のみイベントを検討する。 + +### 9. 抽象化レベルの評価 + +**条件分岐の肥大化検出:** + +| パターン | 判定 | +|---------|------| +| 同じif-elseパターンが3箇所以上 | ポリモーフィズムで抽象化 → **REJECT** | +| switch/caseが5分岐以上 | Strategy/Mapパターンを検討 | +| イベント種別による分岐が増殖 | イベントハンドラを分離 → **REJECT** | +| Aggregate内の状態分岐が複雑 | State Patternを検討 | + +**抽象度の不一致検出:** + +| パターン | 問題 | 修正案 | +|---------|------|--------| +| CommandHandlerにDB操作詳細 | 責務違反 | Repository層に分離 | +| EventHandlerにビジネスロジック | 責務違反 | ドメインサービスに抽出 | +| Aggregateに永続化処理 | レイヤー違反 | EventStore経由に変更 | +| Projectionに計算ロジック | 保守困難 | 専用サービスに抽出 | + +**良い抽象化の例:** +```kotlin +// ❌ イベント種別による分岐の増殖 +@EventHandler +fun on(event: DomainEvent) { + when (event) { + is OrderPlacedEvent -> handleOrderPlaced(event) + is OrderConfirmedEvent -> handleOrderConfirmed(event) + is OrderShippedEvent -> handleOrderShipped(event) + // ...どんどん増える + } +} + +// ✅ イベントごとにハンドラを分離 +@EventHandler +fun on(event: OrderPlacedEvent) { ... } + +@EventHandler +fun on(event: OrderConfirmedEvent) { ... } + +@EventHandler +fun on(event: OrderShippedEvent) { ... } +``` + +```kotlin +// ❌ 状態による分岐が複雑 +fun process(command: ProcessCommand) { + when (status) { + PENDING -> if (command.type == "approve") { ... } else if (command.type == "reject") { ... } + APPROVED -> if (command.type == "ship") { ... } + // ...複雑化 + } +} + +// ✅ State Patternで抽象化 +sealed class OrderState { + abstract fun handle(command: ProcessCommand): List +} +class PendingState : OrderState() { + override fun handle(command: ProcessCommand) = when (command) { + is ApproveCommand -> listOf(OrderApprovedEvent(...)) + is RejectCommand -> listOf(OrderRejectedEvent(...)) + else -> throw InvalidCommandException() + } +} +``` + +### 10. アンチパターン検出 以下を見つけたら **REJECT**: @@ -131,7 +396,59 @@ OrderUpdated, OrderDeleted | Missing Events | 重要なドメインイベントが欠落 | | God Aggregate | 1つのAggregateに全責務が集中 | -### 7. インフラ層 +### 11. テスト戦略 + +**原則: レイヤーごとにテスト方針を分ける** + +**テストピラミッド:** +``` + ┌─────────────┐ + │ E2E Test │ ← 少数：全体フロー確認 + ├─────────────┤ + │ Integration │ ← Command→Event→Projection→Query の連携確認 + ├─────────────┤ + │ Unit Test │ ← 多数：各レイヤー独立テスト + └─────────────┘ +``` + +**Command側（Aggregate）:** +```kotlin +// AggregateTestFixture使用 +@Test +fun `確定コマンドでイベントが発行される`() { + fixture + .given(OrderPlacedEvent(...)) + .`when`(ConfirmOrderCommand(orderId, confirmedBy)) + .expectSuccessfulHandlerExecution() + .expectEvents(OrderConfirmedEvent(...)) +} +``` + +**Query側:** +```kotlin +// Read Model直接セットアップ + QueryGateway +@Test +fun `注文詳細が取得できる`() { + // Given: Read Modelを直接セットアップ + orderRepository.save(OrderEntity(...)) + + // When: QueryGateway経由でクエリ実行 + val detail = queryGateway.query(GetOrderDetailQuery(orderId), ...).join() + + // Then + assertEquals(expectedDetail, detail) +} +``` + +**チェック項目:** + +| 観点 | 判定 | +|------|------| +| Aggregateテストが状態ではなくイベントを検証している | 必須 | +| Query側テストがCommand経由でデータを作っていない | 推奨 | +| 統合テストでAxonの非同期処理を考慮している | 必須 | + +### 12. インフラ層 **確認事項:** - イベントストアの選択は適切か @@ -147,6 +464,7 @@ OrderUpdated, OrderDeleted | Aggregate設計に問題 | REJECT | | イベント設計が不適切 | REJECT | | 結果整合性の考慮不足 | REJECT | +| 抽象化レベルの不一致 | REJECT | | 軽微な改善点のみ | APPROVE（改善提案は付記） | ## 口調の特徴 @@ -162,3 +480,5 @@ OrderUpdated, OrderDeleted - **イベントの質にこだわる**: イベントはドメインの歴史書である - **結果整合性を恐れない**: 正しく設計されたESは強整合性より堅牢 - **過度な複雑さを警戒**: シンプルなCRUDで十分なケースにCQRS+ESを強制しない +- **Aggregateは軽く保つ**: 判断に不要なフィールドは持たない +- **Sagaを乱用しない**: 競合制御が必要な操作にのみ使用する diff --git a/resources/global/ja/agents/expert-review/frontend-reviewer.md b/resources/global/ja/agents/expert-review/frontend-reviewer.md index a28bad5..93789b3 100644 --- a/resources/global/ja/agents/expert-review/frontend-reviewer.md +++ b/resources/global/ja/agents/expert-review/frontend-reviewer.md @@ -36,6 +36,15 @@ ### 1. コンポーネント設計 +**原則: 1ファイルにベタ書きしない。必ずコンポーネント分割する。** + +**分離が必須なケース:** +- 独自のstateを持つ → 必ず分離 +- 50行超のJSX → 分離 +- 再利用可能 → 分離 +- 責務が複数 → 分離 +- ページ内の独立したセクション → 分離 + **必須チェック:** | 基準 | 判定 | @@ -60,8 +69,44 @@ | Layout | 配置・構造 | `PageLayout`, `Grid` | | Utility | 共通機能 | `ErrorBoundary`, `Portal` | +**ディレクトリ構成:** +``` +features/{feature-name}/ +├── components/ +│ ├── {feature}-view.tsx # メインビュー（子を組み合わせる） +│ ├── {sub-component}.tsx # サブコンポーネント +│ └── index.ts +├── hooks/ +├── types.ts +└── index.ts +``` + ### 2. 状態管理 +**原則: 子コンポーネントは自身で状態を変更しない。イベントを親にバブリングし、親が状態を操作する。** + +```tsx +// ❌ 子が自分で状態を変更 +const ChildBad = ({ initialValue }: { initialValue: string }) => { + const [value, setValue] = useState(initialValue) + return setValue(e.target.value)} /> +} + +// ✅ 親が状態を管理、子はコールバックで通知 +const ChildGood = ({ value, onChange }: { value: string; onChange: (v: string) => void }) => { + return onChange(e.target.value)} /> +} + +const Parent = () => { + const [value, setValue] = useState('') + return +} +``` + +**例外（子がローカルstate持ってOK）:** +- UI専用の一時状態（ホバー、フォーカス、アニメーション） +- 親に伝える必要がない完全にローカルな状態 + **必須チェック:** | 基準 | 判定 | @@ -81,7 +126,252 @@ | 複数コンポーネントで共有 | Context or 状態管理ライブラリ | | サーバーデータのキャッシュ | TanStack Query等のデータフェッチライブラリ | -### 3. パフォーマンス +### 3. データ取得 + +**原則: API呼び出しはルート（View）コンポーネントで行い、子コンポーネントにはpropsで渡す。** + +```tsx +// ✅ CORRECT - ルートでデータ取得、子に渡す +const OrderDetailView = () => { + const { data: order, isLoading, error } = useGetOrder(orderId) + const { data: items } = useListOrderItems(orderId) + + if (isLoading) return + if (error) return + + return ( + + ) +} + +// ❌ WRONG - 子コンポーネントが自分でデータ取得 +const OrderSummary = ({ orderId }) => { + const { data: order } = useGetOrder(orderId) + // ... +} +``` + +**理由:** +- データフローが明示的で追跡しやすい +- 子コンポーネントは純粋なプレゼンテーション（テストしやすい） +- 子コンポーネントに隠れた依存関係がなくなる + +**UIの状態変更でパラメータが変わる場合（週切り替え、フィルタ等）:** + +状態もViewレベルで管理し、コンポーネントにはコールバックを渡す。 + +```tsx +// ✅ CORRECT - 状態もViewで管理 +const ScheduleView = () => { + const [currentWeek, setCurrentWeek] = useState(startOfWeek(new Date())) + const { data } = useListSchedules({ + from: format(currentWeek, 'yyyy-MM-dd'), + to: format(endOfWeek(currentWeek), 'yyyy-MM-dd'), + }) + + return ( + + ) +} + +// ❌ WRONG - コンポーネント内で状態管理＋データ取得 +const WeeklyCalendar = ({ facilityId }) => { + const [currentWeek, setCurrentWeek] = useState(...) + const { data } = useListSchedules({ facilityId, from, to }) + // ... +} +``` + +**例外（コンポーネント内フェッチが許容されるケース）:** + +| ケース | 理由 | +|--------|------| +| 無限スクロール | スクロール位置というUI内部状態に依存 | +| 検索オートコンプリート | 入力値に依存したリアルタイム検索 | +| 独立したウィジェット | 通知バッジ、天気等。親のデータと完全に無関係 | +| リアルタイム更新 | WebSocket/Pollingでの自動更新 | +| モーダル内の詳細取得 | 開いたときだけ追加データを取得 | + +**判断基準: 「親が管理する意味がない / 親に影響を与えない」ケースのみ許容。** + +**必須チェック:** + +| 基準 | 判定 | +|------|------| +| コンポーネント内で直接fetch | Container層に分離 | +| エラーハンドリングなし | REJECT | +| ローディング状態の未処理 | REJECT | +| キャンセル処理なし | 警告 | +| N+1クエリ的なフェッチ | REJECT | + +### 4. 共有コンポーネントと抽象化 + +**原則: 同じパターンのUIは共有コンポーネント化する。インラインスタイルのコピペは禁止。** + +```tsx +// ❌ WRONG - インラインスタイルのコピペ + + +// ✅ CORRECT - 共有コンポーネント使用 + + + +``` + +**共有コンポーネント化すべきパターン:** +- アイコンボタン（閉じる、編集、削除等） +- ローディング/エラー表示 +- ステータスバッジ +- タブ切り替え +- ラベル+値の表示（詳細画面） +- 検索入力 +- カラー凡例 + +**過度な汎用化を避ける:** + +```tsx +// ❌ WRONG - IconButtonに無理やりステッパー用バリアントを追加 +export const iconButtonVariants = cva('...', { + variants: { + variant: { + default: '...', + outlined: '...', // ← ステッパー専用、他で使わない + }, + size: { + medium: 'p-2', + stepper: 'w-8 h-8', // ← outlinedとセットでしか使わない + }, + }, +}) + +// ✅ CORRECT - 用途別に専用コンポーネント +export function StepperButton(props) { + return ( + + ) +} +``` + +**別コンポーネントにすべきサイン:** +- 「このvariantはこのsizeとセット」のような暗黙の制約がある +- 追加したvariantが元のコンポーネントの用途と明らかに違う +- 使う側のprops指定が複雑になる + +### 5. 抽象化レベルの評価 + +**条件分岐の肥大化検出:** + +| パターン | 判定 | +|---------|------| +| 同じ条件分岐が3箇所以上 | 共通コンポーネントに抽出 → **REJECT** | +| propsによる分岐が5種類以上 | コンポーネント分割を検討 | +| render内の三項演算子のネスト | 早期リターンまたはコンポーネント分離 → **REJECT** | +| 型による分岐レンダリング | ポリモーフィックコンポーネントを検討 | + +**抽象度の不一致検出:** + +| パターン | 問題 | 修正案 | +|---------|------|--------| +| データ取得ロジックがJSXに混在 | 読みにくい | カスタムフックに抽出 | +| ビジネスロジックがコンポーネントに混在 | 責務違反 | hooks/utilsに分離 | +| スタイル計算ロジックが散在 | 保守困難 | ユーティリティ関数に抽出 | +| 同じ変換処理が複数箇所に | DRY違反 | 共通関数に抽出 | + +**良い抽象化の例:** +```tsx +// ❌ 条件分岐が肥大化 +function UserBadge({ user }) { + if (user.role === 'admin') { + return 管理者 + } else if (user.role === 'moderator') { + return モデレーター + } else if (user.role === 'premium') { + return プレミアム + } else { + return 一般 + } +} + +// ✅ Mapで抽象化 +const ROLE_CONFIG = { + admin: { label: '管理者', className: 'bg-red-500' }, + moderator: { label: 'モデレーター', className: 'bg-yellow-500' }, + premium: { label: 'プレミアム', className: 'bg-purple-500' }, + default: { label: '一般', className: 'bg-gray-500' }, +} + +function UserBadge({ user }) { + const config = ROLE_CONFIG[user.role] ?? ROLE_CONFIG.default + return {config.label} +} +``` + +```tsx +// ❌ 抽象度が混在 +function OrderList() { + const [orders, setOrders] = useState([]) + useEffect(() => { + fetch('/api/orders') + .then(res => res.json()) + .then(data => setOrders(data)) + }, []) + + return orders.map(order => ( +
{order.total.toLocaleString()}円
+ )) +} + +// ✅ 抽象度を揃える +function OrderList() { + const { data: orders } = useOrders() // データ取得を隠蔽 + + return orders.map(order => ( + + )) +} +``` + +### 6. データと表示形式の責務分離 + +**原則: バックエンドは「データ」を返し、フロントエンドが「表示形式」に変換する。** + +```tsx +// ✅ フロントエンド: 表示形式に変換 +export function formatPrice(amount: number): string { + return `¥${amount.toLocaleString()}` +} + +export function formatDate(date: Date): string { + return format(date, 'yyyy年M月d日') +} +``` + +**理由:** +- 表示形式はUI要件であり、バックエンドの責務ではない +- 国際化対応が容易 +- フロントエンドが柔軟に表示を変更できる + +**必須チェック:** + +| 基準 | 判定 | +|------|------| +| バックエンドが表示用文字列を返している | 設計見直しを提案 | +| 同じフォーマット処理が複数箇所にコピペ | ユーティリティ関数に統一 | +| コンポーネント内でインラインフォーマット | 関数に抽出 | + +### 7. パフォーマンス **必須チェック:** @@ -102,40 +392,15 @@ **アンチパターン:** ```tsx -// Bad: レンダリングごとに新しいオブジェクト +// ❌ レンダリングごとに新しいオブジェクト -// Good: 定数化 or useMemo +// ✅ 定数化 or useMemo const style = useMemo(() => ({ color: 'red' }), []); ``` -### 4. データフェッチ - -**必須チェック:** - -| 基準 | 判定 | -|------|------| -| コンポーネント内で直接fetch | Container層に分離 | -| エラーハンドリングなし | REJECT | -| ローディング状態の未処理 | REJECT | -| キャンセル処理なし | 警告 | -| N+1クエリ的なフェッチ | REJECT | - -**推奨パターン:** -```tsx -// Good: データフェッチはルートで -function UserPage() { - const { data, isLoading, error } = useQuery(['user', id], fetchUser); - - if (isLoading) return ; - if (error) return ; - - return ; -} -``` - -### 5. アクセシビリティ +### 8. アクセシビリティ **必須チェック:** @@ -154,7 +419,7 @@ function UserPage() { - [ ] スクリーンリーダーで意味が通じるか - [ ] カラーコントラストは十分か -### 6. TypeScript/型安全性 +### 9. TypeScript/型安全性 **必須チェック:** @@ -165,7 +430,7 @@ function UserPage() { | Props型定義なし | REJECT | | イベントハンドラの型が不適切 | 修正が必要 | -### 7. フロントエンドセキュリティ +### 10. フロントエンドセキュリティ **必須チェック:** @@ -176,7 +441,7 @@ function UserPage() { | 機密情報のフロントエンド保存 | REJECT | | CSRFトークンの未使用 | 要確認 | -### 8. テスタビリティ +### 11. テスタビリティ **必須チェック:** @@ -186,7 +451,7 @@ function UserPage() { | テスト困難な構造 | 分離を検討 | | ビジネスロジックのUIへの埋め込み | REJECT | -### 9. アンチパターン検出 +### 12. アンチパターン検出 以下を見つけたら **REJECT**: @@ -198,6 +463,8 @@ function UserPage() { | useEffect地獄 | 依存関係が複雑すぎる | | Premature Optimization | 不要なメモ化 | | Magic Strings | ハードコードされた文字列 | +| Hidden Dependencies | 子コンポーネントの隠れたAPI呼び出し | +| Over-generalization | 無理やり汎用化したコンポーネント | ## 判定基準 @@ -206,6 +473,7 @@ function UserPage() { | コンポーネント設計に問題 | REJECT | | 状態管理に問題 | REJECT | | アクセシビリティ違反 | REJECT | +| 抽象化レベルの不一致 | REJECT | | パフォーマンス問題 | REJECT（重大な場合） | | 軽微な改善点のみ | APPROVE（改善提案は付記） | @@ -223,3 +491,5 @@ function UserPage() { - **アクセシビリティは後付け困難**: 最初から組み込む - **過度な抽象化を警戒**: シンプルに保つ - **フレームワークの作法に従う**: 独自パターンより標準的なアプローチ +- **データ取得はルートで**: 子コンポーネントに隠れた依存を作らない +- **制御されたコンポーネント**: 状態の流れは単方向 diff --git a/resources/global/ja/workflows/expert-review.yaml b/resources/global/ja/workflows/expert-review.yaml index 0e6a8bc..ba41cdf 100644 --- a/resources/global/ja/workflows/expert-review.yaml +++ b/resources/global/ja/workflows/expert-review.yaml @@ -631,7 +631,7 @@ steps: pass_previous_response: true transitions: - condition: done - next_step: cqrs_es_review + next_step: ai_review - condition: blocked next_step: plan diff --git a/src/agents/runner.ts b/src/agents/runner.ts index 0e06882..4d9d99d 100644 --- a/src/agents/runner.ts +++ b/src/agents/runner.ts @@ -6,17 +6,15 @@ import { execSync } from 'node:child_process'; import { existsSync, readFileSync } from 'node:fs'; import { basename, dirname } from 'node:path'; import { - callClaude, - callClaudeCustom, callClaudeAgent, callClaudeSkill, - ClaudeCallOptions, + type ClaudeCallOptions, } from '../claude/client.js'; import { type StreamCallback, type PermissionHandler, type AskUserQuestionHandler } from '../claude/process.js'; -import { callCodex, callCodexCustom, type CodexCallOptions } from '../codex/client.js'; import { loadCustomAgents, loadAgentPrompt } from '../config/loader.js'; import { loadGlobalConfig } from '../config/globalConfig.js'; import { loadProjectConfig } from '../config/projectConfig.js'; +import { getProvider, type ProviderType, type ProviderCallOptions } from '../providers/index.js'; import type { AgentResponse, CustomAgentConfig } from '../models/types.js'; export type { StreamCallback }; @@ -26,7 +24,7 @@ export interface RunAgentOptions { cwd: string; sessionId?: string; model?: string; - provider?: 'claude' | 'codex'; + provider?: 'claude' | 'codex' | 'mock'; /** Resolved path to agent prompt file */ agentPath?: string; /** Allowed tools for this agent run */ @@ -40,9 +38,8 @@ export interface RunAgentOptions { bypassPermissions?: boolean; } -type AgentProvider = 'claude' | 'codex'; - -function resolveProvider(cwd: string, options?: RunAgentOptions, agentConfig?: CustomAgentConfig): AgentProvider { +function resolveProvider(cwd: string, options?: RunAgentOptions, agentConfig?: CustomAgentConfig): ProviderType { + // Mock provider must be explicitly specified (no fallback) if (options?.provider) return options.provider; if (agentConfig?.provider) return agentConfig.provider; const projectConfig = loadProjectConfig(cwd); @@ -137,33 +134,22 @@ export async function runCustomAgent( systemPrompt = `${systemPrompt}\n\n${options.statusRulesPrompt}`; } - const tools = allowedTools; - const provider = resolveProvider(options.cwd, options, agentConfig); - const model = resolveModel(options.cwd, options, agentConfig); - if (provider === 'codex') { - const callOptions: CodexCallOptions = { - cwd: options.cwd, - sessionId: options.sessionId, - model, - statusPatterns: agentConfig.statusPatterns, - onStream: options.onStream, - }; - return callCodexCustom(agentConfig.name, task, systemPrompt, callOptions); - } + const providerType = resolveProvider(options.cwd, options, agentConfig); + const provider = getProvider(providerType); - const callOptions: ClaudeCallOptions = { - cwd: options.cwd, - sessionId: options.sessionId, - allowedTools: tools, - model, - statusPatterns: agentConfig.statusPatterns, + const callOptions: ProviderCallOptions = { + cwd: options.cwd, + sessionId: options.sessionId, + allowedTools, + model: resolveModel(options.cwd, options, agentConfig), + statusPatterns: agentConfig.statusPatterns, onStream: options.onStream, onPermissionRequest: options.onPermissionRequest, onAskUserQuestion: options.onAskUserQuestion, bypassPermissions: options.bypassPermissions, }; - return callClaudeCustom(agentConfig.name, task, systemPrompt, callOptions); + return provider.callCustom(agentConfig.name, task, systemPrompt, callOptions); } /** @@ -221,26 +207,14 @@ export async function runAgent( systemPrompt = `${systemPrompt}\n\n${options.statusRulesPrompt}`; } - const tools = options.allowedTools; - const provider = resolveProvider(options.cwd, options); - const model = resolveModel(options.cwd, options); + const providerType = resolveProvider(options.cwd, options); + const provider = getProvider(providerType); - if (provider === 'codex') { - const callOptions: CodexCallOptions = { - cwd: options.cwd, - sessionId: options.sessionId, - model, - systemPrompt, - onStream: options.onStream, - }; - return callCodex(agentName, task, callOptions); - } - - const callOptions: ClaudeCallOptions = { + const callOptions: ProviderCallOptions = { cwd: options.cwd, sessionId: options.sessionId, - allowedTools: tools, - model, + allowedTools: options.allowedTools, + model: resolveModel(options.cwd, options), systemPrompt, onStream: options.onStream, onPermissionRequest: options.onPermissionRequest, @@ -248,7 +222,7 @@ export async function runAgent( bypassPermissions: options.bypassPermissions, }; - return callClaude(agentName, task, callOptions); + return provider.call(agentName, task, callOptions); } // Fallback: Look for custom agent by name diff --git a/src/mock/client.ts b/src/mock/client.ts new file mode 100644 index 0000000..d4c4f9f --- /dev/null +++ b/src/mock/client.ts @@ -0,0 +1,85 @@ +/** + * Mock agent client for testing + * + * Returns immediate fixed responses without any API calls. + * Useful for testing workflows without incurring costs or latency. + */ + +import { randomUUID } from 'node:crypto'; +import type { StreamCallback, StreamEvent } from '../claude/process.js'; +import type { AgentResponse } from '../models/types.js'; + +/** Options for mock calls */ +export interface MockCallOptions { + cwd: string; + sessionId?: string; + onStream?: StreamCallback; + /** Fixed response content (optional, defaults to generic mock response) */ + mockResponse?: string; + /** Fixed status to return (optional, defaults to 'done') */ + mockStatus?: 'done' | 'blocked' | 'approved' | 'rejected' | 'improve'; +} + +/** + * Generate a mock session ID + */ +function generateMockSessionId(): string { + return `mock-session-${randomUUID()}`; +} + +/** + * Call mock agent - returns immediate fixed response + */ +export async function callMock( + agentName: string, + prompt: string, + options: MockCallOptions +): Promise { + const sessionId = options.sessionId ?? generateMockSessionId(); + const status = options.mockStatus ?? 'done'; + const statusMarker = `[MOCK:${status.toUpperCase()}]`; + const content = options.mockResponse ?? + `${statusMarker}\n\nMock response for agent "${agentName}".\nPrompt: ${prompt.slice(0, 100)}${prompt.length > 100 ? '...' : ''}`; + + // Emit stream events if callback is provided + if (options.onStream) { + const initEvent: StreamEvent = { + type: 'init', + data: { model: 'mock-model', sessionId }, + }; + options.onStream(initEvent); + + const textEvent: StreamEvent = { + type: 'text', + data: { text: content }, + }; + options.onStream(textEvent); + + const resultEvent: StreamEvent = { + type: 'result', + data: { success: true, result: content, sessionId }, + }; + options.onStream(resultEvent); + } + + return { + agent: agentName, + status, + content, + timestamp: new Date(), + sessionId, + }; +} + +/** + * Call mock agent with custom system prompt (same as callMock for mock provider) + */ +export async function callMockCustom( + agentName: string, + prompt: string, + _systemPrompt: string, + options: MockCallOptions +): Promise { + // For mock, system prompt is ignored - just return fixed response + return callMock(agentName, prompt, options); +} diff --git a/src/models/schemas.ts b/src/models/schemas.ts index 27f77eb..7582737 100644 --- a/src/models/schemas.ts +++ b/src/models/schemas.ts @@ -60,7 +60,7 @@ export const WorkflowStepRawSchema = z.object({ /** Display name for the agent (shown in output). Falls back to agent basename if not specified */ agent_name: z.string().optional(), allowed_tools: z.array(z.string()).optional(), - provider: z.enum(['claude', 'codex']).optional(), + provider: z.enum(['claude', 'codex', 'mock']).optional(), model: z.string().optional(), instruction: z.string().optional(), instruction_template: z.string().optional(), @@ -94,7 +94,7 @@ export const CustomAgentConfigSchema = z.object({ status_patterns: z.record(z.string(), z.string()).optional(), claude_agent: z.string().optional(), claude_skill: z.string().optional(), - provider: z.enum(['claude', 'codex']).optional(), + provider: z.enum(['claude', 'codex', 'mock']).optional(), model: z.string().optional(), }).refine( (data) => data.prompt_file || data.prompt || data.claude_agent || data.claude_skill, @@ -116,7 +116,7 @@ export const GlobalConfigSchema = z.object({ trusted_directories: z.array(z.string()).optional().default([]), default_workflow: z.string().optional().default('default'), log_level: z.enum(['debug', 'info', 'warn', 'error']).optional().default('info'), - provider: z.enum(['claude', 'codex']).optional().default('claude'), + provider: z.enum(['claude', 'codex', 'mock']).optional().default('claude'), model: z.string().optional(), debug: DebugConfigSchema.optional(), }); @@ -125,7 +125,7 @@ export const GlobalConfigSchema = z.object({ export const ProjectConfigSchema = z.object({ workflow: z.string().optional(), agents: z.array(CustomAgentConfigSchema).optional(), - provider: z.enum(['claude', 'codex']).optional(), + provider: z.enum(['claude', 'codex', 'mock']).optional(), }); /** diff --git a/src/models/types.ts b/src/models/types.ts index a5b5309..d39b3c3 100644 --- a/src/models/types.ts +++ b/src/models/types.ts @@ -65,7 +65,7 @@ export interface WorkflowStep { /** Resolved absolute path to agent prompt file (set by loader) */ agentPath?: string; /** Provider override for this step */ - provider?: 'claude' | 'codex'; + provider?: 'claude' | 'codex' | 'mock'; /** Model override for this step */ model?: string; instructionTemplate: string; @@ -129,7 +129,7 @@ export interface CustomAgentConfig { statusPatterns?: Record; claudeAgent?: string; claudeSkill?: string; - provider?: 'claude' | 'codex'; + provider?: 'claude' | 'codex' | 'mock'; model?: string; } @@ -148,7 +148,7 @@ export interface GlobalConfig { trustedDirectories: string[]; defaultWorkflow: string; logLevel: 'debug' | 'info' | 'warn' | 'error'; - provider?: 'claude' | 'codex'; + provider?: 'claude' | 'codex' | 'mock'; model?: string; debug?: DebugConfig; } @@ -157,5 +157,5 @@ export interface GlobalConfig { export interface ProjectConfig { workflow?: string; agents?: CustomAgentConfig[]; - provider?: 'claude' | 'codex'; + provider?: 'claude' | 'codex' | 'mock'; } diff --git a/src/providers/claude.ts b/src/providers/claude.ts new file mode 100644 index 0000000..6988ec2 --- /dev/null +++ b/src/providers/claude.ts @@ -0,0 +1,43 @@ +/** + * Claude provider implementation + */ + +import { callClaude, callClaudeCustom, type ClaudeCallOptions } from '../claude/client.js'; +import type { AgentResponse } from '../models/types.js'; +import type { Provider, ProviderCallOptions } from './index.js'; + +/** Claude provider - wraps existing Claude client */ +export class ClaudeProvider implements Provider { + async call(agentName: string, prompt: string, options: ProviderCallOptions): Promise { + const callOptions: ClaudeCallOptions = { + cwd: options.cwd, + sessionId: options.sessionId, + allowedTools: options.allowedTools, + model: options.model, + systemPrompt: options.systemPrompt, + statusPatterns: options.statusPatterns, + onStream: options.onStream, + onPermissionRequest: options.onPermissionRequest, + onAskUserQuestion: options.onAskUserQuestion, + bypassPermissions: options.bypassPermissions, + }; + + return callClaude(agentName, prompt, callOptions); + } + + async callCustom(agentName: string, prompt: string, systemPrompt: string, options: ProviderCallOptions): Promise { + const callOptions: ClaudeCallOptions = { + cwd: options.cwd, + sessionId: options.sessionId, + allowedTools: options.allowedTools, + model: options.model, + statusPatterns: options.statusPatterns, + onStream: options.onStream, + onPermissionRequest: options.onPermissionRequest, + onAskUserQuestion: options.onAskUserQuestion, + bypassPermissions: options.bypassPermissions, + }; + + return callClaudeCustom(agentName, prompt, systemPrompt, callOptions); + } +} diff --git a/src/providers/codex.ts b/src/providers/codex.ts new file mode 100644 index 0000000..408956f --- /dev/null +++ b/src/providers/codex.ts @@ -0,0 +1,35 @@ +/** + * Codex provider implementation + */ + +import { callCodex, callCodexCustom, type CodexCallOptions } from '../codex/client.js'; +import type { AgentResponse } from '../models/types.js'; +import type { Provider, ProviderCallOptions } from './index.js'; + +/** Codex provider - wraps existing Codex client */ +export class CodexProvider implements Provider { + async call(agentName: string, prompt: string, options: ProviderCallOptions): Promise { + const callOptions: CodexCallOptions = { + cwd: options.cwd, + sessionId: options.sessionId, + model: options.model, + systemPrompt: options.systemPrompt, + statusPatterns: options.statusPatterns, + onStream: options.onStream, + }; + + return callCodex(agentName, prompt, callOptions); + } + + async callCustom(agentName: string, prompt: string, systemPrompt: string, options: ProviderCallOptions): Promise { + const callOptions: CodexCallOptions = { + cwd: options.cwd, + sessionId: options.sessionId, + model: options.model, + statusPatterns: options.statusPatterns, + onStream: options.onStream, + }; + + return callCodexCustom(agentName, prompt, systemPrompt, callOptions); + } +} diff --git a/src/providers/index.ts b/src/providers/index.ts new file mode 100644 index 0000000..d664178 --- /dev/null +++ b/src/providers/index.ts @@ -0,0 +1,63 @@ +/** + * Provider abstraction layer + * + * Provides a unified interface for different agent providers (Claude, Codex, Mock). + * This enables adding new providers without modifying the runner logic. + */ + +import type { StreamCallback, PermissionHandler, AskUserQuestionHandler } from '../claude/process.js'; +import type { AgentResponse } from '../models/types.js'; +import { ClaudeProvider } from './claude.js'; +import { CodexProvider } from './codex.js'; +import { MockProvider } from './mock.js'; + +/** Common options for all providers */ +export interface ProviderCallOptions { + cwd: string; + sessionId?: string; + model?: string; + systemPrompt?: string; + allowedTools?: string[]; + statusPatterns?: Record; + onStream?: StreamCallback; + onPermissionRequest?: PermissionHandler; + onAskUserQuestion?: AskUserQuestionHandler; + bypassPermissions?: boolean; +} + +/** Provider interface - all providers must implement this */ +export interface Provider { + /** Call the provider with a prompt (using systemPrompt from options if provided) */ + call(agentName: string, prompt: string, options: ProviderCallOptions): Promise; + + /** Call the provider with explicit system prompt */ + callCustom(agentName: string, prompt: string, systemPrompt: string, options: ProviderCallOptions): Promise; +} + +/** Provider type */ +export type ProviderType = 'claude' | 'codex' | 'mock'; + +/** Provider registry */ +const providers: Record = { + claude: new ClaudeProvider(), + codex: new CodexProvider(), + mock: new MockProvider(), +}; + +/** + * Get a provider instance by type + */ +export function getProvider(type: ProviderType): Provider { + const provider = providers[type]; + if (!provider) { + throw new Error(`Unknown provider type: ${type}`); + } + return provider; +} + +/** + * Register a custom provider + */ +export function registerProvider(type: string, provider: Provider): void { + (providers as Record)[type] = provider; +} diff --git a/src/providers/mock.ts b/src/providers/mock.ts new file mode 100644 index 0000000..d046988 --- /dev/null +++ b/src/providers/mock.ts @@ -0,0 +1,30 @@ +/** + * Mock provider implementation + */ + +import { callMock, callMockCustom, type MockCallOptions } from '../mock/client.js'; +import type { AgentResponse } from '../models/types.js'; +import type { Provider, ProviderCallOptions } from './index.js'; + +/** Mock provider - wraps existing Mock client */ +export class MockProvider implements Provider { + async call(agentName: string, prompt: string, options: ProviderCallOptions): Promise { + const callOptions: MockCallOptions = { + cwd: options.cwd, + sessionId: options.sessionId, + onStream: options.onStream, + }; + + return callMock(agentName, prompt, callOptions); + } + + async callCustom(agentName: string, prompt: string, _systemPrompt: string, options: ProviderCallOptions): Promise { + const callOptions: MockCallOptions = { + cwd: options.cwd, + sessionId: options.sessionId, + onStream: options.onStream, + }; + + return callMockCustom(agentName, prompt, _systemPrompt, callOptions); + } +}