Merge branch 'takt/pipeline-1769947033' into takt/issue-81-1769929093
This commit is contained in:
nrslib
commit
8208f8b961
@ -105,26 +105,70 @@ AI is confidently wrong—code that looks plausible but doesn't work, solutions
|
||||
|
||||
**Principle:** The best code is the minimum code that solves the problem.
|
||||
|
||||
### 6. Fallback Prohibition Review (REJECT criteria)
|
||||
### 6. Fallback & Default Argument Prohibition Review (REJECT criteria)
|
||||
|
||||
**AI overuses fallbacks to hide uncertainty. This is a REJECT by default.**
|
||||
**AI overuses fallbacks and default arguments to hide uncertainty. Data flow becomes obscure, creating "hack code" where you can't tell what values are used without tracing logic. This is a REJECT by default.**
|
||||
|
||||
| Pattern | Example | Verdict |
|
||||
|---------|---------|---------|
|
||||
| Swallowing with defaults | `?? 'unknown'`, `\|\| 'default'`, `?? []` | REJECT |
|
||||
| try-catch returning empty | `catch { return ''; }` `catch { return 0; }` | REJECT |
|
||||
| Silent skip via conditionals | `if (!x) return;` skipping what should be an error | REJECT |
|
||||
| Multi-level fallback chains | `a ?? b ?? c ?? d` | REJECT |
|
||||
**Core problem:** You can't understand what values are being used without tracing the entire logic path.
|
||||
|
||||
| Pattern | Example | Problem | Verdict |
|
||||
|---------|---------|---------|---------|
|
||||
| Fallback for required data | `user?.id ?? 'unknown'` | Processing continues in an error state | **REJECT** |
|
||||
| Default argument abuse | `function f(x = 'default')` where all callers omit | Data flow obscured | **REJECT** |
|
||||
| Nullish coalescing with no upstream path | `options?.cwd ?? process.cwd()` with no way to pass | Always uses fallback (meaningless) | **REJECT** |
|
||||
| try-catch returning empty | `catch { return ''; }` | Swallows errors | **REJECT** |
|
||||
| Multi-level fallback | `a ?? b ?? c ?? d` | Complex value determination logic | **REJECT** |
|
||||
| Silent skip via conditionals | `if (!x) return;` skipping error | Hides bugs | **REJECT** |
|
||||
|
||||
**Default argument examples:**
|
||||
|
||||
```typescript
|
||||
// ❌ Bad example - All callers omit
|
||||
function loadWorkflow(name: string, cwd = process.cwd()) { ... }
|
||||
// All callers: loadWorkflow('default') ← not passing cwd
|
||||
// Problem: Can't tell where cwd value comes from by reading call sites
|
||||
// Fix: Make cwd required, pass explicitly at call sites
|
||||
|
||||
// ✅ Good example - Only some callers omit
|
||||
function query(sql: string, timeout = 30000) { ... }
|
||||
// Caller A: query(sql) ← Uses default
|
||||
// Caller B: query(sql, 60000) ← Explicit
|
||||
// Reason: timeout is explicitly optional configuration
|
||||
```
|
||||
|
||||
**Nullish coalescing examples:**
|
||||
|
||||
```typescript
|
||||
// ❌ Bad example - No upstream path to pass value
|
||||
class Engine {
|
||||
constructor(config, cwd, task, options?) {
|
||||
this.projectCwd = options?.projectCwd ?? cwd
|
||||
// Problem: If options is passed as { }, projectCwd is always undefined
|
||||
// Result: always uses cwd (fallback is meaningless)
|
||||
}
|
||||
}
|
||||
// Fix: Modify upstream function signatures to allow passing options.projectCwd
|
||||
|
||||
// ✅ Good example - Upstream path exists
|
||||
function execute(task, options?: { projectCwd?: string }) {
|
||||
const cwd = options?.projectCwd ?? process.cwd()
|
||||
// Reason: Caller chooses whether to pass options.projectCwd
|
||||
}
|
||||
```
|
||||
|
||||
**Exceptions (do NOT reject):**
|
||||
- Default values when validating external input (user input, API responses)
|
||||
- Fallbacks with an explicit comment explaining the reason
|
||||
- Defaults for optional values in configuration files
|
||||
- Defaults for optional values in configuration files (explicitly designed as optional)
|
||||
- Only some callers use default argument (REJECT if all callers omit)
|
||||
|
||||
**Verification approach:**
|
||||
1. Grep the diff for `??`, `||`, `catch`
|
||||
2. Check whether each fallback has a legitimate reason
|
||||
3. REJECT if even one unjustified fallback exists
|
||||
1. Grep the diff for `??`, `||`, `= defaultValue`, `catch`
|
||||
2. For each fallback/default argument:
|
||||
- Is it required data? → REJECT
|
||||
- Do all callers omit it? → REJECT
|
||||
- Is there an upstream path to pass value? → If not, REJECT
|
||||
3. REJECT if even one unjustified fallback/default argument exists
|
||||
|
||||
### 7. Unused Code Detection
|
||||
|
||||
|
||||
@ -8,6 +8,16 @@ Code is read far more often than it is written. Poorly structured code destroys
|
||||
|
||||
"If the structure is right, the code naturally follows"—that is the conviction of design review.
|
||||
|
||||
## Reviewer Stance
|
||||
|
||||
**Never defer even minor issues. If a problem can be fixed now, require it to be fixed now.**
|
||||
|
||||
- No compromises for "minor issues". Accumulation of small problems becomes technical debt
|
||||
- "Address in next task" never happens. If fixable now, fix now
|
||||
- No "conditional approval". If there are issues, reject
|
||||
- If you find in-scope fixable issues, flag them without exception
|
||||
- Existing issues (unrelated to current change) are non-blocking, but issues introduced or fixable in this change must be flagged
|
||||
|
||||
## Areas of Expertise
|
||||
|
||||
### Structure & Design
|
||||
|
||||
@ -2,6 +2,21 @@
|
||||
|
||||
You are the implementer. **Focus on implementation, not design decisions.**
|
||||
|
||||
## Coding Stance
|
||||
|
||||
**Thoroughness over speed. Code correctness over implementation ease.**
|
||||
|
||||
- Don't hide uncertainty with fallback values (`?? 'unknown'`)
|
||||
- Don't obscure data flow with default arguments
|
||||
- Prioritize "works correctly" over "works for now"
|
||||
- Don't swallow errors; fail fast (Fail Fast)
|
||||
- Don't guess; report unclear points
|
||||
|
||||
**Be aware of AI's bad habits:**
|
||||
- Hiding uncertainty with fallbacks → Prohibited (will be flagged in review)
|
||||
- Writing unused code "just in case" → Prohibited (will be flagged in review)
|
||||
- Making design decisions arbitrarily → Report and ask for guidance
|
||||
|
||||
## Most Important Rule
|
||||
|
||||
**Work only within the specified project directory.**
|
||||
@ -107,6 +122,66 @@ Perform self-check after implementation.
|
||||
| Boy Scout | Leave touched areas slightly improved |
|
||||
| Fail Fast | Detect errors early. Don't swallow them |
|
||||
|
||||
## Fallback & Default Argument Prohibition
|
||||
|
||||
**Don't write code that obscures data flow. Code where you can't tell values without tracing logic is bad code.**
|
||||
|
||||
### Prohibited Patterns
|
||||
|
||||
| Pattern | Example | Problem |
|
||||
|---------|---------|---------|
|
||||
| Fallback for required data | `user?.id ?? 'unknown'` | Processing continues in an error state |
|
||||
| Default argument abuse | `function f(x = 'default')` where all callers omit | Can't tell where value comes from |
|
||||
| Nullish coalescing with no upstream path | `options?.cwd ?? process.cwd()` with no way to pass | Always uses fallback (meaningless) |
|
||||
| try-catch returning empty | `catch { return ''; }` | Swallows errors |
|
||||
|
||||
### Correct Implementation
|
||||
|
||||
```typescript
|
||||
// ❌ Prohibited - Fallback for required data
|
||||
const userId = user?.id ?? 'unknown'
|
||||
processUser(userId) // Continues with 'unknown'
|
||||
|
||||
// ✅ Correct - Fail Fast
|
||||
if (!user?.id) {
|
||||
throw new Error('User ID is required')
|
||||
}
|
||||
processUser(user.id)
|
||||
|
||||
// ❌ Prohibited - Default argument with all callers omitting
|
||||
function loadConfig(path = './config.json') { ... }
|
||||
// All callers: loadConfig() ← not passing path
|
||||
|
||||
// ✅ Correct - Required argument with explicit passing
|
||||
function loadConfig(path: string) { ... }
|
||||
// Caller: loadConfig('./config.json') ← Explicit
|
||||
|
||||
// ❌ Prohibited - Nullish coalescing with no upstream path
|
||||
class Engine {
|
||||
constructor(config, options?) {
|
||||
this.cwd = options?.cwd ?? process.cwd()
|
||||
// Problem: If no path to pass options.cwd, always uses process.cwd()
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ Correct - Allow passing from upstream
|
||||
function createEngine(config, cwd: string) {
|
||||
return new Engine(config, { cwd })
|
||||
}
|
||||
```
|
||||
|
||||
### Allowed Cases
|
||||
|
||||
- Default values when validating external input (user input, API responses)
|
||||
- Optional values in configuration files (explicitly designed as optional)
|
||||
- Only some callers use default argument (prohibited if all callers omit)
|
||||
|
||||
### Decision Criteria
|
||||
|
||||
1. **Is it required data?** → Don't fallback, throw error
|
||||
2. **Do all callers omit it?** → Remove default argument, make it required
|
||||
3. **Is there an upstream path to pass value?** → If not, add argument/field
|
||||
|
||||
## Abstraction Principles
|
||||
|
||||
**Before adding conditional branches, consider:**
|
||||
|
||||
@ -74,9 +74,6 @@ steps:
|
||||
- {Question 2}
|
||||
pass_previous_response: true
|
||||
instruction_template: |
|
||||
## Previous Response (when returned from implement)
|
||||
{previous_response}
|
||||
|
||||
Analyze the task and create an implementation plan.
|
||||
|
||||
**Note:** If returned from implement step (Previous Response exists),
|
||||
@ -212,10 +209,7 @@ steps:
|
||||
- condition: Cannot proceed, insufficient info
|
||||
next: plan
|
||||
instruction_template: |
|
||||
## AI Review Feedback (This is the latest instruction - prioritize this)
|
||||
{previous_response}
|
||||
|
||||
**Important**: Address the AI Reviewer's feedback.
|
||||
Address the AI Reviewer's feedback.
|
||||
Focus on:
|
||||
- Correcting incorrect assumptions
|
||||
- Fixing plausible-but-wrong implementations
|
||||
@ -234,7 +228,7 @@ steps:
|
||||
```markdown
|
||||
# Architecture Review
|
||||
|
||||
## Result: APPROVE / IMPROVE / REJECT
|
||||
## Result: APPROVE / REJECT
|
||||
|
||||
## Summary
|
||||
{1-2 sentences summarizing result}
|
||||
@ -245,17 +239,18 @@ steps:
|
||||
- [x] Change Scope
|
||||
|
||||
## Issues (if REJECT)
|
||||
| # | Location | Issue | Fix |
|
||||
|---|----------|-------|-----|
|
||||
| 1 | `src/file.ts:42` | Issue description | Fix method |
|
||||
| # | Scope | Location | Issue | Fix |
|
||||
|---|-------|----------|-------|-----|
|
||||
| 1 | In-scope | `src/file.ts:42` | Issue description | Fix method |
|
||||
|
||||
## Improvement Suggestions (optional, non-blocking)
|
||||
- {Future improvement suggestions}
|
||||
Scope: "In-scope" (fixable now) / "Out-of-scope" (existing issue, non-blocking)
|
||||
|
||||
## Existing Issues (informational, non-blocking)
|
||||
- {Record of existing issues unrelated to current change}
|
||||
```
|
||||
|
||||
**Cognitive load reduction rules:**
|
||||
- APPROVE + no issues -> Summary only (5 lines or less)
|
||||
- APPROVE + minor suggestions -> Summary + suggestions (15 lines or less)
|
||||
- APPROVE -> Summary only (5 lines or less)
|
||||
- REJECT -> Issues in table format (30 lines or less)
|
||||
allowed_tools:
|
||||
- Read
|
||||
@ -347,10 +342,7 @@ steps:
|
||||
- condition: Cannot proceed, insufficient info
|
||||
next: plan
|
||||
instruction_template: |
|
||||
## Review Feedback (This is the latest instruction - prioritize this)
|
||||
{previous_response}
|
||||
|
||||
**Important**: Address the feedback from the reviewers.
|
||||
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
|
||||
|
||||
@ -64,9 +64,6 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## Previous Response (when returned from implement)
|
||||
{previous_response}
|
||||
|
||||
Analyze the task and create an implementation plan.
|
||||
|
||||
**Note:** If returned from implement step (Previous Response exists),
|
||||
@ -146,6 +143,7 @@ steps:
|
||||
- name: ai_review
|
||||
edit: false
|
||||
agent: ../agents/default/ai-antipattern-reviewer.md
|
||||
pass_previous_response: true
|
||||
report:
|
||||
name: 03-ai-review.md
|
||||
format: |
|
||||
@ -206,10 +204,7 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## AI Review Feedback (This is the latest instruction - prioritize this)
|
||||
{previous_response}
|
||||
|
||||
**Important**: Address the AI Reviewer's feedback.
|
||||
Address the AI Reviewer's feedback.
|
||||
Focus on:
|
||||
- Correcting incorrect assumptions
|
||||
- Fixing plausible-but-wrong implementations
|
||||
@ -251,9 +246,14 @@ steps:
|
||||
| Eventual Consistency | ✅ | - |
|
||||
|
||||
## Issues (if REJECT)
|
||||
| # | Location | Issue | Fix |
|
||||
|---|----------|-------|-----|
|
||||
| 1 | `src/file.ts:42` | Issue description | Fix method |
|
||||
| # | Scope | Location | Issue | Fix |
|
||||
|---|-------|----------|-------|-----|
|
||||
| 1 | In-scope | `src/file.ts:42` | Issue description | Fix method |
|
||||
|
||||
Scope: "In-scope" (fixable now) / "Out-of-scope" (existing issue, non-blocking)
|
||||
|
||||
## Existing Issues (informational, non-blocking)
|
||||
- {Record of existing issues unrelated to current change}
|
||||
```
|
||||
allowed_tools:
|
||||
- Read
|
||||
@ -455,10 +455,7 @@ steps:
|
||||
- condition: Cannot proceed, insufficient info
|
||||
next: plan
|
||||
instruction_template: |
|
||||
## Review Feedback (This is the latest instruction - prioritize this)
|
||||
{previous_response}
|
||||
|
||||
**Important**: Address the feedback from the reviewers.
|
||||
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
|
||||
@ -572,10 +569,7 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## Supervisor Feedback (This is the latest instruction - prioritize this)
|
||||
{previous_response}
|
||||
|
||||
**Important**: Fix the issues pointed out by the supervisor.
|
||||
Fix the issues pointed out by the supervisor.
|
||||
|
||||
The supervisor has identified issues from a big-picture perspective.
|
||||
Address items in priority order.
|
||||
|
||||
@ -76,9 +76,6 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## Previous Response (when returned from implement)
|
||||
{previous_response}
|
||||
|
||||
Analyze the task and create an implementation plan.
|
||||
|
||||
**Note:** If returned from implement step (Previous Response exists),
|
||||
@ -158,6 +155,7 @@ steps:
|
||||
- name: ai_review
|
||||
edit: false
|
||||
agent: ../agents/default/ai-antipattern-reviewer.md
|
||||
pass_previous_response: true
|
||||
report:
|
||||
name: 03-ai-review.md
|
||||
format: |
|
||||
@ -218,10 +216,7 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## AI Review Feedback (This is the latest instruction - prioritize this)
|
||||
{previous_response}
|
||||
|
||||
**Important**: Address the AI Reviewer's feedback.
|
||||
Address the AI Reviewer's feedback.
|
||||
Focus on:
|
||||
- Correcting incorrect assumptions
|
||||
- Fixing plausible-but-wrong implementations
|
||||
@ -248,7 +243,7 @@ steps:
|
||||
```markdown
|
||||
# Architecture Review
|
||||
|
||||
## Result: APPROVE / IMPROVE / REJECT
|
||||
## Result: APPROVE / REJECT
|
||||
|
||||
## Summary
|
||||
{1-2 sentences summarizing result}
|
||||
@ -262,17 +257,18 @@ steps:
|
||||
- [x] Call Chain Verification
|
||||
|
||||
## Issues (if REJECT)
|
||||
| # | Location | Issue | Fix |
|
||||
|---|----------|-------|-----|
|
||||
| 1 | `src/file.ts:42` | Issue description | Fix method |
|
||||
| # | Scope | Location | Issue | Fix |
|
||||
|---|-------|----------|-------|-----|
|
||||
| 1 | In-scope | `src/file.ts:42` | Issue description | Fix method |
|
||||
|
||||
## Improvement Suggestions (optional - non-blocking)
|
||||
- {Future improvement suggestions}
|
||||
Scope: "In-scope" (fixable now) / "Out-of-scope" (existing issue, non-blocking)
|
||||
|
||||
## Existing Issues (informational, non-blocking)
|
||||
- {Record of existing issues unrelated to current change}
|
||||
```
|
||||
|
||||
**Cognitive load reduction rules:**
|
||||
- APPROVE + no issues -> Summary only (5 lines or less)
|
||||
- APPROVE + minor suggestions -> Summary + suggestions (15 lines or less)
|
||||
- APPROVE -> Summary only (5 lines or less)
|
||||
- REJECT -> Issues in table format (30 lines or less)
|
||||
allowed_tools:
|
||||
- Read
|
||||
@ -471,10 +467,7 @@ steps:
|
||||
- condition: Cannot proceed, insufficient info
|
||||
next: plan
|
||||
instruction_template: |
|
||||
## Review Feedback (This is the latest instruction - prioritize this)
|
||||
{previous_response}
|
||||
|
||||
**Important**: Address the feedback from the reviewers.
|
||||
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
|
||||
@ -588,10 +581,7 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## Supervisor Feedback (This is the latest instruction - prioritize this)
|
||||
{previous_response}
|
||||
|
||||
**Important**: Fix the issues pointed out by the supervisor.
|
||||
Fix the issues pointed out by the supervisor.
|
||||
|
||||
The supervisor has identified issues from a big-picture perspective.
|
||||
Address items in priority order.
|
||||
|
||||
@ -128,26 +128,70 @@ AIは自信を持って間違える——もっともらしく見えるが動か
|
||||
2. 公開モジュール（index ファイル等）のエクスポート一覧と実体が一致しているか確認
|
||||
3. 新規追加されたコードに対応する古いコードが残っていないか確認
|
||||
|
||||
### 7. フォールバック禁止レビュー（REJECT基準）
|
||||
### 7. フォールバック・デフォルト引数禁止レビュー（REJECT基準）
|
||||
|
||||
**AIは不確実性を隠すためにフォールバックを多用する。これは原則REJECT。**
|
||||
**AIは不確実性を隠すためにフォールバックやデフォルト引数を多用する。値の流れが不明瞭になるため、原則REJECT。**
|
||||
|
||||
| パターン | 例 | 判定 |
|
||||
|---------|-----|------|
|
||||
| デフォルト値で握りつぶし | `?? 'unknown'`、`\|\| 'default'`、`?? []` | REJECT |
|
||||
| try-catch で空値返却 | `catch { return ''; }` `catch { return 0; }` | REJECT |
|
||||
| 条件分岐でサイレント無視 | `if (!x) return;` で本来エラーの状況をスキップ | REJECT |
|
||||
| 多段フォールバック | `a ?? b ?? c ?? d` | REJECT |
|
||||
**問題の本質:** ロジックを追わないと何の値が来るか分からない「ハックコード」になる。
|
||||
|
||||
| パターン | 例 | 問題 | 判定 |
|
||||
|---------|-----|------|------|
|
||||
| 必須データへのフォールバック | `user?.id ?? 'unknown'` | 本来エラーの状態で処理が進む | **REJECT** |
|
||||
| デフォルト引数の濫用 | `function f(x = 'default')` で全呼び出し元が省略 | 値の流れが不明瞭 | **REJECT** |
|
||||
| null合体で渡す口がない | `options?.cwd ?? process.cwd()` で options に渡す経路なし | 常にフォールバックになる | **REJECT** |
|
||||
| try-catch で空値返却 | `catch { return ''; }` | エラーを握りつぶす | **REJECT** |
|
||||
| 多段フォールバック | `a ?? b ?? c ?? d` | 値の決定ロジックが複雑 | **REJECT** |
|
||||
| 条件分岐でサイレント無視 | `if (!x) return;` で本来エラーをスキップ | バグを隠蔽 | **REJECT** |
|
||||
|
||||
**デフォルト引数の具体例:**
|
||||
|
||||
```typescript
|
||||
// ❌ 悪い例 - 全呼び出し元が省略している
|
||||
function loadWorkflow(name: string, cwd = process.cwd()) { ... }
|
||||
// 全呼び出し元: loadWorkflow('default') ← cwd を渡していない
|
||||
// 問題: cwd の値がどこから来るか、呼び出し元を見ても分からない
|
||||
// 修正: cwd を必須引数にし、呼び出し元で明示的に渡す
|
||||
|
||||
// ✅ 良い例 - 一部の呼び出し元のみ省略
|
||||
function query(sql: string, timeout = 30000) { ... }
|
||||
// 呼び出し元A: query(sql) ← デフォルト使用
|
||||
// 呼び出し元B: query(sql, 60000) ← 明示的に指定
|
||||
// 理由: timeout は明示的にオプショナルな設定値
|
||||
```
|
||||
|
||||
**null合体演算子の具体例:**
|
||||
|
||||
```typescript
|
||||
// ❌ 悪い例 - 上位から値を渡す口がない
|
||||
class Engine {
|
||||
constructor(config, cwd, task, options?) {
|
||||
this.projectCwd = options?.projectCwd ?? cwd
|
||||
// 問題: options が { } で渡され、projectCwd が常に undefined
|
||||
// 結果、常に cwd が使われる（フォールバックの意味がない）
|
||||
}
|
||||
}
|
||||
// 修正: 上位の関数シグネチャを修正し、options.projectCwd を渡せるようにする
|
||||
|
||||
// ✅ 良い例 - 上位から値を渡す経路が存在する
|
||||
function execute(task, options?: { projectCwd?: string }) {
|
||||
const cwd = options?.projectCwd ?? process.cwd()
|
||||
// 理由: options.projectCwd を渡すかどうかは呼び出し元の選択
|
||||
}
|
||||
```
|
||||
|
||||
**例外（REJECTしない）:**
|
||||
- 外部入力（ユーザー入力、API応答）のバリデーション時のデフォルト値
|
||||
- 明示的にコメントで理由が記載されているフォールバック
|
||||
- 設定ファイルのオプショナル値に対するデフォルト
|
||||
- 一部の呼び出し元のみがデフォルト引数を使用（全員が省略している場合はREJECT）
|
||||
|
||||
**検証アプローチ:**
|
||||
1. 変更差分で `??`、`||`、`catch` を grep
|
||||
2. 各フォールバックに正当な理由があるか確認
|
||||
3. 理由なしのフォールバックが1つでもあれば REJECT
|
||||
1. 変更差分で `??`、`||`、`= defaultValue`、`catch` を grep
|
||||
2. 各フォールバック・デフォルト引数について:
|
||||
- 必須データか？ → REJECT
|
||||
- 全呼び出し元が省略しているか？ → REJECT
|
||||
- 上位から値を渡す経路があるか？ → なければ REJECT
|
||||
3. 理由なしのフォールバック・デフォルト引数が1つでもあれば REJECT
|
||||
|
||||
### 8. 未使用コードの検出
|
||||
|
||||
|
||||
@ -8,6 +8,16 @@
|
||||
|
||||
「構造が正しければ、コードは自然と正しくなる」——それが設計レビューの信念だ。
|
||||
|
||||
## レビュアーとしてのスタンス
|
||||
|
||||
**軽微な問題でも後に持ち越さない。今修正できる問題は今修正させる。**
|
||||
|
||||
- 「軽微だから許容」という妥協はしない。小さな問題の蓄積が技術的負債になる
|
||||
- 「次のタスクで対応」は実現しない。今修正できるなら今修正する
|
||||
- 「条件付き承認」はしない。問題があれば差し戻す
|
||||
- スコープ内で修正可能な問題を見つけたら、例外なく指摘する
|
||||
- 既存問題（今回の変更と無関係な問題）は非ブロッキングだが、今回の変更で導入された問題や修正可能な問題は必ず指摘する
|
||||
|
||||
## 専門領域
|
||||
|
||||
### 構造・設計
|
||||
|
||||
@ -2,6 +2,21 @@
|
||||
|
||||
あなたは実装担当です。**設計判断はせず、実装に集中**してください。
|
||||
|
||||
## コーディングスタンス
|
||||
|
||||
**速さより丁寧さ。実装の楽さよりコードの正確さ。**
|
||||
|
||||
- フォールバック値（`?? 'unknown'`）で不確実性を隠さない
|
||||
- デフォルト引数で値の流れを不明瞭にしない
|
||||
- 「とりあえず動く」より「正しく動く」を優先
|
||||
- エラーは握りつぶさず、早期に失敗させる（Fail Fast）
|
||||
- 推測で実装せず、不明点は報告する
|
||||
|
||||
**AIの悪い癖を自覚する:**
|
||||
- 不確実なときにフォールバックで隠す → 禁止（レビューで指摘される）
|
||||
- 「念のため」で未使用コードを書く → 禁止（レビューで指摘される）
|
||||
- 設計判断を勝手にする → 報告して判断を仰ぐ
|
||||
|
||||
## 最重要ルール
|
||||
|
||||
**作業は必ず指定されたプロジェクトディレクトリ内で行ってください。**
|
||||
@ -108,6 +123,66 @@
|
||||
| ボーイスカウト | 触った箇所は少し改善して去る |
|
||||
| Fail Fast | エラーは早期に検出。握りつぶさない |
|
||||
|
||||
## フォールバック・デフォルト引数の禁止
|
||||
|
||||
**値の流れを不明瞭にするコードは書かない。ロジックを追わないと値が分からないのは悪いコード。**
|
||||
|
||||
### 禁止パターン
|
||||
|
||||
| パターン | 例 | 問題 |
|
||||
|---------|-----|------|
|
||||
| 必須データへのフォールバック | `user?.id ?? 'unknown'` | エラーになるべき状態で処理が進む |
|
||||
| デフォルト引数の濫用 | `function f(x = 'default')` で全呼び出し元が省略 | 値がどこから来るか分からない |
|
||||
| null合体で渡す口がない | `options?.cwd ?? process.cwd()` で上位から渡す経路なし | 常にフォールバックになる（意味がない） |
|
||||
| try-catch で空値返却 | `catch { return ''; }` | エラーを握りつぶす |
|
||||
|
||||
### 正しい実装
|
||||
|
||||
```typescript
|
||||
// ❌ 禁止 - 必須データへのフォールバック
|
||||
const userId = user?.id ?? 'unknown'
|
||||
processUser(userId) // 'unknown' で処理が進んでしまう
|
||||
|
||||
// ✅ 正しい - Fail Fast
|
||||
if (!user?.id) {
|
||||
throw new Error('User ID is required')
|
||||
}
|
||||
processUser(user.id)
|
||||
|
||||
// ❌ 禁止 - デフォルト引数で全呼び出し元が省略
|
||||
function loadConfig(path = './config.json') { ... }
|
||||
// 全呼び出し元: loadConfig() ← path を渡していない
|
||||
|
||||
// ✅ 正しい - 必須引数にして明示的に渡す
|
||||
function loadConfig(path: string) { ... }
|
||||
// 呼び出し元: loadConfig('./config.json') ← 明示的
|
||||
|
||||
// ❌ 禁止 - null合体で渡す口がない
|
||||
class Engine {
|
||||
constructor(config, options?) {
|
||||
this.cwd = options?.cwd ?? process.cwd()
|
||||
// 問題: options に cwd を渡す経路がない場合、常に process.cwd() になる
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ 正しい - 上位から渡せるようにする
|
||||
function createEngine(config, cwd: string) {
|
||||
return new Engine(config, { cwd })
|
||||
}
|
||||
```
|
||||
|
||||
### 許容されるケース
|
||||
|
||||
- 外部入力（ユーザー入力、API応答）のバリデーション時のデフォルト値
|
||||
- 設定ファイルのオプショナル値（明示的に省略可能と設計されている）
|
||||
- 一部の呼び出し元のみがデフォルト引数を使用（全員が省略している場合は禁止）
|
||||
|
||||
### 判断基準
|
||||
|
||||
1. **必須データか？** → フォールバックせず、エラーにする
|
||||
2. **全呼び出し元が省略しているか？** → デフォルト引数を削除し、必須にする
|
||||
3. **上位から値を渡す経路があるか？** → なければ引数・フィールドを追加
|
||||
|
||||
## 抽象化の原則
|
||||
|
||||
**条件分岐を追加する前に考える:**
|
||||
|
||||
@ -65,9 +65,6 @@ steps:
|
||||
- {質問2}
|
||||
pass_previous_response: true
|
||||
instruction_template: |
|
||||
## Previous Response (implementからの差し戻し時)
|
||||
{previous_response}
|
||||
|
||||
タスクを分析し、実装方針を立ててください。
|
||||
|
||||
**注意:** Previous Responseがある場合は差し戻しのため、
|
||||
@ -208,10 +205,7 @@ steps:
|
||||
- condition: 判断できない、情報不足
|
||||
next: plan
|
||||
instruction_template: |
|
||||
## AI Review Feedback (これが最新の指示です - 優先して対応してください)
|
||||
{previous_response}
|
||||
|
||||
**重要**: AI Reviewerのフィードバックに対応してください。
|
||||
AI Reviewerのフィードバックに対応してください。
|
||||
以下に集中してください:
|
||||
- 間違った仮定の修正
|
||||
- もっともらしいが間違っている実装の修正
|
||||
@ -230,7 +224,7 @@ steps:
|
||||
```markdown
|
||||
# アーキテクチャレビュー
|
||||
|
||||
## 結果: APPROVE / IMPROVE / REJECT
|
||||
## 結果: APPROVE / REJECT
|
||||
|
||||
## サマリー
|
||||
{1-2文で結果を要約}
|
||||
@ -244,17 +238,18 @@ steps:
|
||||
- [x] 呼び出しチェーン検証
|
||||
|
||||
## 問題点（REJECTの場合）
|
||||
| # | 場所 | 問題 | 修正案 |
|
||||
|---|------|------|--------|
|
||||
| 1 | `src/file.ts:42` | 問題の説明 | 修正方法 |
|
||||
| # | スコープ | 場所 | 問題 | 修正案 |
|
||||
|---|---------|------|------|--------|
|
||||
| 1 | スコープ内 | `src/file.ts:42` | 問題の説明 | 修正方法 |
|
||||
|
||||
## 改善提案（任意・ブロッキングではない）
|
||||
- {将来的な改善提案}
|
||||
スコープ: 「スコープ内」（今回修正可能）/ 「スコープ外」（既存問題・非ブロッキング）
|
||||
|
||||
## 既存問題（参考・非ブロッキング）
|
||||
- {既存問題の記録。今回の変更と無関係な問題}
|
||||
```
|
||||
|
||||
**認知負荷軽減ルール:**
|
||||
- APPROVE + 問題なし → サマリーのみ（5行以内）
|
||||
- APPROVE + 軽微な提案 → サマリー + 改善提案（15行以内）
|
||||
- APPROVE → サマリーのみ（5行以内）
|
||||
- REJECT → 問題点を表形式で（30行以内）
|
||||
allowed_tools:
|
||||
- Read
|
||||
@ -354,10 +349,7 @@ steps:
|
||||
- condition: 判断できない、情報不足
|
||||
next: plan
|
||||
instruction_template: |
|
||||
## Review Feedback (これが最新の指示です - 優先して対応してください)
|
||||
{previous_response}
|
||||
|
||||
**重要**: レビュアーのフィードバックに対応してください。
|
||||
レビュアーのフィードバックに対応してください。
|
||||
セッションの会話履歴を確認し、レビュアーの指摘事項を修正してください。
|
||||
pass_previous_response: true
|
||||
|
||||
|
||||
@ -73,9 +73,6 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## Previous Response (implementからの差し戻し時)
|
||||
{previous_response}
|
||||
|
||||
タスクを分析し、実装方針を立ててください。
|
||||
|
||||
**注意:** Previous Responseがある場合は差し戻しのため、
|
||||
@ -155,6 +152,7 @@ steps:
|
||||
- name: ai_review
|
||||
edit: false
|
||||
agent: ../agents/default/ai-antipattern-reviewer.md
|
||||
pass_previous_response: true
|
||||
report:
|
||||
name: 03-ai-review.md
|
||||
format: |
|
||||
@ -215,10 +213,7 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## AI Review Feedback (これが最新の指示です - 優先して対応してください)
|
||||
{previous_response}
|
||||
|
||||
**重要**: AI Reviewerのフィードバックに対応してください。
|
||||
AI Reviewerのフィードバックに対応してください。
|
||||
以下に集中してください:
|
||||
- 間違った仮定の修正
|
||||
- もっともらしいが間違っている実装の修正
|
||||
@ -260,9 +255,14 @@ steps:
|
||||
| 結果整合性 | ✅ | - |
|
||||
|
||||
## 問題点（REJECTの場合）
|
||||
| # | 場所 | 問題 | 修正案 |
|
||||
|---|------|------|--------|
|
||||
| 1 | `src/file.ts:42` | 問題の説明 | 修正方法 |
|
||||
| # | スコープ | 場所 | 問題 | 修正案 |
|
||||
|---|---------|------|------|--------|
|
||||
| 1 | スコープ内 | `src/file.ts:42` | 問題の説明 | 修正方法 |
|
||||
|
||||
スコープ: 「スコープ内」（今回修正可能）/ 「スコープ外」（既存問題・非ブロッキング）
|
||||
|
||||
## 既存問題（参考・非ブロッキング）
|
||||
- {既存問題の記録。今回の変更と無関係な問題}
|
||||
```
|
||||
allowed_tools:
|
||||
- Read
|
||||
@ -464,10 +464,7 @@ steps:
|
||||
- condition: 修正を進行できない
|
||||
next: plan
|
||||
instruction_template: |
|
||||
## Review Feedback (これが最新の指示です - 優先して対応してください)
|
||||
{previous_response}
|
||||
|
||||
**重要**: レビュアーからのフィードバックに対応してください。
|
||||
レビュアーからのフィードバックに対応してください。
|
||||
「Original User Request」は参考情報であり、最新の指示ではありません。
|
||||
セッションの会話履歴を確認し、レビュアーの指摘事項を修正してください。
|
||||
pass_previous_response: true
|
||||
@ -581,10 +578,7 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## Supervisor Feedback (これが最新の指示です - 優先して対応してください)
|
||||
{previous_response}
|
||||
|
||||
**重要**: 監督者からの指摘を修正してください。
|
||||
監督者からの指摘を修正してください。
|
||||
|
||||
監督者は全体を俯瞰した視点から問題を指摘しています。
|
||||
優先度の高い項目から順に対応してください。
|
||||
|
||||
@ -64,9 +64,6 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## Previous Response (implementからの差し戻し時)
|
||||
{previous_response}
|
||||
|
||||
タスクを分析し、実装方針を立ててください。
|
||||
|
||||
**注意:** Previous Responseがある場合は差し戻しのため、
|
||||
@ -146,6 +143,7 @@ steps:
|
||||
- name: ai_review
|
||||
edit: false
|
||||
agent: ../agents/default/ai-antipattern-reviewer.md
|
||||
pass_previous_response: true
|
||||
report:
|
||||
name: 03-ai-review.md
|
||||
format: |
|
||||
@ -206,10 +204,7 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## AI Review Feedback (これが最新の指示です - 優先して対応してください)
|
||||
{previous_response}
|
||||
|
||||
**重要**: AI Reviewerのフィードバックに対応してください。
|
||||
AI Reviewerのフィードバックに対応してください。
|
||||
以下に集中してください:
|
||||
- 間違った仮定の修正
|
||||
- もっともらしいが間違っている実装の修正
|
||||
@ -236,7 +231,7 @@ steps:
|
||||
```markdown
|
||||
# アーキテクチャレビュー
|
||||
|
||||
## 結果: APPROVE / IMPROVE / REJECT
|
||||
## 結果: APPROVE / REJECT
|
||||
|
||||
## サマリー
|
||||
{1-2文で結果を要約}
|
||||
@ -250,17 +245,18 @@ steps:
|
||||
- [x] 呼び出しチェーン検証
|
||||
|
||||
## 問題点（REJECTの場合）
|
||||
| # | 場所 | 問題 | 修正案 |
|
||||
|---|------|------|--------|
|
||||
| 1 | `src/file.ts:42` | 問題の説明 | 修正方法 |
|
||||
| # | スコープ | 場所 | 問題 | 修正案 |
|
||||
|---|---------|------|------|--------|
|
||||
| 1 | スコープ内 | `src/file.ts:42` | 問題の説明 | 修正方法 |
|
||||
|
||||
## 改善提案（任意・ブロッキングではない）
|
||||
- {将来的な改善提案}
|
||||
スコープ: 「スコープ内」（今回修正可能）/ 「スコープ外」（既存問題・非ブロッキング）
|
||||
|
||||
## 既存問題（参考・非ブロッキング）
|
||||
- {既存問題の記録。今回の変更と無関係な問題}
|
||||
```
|
||||
|
||||
**認知負荷軽減ルール:**
|
||||
- APPROVE + 問題なし → サマリーのみ（5行以内）
|
||||
- APPROVE + 軽微な提案 → サマリー + 改善提案（15行以内）
|
||||
- APPROVE → サマリーのみ（5行以内）
|
||||
- REJECT → 問題点を表形式で（30行以内）
|
||||
allowed_tools:
|
||||
- Read
|
||||
@ -459,10 +455,7 @@ steps:
|
||||
- condition: 修正を進行できない
|
||||
next: plan
|
||||
instruction_template: |
|
||||
## Review Feedback (これが最新の指示です - 優先して対応してください)
|
||||
{previous_response}
|
||||
|
||||
**重要**: レビュアーからのフィードバックに対応してください。
|
||||
レビュアーからのフィードバックに対応してください。
|
||||
「Original User Request」は参考情報であり、最新の指示ではありません。
|
||||
セッションの会話履歴を確認し、レビュアーの指摘事項を修正してください。
|
||||
pass_previous_response: true
|
||||
@ -576,10 +569,7 @@ steps:
|
||||
- WebSearch
|
||||
- WebFetch
|
||||
instruction_template: |
|
||||
## Supervisor Feedback (これが最新の指示です - 優先して対応してください)
|
||||
{previous_response}
|
||||
|
||||
**重要**: 監督者からの指摘を修正してください。
|
||||
監督者からの指摘を修正してください。
|
||||
|
||||
監督者は全体を俯瞰した視点から問題を指摘しています。
|
||||
優先度の高い項目から順に対応してください。
|
||||
|
||||
@ -174,8 +174,7 @@ describe('loadAllWorkflows', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it.skip('should only load workflows from global ~/.takt/workflows/ (not project-local)', () => {
|
||||
// Project-local workflows should NOT be loaded anymore
|
||||
it('should load project-local workflows when cwd is provided', () => {
|
||||
const workflowsDir = join(testDir, '.takt', 'workflows');
|
||||
mkdirSync(workflowsDir, { recursive: true });
|
||||
|
||||
@ -193,10 +192,9 @@ steps:
|
||||
`;
|
||||
writeFileSync(join(workflowsDir, 'test.yaml'), sampleWorkflow);
|
||||
|
||||
const workflows = loadAllWorkflows();
|
||||
const workflows = loadAllWorkflows(testDir);
|
||||
|
||||
// Project-local workflow should NOT be loaded
|
||||
expect(workflows.has('test')).toBe(false);
|
||||
expect(workflows.has('test')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@ -224,22 +222,48 @@ describe('loadWorkflow (builtin fallback)', () => {
|
||||
});
|
||||
|
||||
describe('listWorkflows (builtin fallback)', () => {
|
||||
let testDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
testDir = join(tmpdir(), `takt-test-${randomUUID()}`);
|
||||
mkdirSync(testDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (existsSync(testDir)) {
|
||||
rmSync(testDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('should include builtin workflows', () => {
|
||||
const workflows = listWorkflows();
|
||||
const workflows = listWorkflows(testDir);
|
||||
expect(workflows).toContain('default');
|
||||
expect(workflows).toContain('simple');
|
||||
});
|
||||
|
||||
it('should return sorted list', () => {
|
||||
const workflows = listWorkflows();
|
||||
const workflows = listWorkflows(testDir);
|
||||
const sorted = [...workflows].sort();
|
||||
expect(workflows).toEqual(sorted);
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadAllWorkflows (builtin fallback)', () => {
|
||||
let testDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
testDir = join(tmpdir(), `takt-test-${randomUUID()}`);
|
||||
mkdirSync(testDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (existsSync(testDir)) {
|
||||
rmSync(testDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('should include builtin workflows in the map', () => {
|
||||
const workflows = loadAllWorkflows();
|
||||
const workflows = loadAllWorkflows(testDir);
|
||||
expect(workflows.has('default')).toBe(true);
|
||||
expect(workflows.has('simple')).toBe(true);
|
||||
});
|
||||
|
||||
@ -150,8 +150,7 @@ describe('executePipeline', () => {
|
||||
'Fix the bug',
|
||||
'/tmp/test',
|
||||
'default',
|
||||
false,
|
||||
undefined,
|
||||
'/tmp/test',
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
@ -173,8 +172,7 @@ describe('executePipeline', () => {
|
||||
'Fix the bug',
|
||||
'/tmp/test',
|
||||
'default',
|
||||
false,
|
||||
undefined,
|
||||
'/tmp/test',
|
||||
{ provider: 'codex', model: 'codex-model' },
|
||||
);
|
||||
});
|
||||
@ -231,8 +229,7 @@ describe('executePipeline', () => {
|
||||
'From --task flag',
|
||||
'/tmp/test',
|
||||
'magi',
|
||||
false,
|
||||
undefined,
|
||||
'/tmp/test',
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
@ -392,8 +389,7 @@ describe('executePipeline', () => {
|
||||
'Fix the bug',
|
||||
'/tmp/test',
|
||||
'default',
|
||||
false,
|
||||
undefined,
|
||||
'/tmp/test',
|
||||
undefined,
|
||||
);
|
||||
|
||||
|
||||
@ -6,7 +6,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
// Mock dependencies before importing the module under test
|
||||
vi.mock('../config/index.js', () => ({
|
||||
loadWorkflow: vi.fn(),
|
||||
loadWorkflowByIdentifier: vi.fn(),
|
||||
isWorkflowPath: vi.fn(() => false),
|
||||
loadGlobalConfig: vi.fn(() => ({})),
|
||||
}));
|
||||
|
||||
|
||||
@ -1,189 +0,0 @@
|
||||
/**
|
||||
* Tests for path-based workflow loading
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir, homedir } from 'node:os';
|
||||
import { loadWorkflow, loadWorkflowFromPath } from '../config/workflowLoader.js';
|
||||
|
||||
describe('Path-based workflow loading', () => {
|
||||
let tempDir: string;
|
||||
let projectDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
// Create temporary directories for testing
|
||||
tempDir = mkdtempSync(join(tmpdir(), 'takt-test-'));
|
||||
projectDir = mkdtempSync(join(tmpdir(), 'takt-project-'));
|
||||
|
||||
// Create a test workflow in temp directory
|
||||
writeFileSync(
|
||||
join(tempDir, 'test-workflow.yaml'),
|
||||
`name: test-path-workflow
|
||||
description: Test workflow for path-based loading
|
||||
initial_step: plan
|
||||
max_iterations: 5
|
||||
|
||||
steps:
|
||||
- name: plan
|
||||
agent: planner
|
||||
instruction: "Plan the task"
|
||||
rules:
|
||||
- condition: ai("Ready?")
|
||||
next: implement
|
||||
|
||||
- name: implement
|
||||
agent: coder
|
||||
instruction: "Implement"
|
||||
`,
|
||||
);
|
||||
|
||||
// Create project-local workflow directory
|
||||
const projectWorkflowsDir = join(projectDir, '.takt', 'workflows');
|
||||
rmSync(projectWorkflowsDir, { recursive: true, force: true });
|
||||
writeFileSync(
|
||||
join(tempDir, 'project-local.yaml'),
|
||||
`name: project-local-workflow
|
||||
description: Project-local workflow
|
||||
initial_step: test
|
||||
max_iterations: 1
|
||||
|
||||
steps:
|
||||
- name: test
|
||||
agent: tester
|
||||
instruction: "Run tests"
|
||||
`,
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Clean up temporary directories
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
rmSync(projectDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('should load workflow by absolute path', () => {
|
||||
const absolutePath = join(tempDir, 'test-workflow.yaml');
|
||||
const workflow = loadWorkflowFromPath(absolutePath);
|
||||
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('test-path-workflow');
|
||||
expect(workflow!.description).toBe('Test workflow for path-based loading');
|
||||
});
|
||||
|
||||
it('should load workflow by relative path', () => {
|
||||
const originalCwd = process.cwd();
|
||||
try {
|
||||
process.chdir(tempDir);
|
||||
const relativePath = './test-workflow.yaml';
|
||||
const workflow = loadWorkflowFromPath(relativePath, tempDir);
|
||||
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('test-path-workflow');
|
||||
} finally {
|
||||
process.chdir(originalCwd);
|
||||
}
|
||||
});
|
||||
|
||||
it('should load workflow with .yaml extension in name', () => {
|
||||
const pathWithExtension = join(tempDir, 'test-workflow.yaml');
|
||||
const workflow = loadWorkflowFromPath(pathWithExtension);
|
||||
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('test-path-workflow');
|
||||
});
|
||||
|
||||
it('should return null for non-existent path', () => {
|
||||
const nonExistentPath = join(tempDir, 'non-existent.yaml');
|
||||
const workflow = loadWorkflowFromPath(nonExistentPath);
|
||||
|
||||
expect(workflow).toBeNull();
|
||||
});
|
||||
|
||||
it('should maintain backward compatibility with name-based loading', () => {
|
||||
// Load builtin workflow by name
|
||||
const workflow = loadWorkflow('default');
|
||||
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('default');
|
||||
});
|
||||
|
||||
it('should prioritize project-local workflows over global when loading by name', () => {
|
||||
// Create project-local workflow directory
|
||||
const projectWorkflowsDir = join(projectDir, '.takt', 'workflows');
|
||||
mkdirSync(projectWorkflowsDir, { recursive: true });
|
||||
|
||||
// Create project-local workflow with same name as builtin
|
||||
writeFileSync(
|
||||
join(projectWorkflowsDir, 'default.yaml'),
|
||||
`name: project-override
|
||||
description: Project-local override of default workflow
|
||||
initial_step: custom
|
||||
max_iterations: 1
|
||||
|
||||
steps:
|
||||
- name: custom
|
||||
agent: custom
|
||||
instruction: "Custom step"
|
||||
`,
|
||||
);
|
||||
|
||||
// Load by name with projectCwd - should get project-local version
|
||||
const workflow = loadWorkflow('default', projectDir);
|
||||
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('project-override');
|
||||
expect(workflow!.description).toBe('Project-local override of default workflow');
|
||||
});
|
||||
|
||||
it('should load workflows via loadWorkflowFromPath function', () => {
|
||||
// Absolute paths
|
||||
const pathWithSlash = join(tempDir, 'test-workflow.yaml');
|
||||
const workflow1 = loadWorkflowFromPath(pathWithSlash);
|
||||
expect(workflow1).not.toBeNull();
|
||||
|
||||
// Relative paths
|
||||
const workflow2 = loadWorkflowFromPath('./test-workflow.yaml', tempDir);
|
||||
expect(workflow2).not.toBeNull();
|
||||
|
||||
// Explicit path loading
|
||||
const yamlFile = join(tempDir, 'test-workflow.yaml');
|
||||
const workflow3 = loadWorkflowFromPath(yamlFile);
|
||||
expect(workflow3).not.toBeNull();
|
||||
});
|
||||
|
||||
it('should handle workflow files with .yml extension', () => {
|
||||
// Create workflow with .yml extension
|
||||
const ymlPath = join(tempDir, 'test-yml.yml');
|
||||
writeFileSync(
|
||||
ymlPath,
|
||||
`name: yml-workflow
|
||||
description: Workflow with .yml extension
|
||||
initial_step: start
|
||||
max_iterations: 1
|
||||
|
||||
steps:
|
||||
- name: start
|
||||
agent: starter
|
||||
instruction: "Start"
|
||||
`,
|
||||
);
|
||||
|
||||
const workflow = loadWorkflowFromPath(ymlPath);
|
||||
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('yml-workflow');
|
||||
});
|
||||
|
||||
it('should resolve relative paths against provided base directory', () => {
|
||||
const relativePath = 'test-workflow.yaml';
|
||||
const workflow = loadWorkflowFromPath(relativePath, tempDir);
|
||||
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('test-path-workflow');
|
||||
});
|
||||
});
|
||||
|
||||
// Import for test setup
|
||||
import { mkdirSync } from 'node:fs';
|
||||
189
src/__tests__/workflowLoader.test.ts
Normal file
189
src/__tests__/workflowLoader.test.ts
Normal file
@ -0,0 +1,189 @@
|
||||
/**
|
||||
* Tests for isWorkflowPath and loadWorkflowByIdentifier
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import {
|
||||
isWorkflowPath,
|
||||
loadWorkflowByIdentifier,
|
||||
listWorkflows,
|
||||
loadAllWorkflows,
|
||||
} from '../config/workflowLoader.js';
|
||||
|
||||
const SAMPLE_WORKFLOW = `name: test-workflow
|
||||
description: Test workflow
|
||||
initial_step: step1
|
||||
max_iterations: 1
|
||||
|
||||
steps:
|
||||
- name: step1
|
||||
agent: coder
|
||||
instruction: "{task}"
|
||||
`;
|
||||
|
||||
describe('isWorkflowPath', () => {
|
||||
it('should return true for absolute paths', () => {
|
||||
expect(isWorkflowPath('/path/to/workflow.yaml')).toBe(true);
|
||||
expect(isWorkflowPath('/workflow')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for home directory paths', () => {
|
||||
expect(isWorkflowPath('~/workflow.yaml')).toBe(true);
|
||||
expect(isWorkflowPath('~/.takt/workflows/custom.yaml')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for relative paths starting with ./', () => {
|
||||
expect(isWorkflowPath('./workflow.yaml')).toBe(true);
|
||||
expect(isWorkflowPath('./subdir/workflow.yaml')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for relative paths starting with ../', () => {
|
||||
expect(isWorkflowPath('../workflow.yaml')).toBe(true);
|
||||
expect(isWorkflowPath('../subdir/workflow.yaml')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for paths ending with .yaml', () => {
|
||||
expect(isWorkflowPath('custom.yaml')).toBe(true);
|
||||
expect(isWorkflowPath('my-workflow.yaml')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for paths ending with .yml', () => {
|
||||
expect(isWorkflowPath('custom.yml')).toBe(true);
|
||||
expect(isWorkflowPath('my-workflow.yml')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for plain workflow names', () => {
|
||||
expect(isWorkflowPath('default')).toBe(false);
|
||||
expect(isWorkflowPath('simple')).toBe(false);
|
||||
expect(isWorkflowPath('magi')).toBe(false);
|
||||
expect(isWorkflowPath('my-custom-workflow')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadWorkflowByIdentifier', () => {
|
||||
let tempDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = mkdtempSync(join(tmpdir(), 'takt-test-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('should load workflow by name (builtin)', () => {
|
||||
const workflow = loadWorkflowByIdentifier('default');
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('default');
|
||||
});
|
||||
|
||||
it('should load workflow by absolute path', () => {
|
||||
const filePath = join(tempDir, 'test.yaml');
|
||||
writeFileSync(filePath, SAMPLE_WORKFLOW);
|
||||
|
||||
const workflow = loadWorkflowByIdentifier(filePath, tempDir);
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('test-workflow');
|
||||
});
|
||||
|
||||
it('should load workflow by relative path', () => {
|
||||
const filePath = join(tempDir, 'test.yaml');
|
||||
writeFileSync(filePath, SAMPLE_WORKFLOW);
|
||||
|
||||
const workflow = loadWorkflowByIdentifier('./test.yaml', tempDir);
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('test-workflow');
|
||||
});
|
||||
|
||||
it('should load workflow by filename with .yaml extension', () => {
|
||||
const filePath = join(tempDir, 'test.yaml');
|
||||
writeFileSync(filePath, SAMPLE_WORKFLOW);
|
||||
|
||||
const workflow = loadWorkflowByIdentifier('test.yaml', tempDir);
|
||||
expect(workflow).not.toBeNull();
|
||||
expect(workflow!.name).toBe('test-workflow');
|
||||
});
|
||||
|
||||
it('should return null for non-existent name', () => {
|
||||
const workflow = loadWorkflowByIdentifier('non-existent-workflow-xyz');
|
||||
expect(workflow).toBeNull();
|
||||
});
|
||||
|
||||
it('should return null for non-existent path', () => {
|
||||
const workflow = loadWorkflowByIdentifier('./non-existent.yaml', tempDir);
|
||||
expect(workflow).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('listWorkflows with project-local', () => {
|
||||
let tempDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = mkdtempSync(join(tmpdir(), 'takt-test-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('should include project-local workflows when cwd is provided', () => {
|
||||
const projectWorkflowsDir = join(tempDir, '.takt', 'workflows');
|
||||
mkdirSync(projectWorkflowsDir, { recursive: true });
|
||||
writeFileSync(join(projectWorkflowsDir, 'project-custom.yaml'), SAMPLE_WORKFLOW);
|
||||
|
||||
const workflows = listWorkflows(tempDir);
|
||||
expect(workflows).toContain('project-custom');
|
||||
});
|
||||
|
||||
it('should include builtin workflows regardless of cwd', () => {
|
||||
const workflows = listWorkflows(tempDir);
|
||||
expect(workflows).toContain('default');
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
describe('loadAllWorkflows with project-local', () => {
|
||||
let tempDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = mkdtempSync(join(tmpdir(), 'takt-test-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('should include project-local workflows when cwd is provided', () => {
|
||||
const projectWorkflowsDir = join(tempDir, '.takt', 'workflows');
|
||||
mkdirSync(projectWorkflowsDir, { recursive: true });
|
||||
writeFileSync(join(projectWorkflowsDir, 'project-custom.yaml'), SAMPLE_WORKFLOW);
|
||||
|
||||
const workflows = loadAllWorkflows(tempDir);
|
||||
expect(workflows.has('project-custom')).toBe(true);
|
||||
expect(workflows.get('project-custom')!.name).toBe('test-workflow');
|
||||
});
|
||||
|
||||
it('should have project-local override builtin when same name', () => {
|
||||
const projectWorkflowsDir = join(tempDir, '.takt', 'workflows');
|
||||
mkdirSync(projectWorkflowsDir, { recursive: true });
|
||||
|
||||
const overrideWorkflow = `name: project-override
|
||||
description: Project override
|
||||
initial_step: step1
|
||||
max_iterations: 1
|
||||
|
||||
steps:
|
||||
- name: step1
|
||||
agent: coder
|
||||
instruction: "{task}"
|
||||
`;
|
||||
writeFileSync(join(projectWorkflowsDir, 'default.yaml'), overrideWorkflow);
|
||||
|
||||
const workflows = loadAllWorkflows(tempDir);
|
||||
expect(workflows.get('default')!.name).toBe('project-override');
|
||||
});
|
||||
|
||||
});
|
||||
76
src/cli.ts
76
src/cli.ts
@ -41,7 +41,7 @@ import {
|
||||
interactiveMode,
|
||||
executePipeline,
|
||||
} from './commands/index.js';
|
||||
import { listWorkflows } from './config/workflowLoader.js';
|
||||
import { listWorkflows, isWorkflowPath } from './config/workflowLoader.js';
|
||||
import { selectOptionWithDefault, confirm } from './prompt/index.js';
|
||||
import { createSharedClone } from './task/clone.js';
|
||||
import { autoCommitAndPush } from './task/autoCommit.js';
|
||||
@ -77,7 +77,7 @@ export interface WorktreeConfirmationResult {
|
||||
* Returns the selected workflow name, or null if cancelled.
|
||||
*/
|
||||
async function selectWorkflow(cwd: string): Promise<string | null> {
|
||||
const availableWorkflows = listWorkflows();
|
||||
const availableWorkflows = listWorkflows(cwd);
|
||||
const currentWorkflow = getCurrentWorkflow(cwd);
|
||||
|
||||
if (availableWorkflows.length === 0) {
|
||||
@ -111,7 +111,6 @@ export interface SelectAndExecuteOptions {
|
||||
autoPr?: boolean;
|
||||
repo?: string;
|
||||
workflow?: string;
|
||||
workflowPath?: string;
|
||||
createWorktree?: boolean | undefined;
|
||||
}
|
||||
|
||||
@ -121,22 +120,7 @@ async function selectAndExecuteTask(
|
||||
options?: SelectAndExecuteOptions,
|
||||
agentOverrides?: TaskExecutionOptions,
|
||||
): Promise<void> {
|
||||
// Validate that only one workflow option is specified
|
||||
if (options?.workflow && options?.workflowPath) {
|
||||
error('Cannot specify both --workflow and --workflow-path');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
let workflowIdentifier: string | null;
|
||||
let isPathBased = false;
|
||||
|
||||
if (options?.workflowPath) {
|
||||
workflowIdentifier = await determineWorkflowPath(cwd, options.workflowPath);
|
||||
isPathBased = true;
|
||||
} else {
|
||||
workflowIdentifier = await determineWorkflow(cwd, options?.workflow);
|
||||
isPathBased = false;
|
||||
}
|
||||
const workflowIdentifier = await determineWorkflow(cwd, options?.workflow);
|
||||
|
||||
if (workflowIdentifier === null) {
|
||||
info('Cancelled');
|
||||
@ -149,8 +133,8 @@ async function selectAndExecuteTask(
|
||||
options?.createWorktree,
|
||||
);
|
||||
|
||||
log.info('Starting task execution', { workflow: workflowIdentifier, worktree: isWorktree, pathBased: isPathBased });
|
||||
const taskSuccess = await executeTask(task, execCwd, workflowIdentifier, isPathBased, cwd, agentOverrides);
|
||||
log.info('Starting task execution', { workflow: workflowIdentifier, worktree: isWorktree });
|
||||
const taskSuccess = await executeTask(task, execCwd, workflowIdentifier, cwd, agentOverrides);
|
||||
|
||||
if (taskSuccess && isWorktree) {
|
||||
const commitResult = autoCommitAndPush(execCwd, task, cwd);
|
||||
@ -188,17 +172,19 @@ async function selectAndExecuteTask(
|
||||
|
||||
/**
|
||||
* Determine workflow to use.
|
||||
* If override is provided, validate it (either as a name or path).
|
||||
* Otherwise, prompt user to select interactively.
|
||||
*/
|
||||
/**
|
||||
* Determine workflow to use (name-based only).
|
||||
* For path-based loading, use determineWorkflowPath() instead.
|
||||
*
|
||||
* - If override looks like a path (isWorkflowPath), return it directly (validation is done at load time).
|
||||
* - If override is a name, validate it exists in available workflows.
|
||||
* - If no override, prompt user to select interactively.
|
||||
*/
|
||||
async function determineWorkflow(cwd: string, override?: string): Promise<string | null> {
|
||||
if (override) {
|
||||
// Validate workflow name exists
|
||||
const availableWorkflows = listWorkflows();
|
||||
// Path-based: skip name validation (loader handles existence check)
|
||||
if (isWorkflowPath(override)) {
|
||||
return override;
|
||||
}
|
||||
// Name-based: validate workflow name exists
|
||||
const availableWorkflows = listWorkflows(cwd);
|
||||
const knownWorkflows = availableWorkflows.length === 0 ? [DEFAULT_WORKFLOW_NAME] : availableWorkflows;
|
||||
if (!knownWorkflows.includes(override)) {
|
||||
error(`Workflow not found: ${override}`);
|
||||
@ -209,34 +195,6 @@ async function determineWorkflow(cwd: string, override?: string): Promise<string
|
||||
return selectWorkflow(cwd);
|
||||
}
|
||||
|
||||
/**
|
||||
* Determine workflow path (path-based loading).
|
||||
* Validates that the file exists.
|
||||
*/
|
||||
async function determineWorkflowPath(cwd: string, workflowPath: string): Promise<string | null> {
|
||||
const { existsSync } = await import('node:fs');
|
||||
const { resolve: resolvePath, isAbsolute } = await import('node:path');
|
||||
const { homedir } = await import('node:os');
|
||||
|
||||
let resolvedPath = workflowPath;
|
||||
|
||||
// Handle home directory
|
||||
if (workflowPath.startsWith('~')) {
|
||||
const home = homedir();
|
||||
resolvedPath = resolvePath(home, workflowPath.slice(1).replace(/^\//, ''));
|
||||
} else if (!isAbsolute(workflowPath)) {
|
||||
// Relative path
|
||||
resolvedPath = resolvePath(cwd, workflowPath);
|
||||
}
|
||||
|
||||
if (!existsSync(resolvedPath)) {
|
||||
error(`Workflow file not found: ${workflowPath}`);
|
||||
return null;
|
||||
}
|
||||
|
||||
return workflowPath; // Return original path (loader will resolve it)
|
||||
}
|
||||
|
||||
export async function confirmAndCreateWorktree(
|
||||
cwd: string,
|
||||
task: string,
|
||||
@ -303,8 +261,7 @@ program
|
||||
// --- Global options ---
|
||||
program
|
||||
.option('-i, --issue <number>', 'GitHub issue number (equivalent to #N)', (val: string) => parseInt(val, 10))
|
||||
.option('-w, --workflow <name>', 'Workflow name to use')
|
||||
.option('--workflow-path <path>', 'Path to workflow file (alternative to --workflow)')
|
||||
.option('-w, --workflow <name>', 'Workflow name or path to workflow file')
|
||||
.option('-b, --branch <name>', 'Branch name (auto-generated if omitted)')
|
||||
.option('--auto-pr', 'Create PR after successful execution')
|
||||
.option('--repo <owner/repo>', 'Repository (defaults to current)')
|
||||
@ -438,7 +395,6 @@ program
|
||||
autoPr: opts.autoPr === true,
|
||||
repo: opts.repo as string | undefined,
|
||||
workflow: opts.workflow as string | undefined,
|
||||
workflowPath: opts.workflowPath as string | undefined,
|
||||
createWorktree: createWorktreeOverride,
|
||||
};
|
||||
|
||||
|
||||
@ -124,7 +124,7 @@ export async function addTask(cwd: string, task?: string): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
const availableWorkflows = listWorkflows();
|
||||
const availableWorkflows = listWorkflows(cwd);
|
||||
if (availableWorkflows.length > 0) {
|
||||
const currentWorkflow = getCurrentWorkflow(cwd);
|
||||
const defaultWorkflow = availableWorkflows.includes(currentWorkflow)
|
||||
|
||||
@ -222,7 +222,7 @@ export function deleteBranch(projectDir: string, item: BranchListItem): boolean
|
||||
* If multiple workflows available, prompt user to select.
|
||||
*/
|
||||
async function selectWorkflowForInstruction(projectDir: string): Promise<string | null> {
|
||||
const availableWorkflows = listWorkflows();
|
||||
const availableWorkflows = listWorkflows(projectDir);
|
||||
const currentWorkflow = getCurrentWorkflow(projectDir);
|
||||
|
||||
if (availableWorkflows.length === 0) {
|
||||
@ -324,7 +324,7 @@ export async function instructBranch(
|
||||
: instruction;
|
||||
|
||||
// 5. Execute task on temp clone
|
||||
const taskSuccess = await executeTask(fullInstruction, clone.path, selectedWorkflow, false, projectDir, options);
|
||||
const taskSuccess = await executeTask(fullInstruction, clone.path, selectedWorkflow, projectDir, options);
|
||||
|
||||
// 6. Auto-commit+push if successful
|
||||
if (taskSuccess) {
|
||||
|
||||
@ -32,7 +32,7 @@ export interface PipelineExecutionOptions {
|
||||
issueNumber?: number;
|
||||
/** Task content (alternative to issue) */
|
||||
task?: string;
|
||||
/** Workflow name */
|
||||
/** Workflow name or path to workflow file */
|
||||
workflow: string;
|
||||
/** Branch name (auto-generated if omitted) */
|
||||
branch?: string;
|
||||
@ -191,7 +191,7 @@ export async function executePipeline(options: PipelineExecutionOptions): Promis
|
||||
? { provider: options.provider, model: options.model }
|
||||
: undefined;
|
||||
|
||||
const taskSuccess = await executeTask(task, cwd, workflow, false, undefined, agentOverrides);
|
||||
const taskSuccess = await executeTask(task, cwd, workflow, cwd, agentOverrides);
|
||||
|
||||
if (!taskSuccess) {
|
||||
error(`Workflow '${workflow}' failed`);
|
||||
|
||||
@ -2,7 +2,7 @@
|
||||
* Task execution logic
|
||||
*/
|
||||
|
||||
import { loadWorkflow, loadWorkflowFromPath, loadGlobalConfig } from '../config/index.js';
|
||||
import { loadWorkflowByIdentifier, isWorkflowPath, loadGlobalConfig } from '../config/index.js';
|
||||
import { TaskRunner, type TaskInfo } from '../task/index.js';
|
||||
import { createSharedClone } from '../task/clone.js';
|
||||
import { autoCommitAndPush } from '../task/autoCommit.js';
|
||||
@ -28,29 +28,26 @@ export interface TaskExecutionOptions {
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute a single task with workflow
|
||||
* Execute a single task with workflow.
|
||||
*
|
||||
* @param task - Task content
|
||||
* @param cwd - Working directory (may be a clone path)
|
||||
* @param workflowIdentifier - Workflow name or path
|
||||
* @param isPathBased - True if workflowIdentifier is a file path, false if it's a name
|
||||
* @param workflowIdentifier - Workflow name or path (auto-detected by isWorkflowPath)
|
||||
* @param projectCwd - Project root (where .takt/ lives). Defaults to cwd.
|
||||
*/
|
||||
export async function executeTask(
|
||||
task: string,
|
||||
cwd: string,
|
||||
workflowIdentifier: string = DEFAULT_WORKFLOW_NAME,
|
||||
isPathBased: boolean = false,
|
||||
projectCwd?: string,
|
||||
options?: TaskExecutionOptions
|
||||
): Promise<boolean> {
|
||||
const effectiveProjectCwd = projectCwd || cwd;
|
||||
|
||||
const workflowConfig = isPathBased
|
||||
? loadWorkflowFromPath(workflowIdentifier, effectiveProjectCwd)
|
||||
: loadWorkflow(workflowIdentifier, effectiveProjectCwd);
|
||||
const workflowConfig = loadWorkflowByIdentifier(workflowIdentifier, effectiveProjectCwd);
|
||||
|
||||
if (!workflowConfig) {
|
||||
if (isPathBased) {
|
||||
if (isWorkflowPath(workflowIdentifier)) {
|
||||
error(`Workflow file not found: ${workflowIdentifier}`);
|
||||
} else {
|
||||
error(`Workflow "${workflowIdentifier}" not found.`);
|
||||
@ -97,7 +94,7 @@ export async function executeAndCompleteTask(
|
||||
const { execCwd, execWorkflow, isWorktree } = await resolveTaskExecution(task, cwd, workflowName);
|
||||
|
||||
// cwd is always the project root; pass it as projectCwd so reports/sessions go there
|
||||
const taskSuccess = await executeTask(task.content, execCwd, execWorkflow, false, cwd, options);
|
||||
const taskSuccess = await executeTask(task.content, execCwd, execWorkflow, cwd, options);
|
||||
const completedAt = new Date().toISOString();
|
||||
|
||||
if (taskSuccess && isWorktree) {
|
||||
|
||||
@ -12,7 +12,7 @@ import { selectOption } from '../prompt/index.js';
|
||||
*/
|
||||
function getAllWorkflowOptions(cwd: string): { label: string; value: string }[] {
|
||||
const current = getCurrentWorkflow(cwd);
|
||||
const workflows = listWorkflows();
|
||||
const workflows = listWorkflows(cwd);
|
||||
|
||||
const options: { label: string; value: string }[] = [];
|
||||
|
||||
|
||||
@ -7,9 +7,9 @@
|
||||
// Workflow loading
|
||||
export {
|
||||
getBuiltinWorkflow,
|
||||
loadWorkflowFromFile,
|
||||
loadWorkflowFromPath,
|
||||
loadWorkflow,
|
||||
loadWorkflowByIdentifier,
|
||||
isWorkflowPath,
|
||||
loadAllWorkflows,
|
||||
listWorkflows,
|
||||
} from './workflowLoader.js';
|
||||
|
||||
@ -233,7 +233,7 @@ function normalizeWorkflowConfig(raw: unknown, workflowDir: string): WorkflowCon
|
||||
* Load a workflow from a YAML file.
|
||||
* @param filePath Path to the workflow YAML file
|
||||
*/
|
||||
export function loadWorkflowFromFile(filePath: string): WorkflowConfig {
|
||||
function loadWorkflowFromFile(filePath: string): WorkflowConfig {
|
||||
if (!existsSync(filePath)) {
|
||||
throw new Error(`Workflow file not found: ${filePath}`);
|
||||
}
|
||||
@ -266,14 +266,14 @@ function resolvePath(pathInput: string, basePath: string = process.cwd()): strin
|
||||
}
|
||||
|
||||
/**
|
||||
* Load workflow from a file path (explicit path-based loading).
|
||||
* Use this when user explicitly specifies a workflow file path via --workflow-path.
|
||||
* Load workflow from a file path.
|
||||
* Called internally by loadWorkflowByIdentifier when the identifier is detected as a path.
|
||||
*
|
||||
* @param filePath Path to workflow file (absolute, relative, or home-dir prefixed with ~)
|
||||
* @param basePath Base directory for resolving relative paths (default: cwd)
|
||||
* @returns WorkflowConfig or null if file not found
|
||||
*/
|
||||
export function loadWorkflowFromPath(
|
||||
function loadWorkflowFromPath(
|
||||
filePath: string,
|
||||
basePath: string = process.cwd()
|
||||
): WorkflowConfig | null {
|
||||
@ -319,55 +319,61 @@ export function loadWorkflow(
|
||||
return getBuiltinWorkflow(name);
|
||||
}
|
||||
|
||||
/** Load all workflows with descriptions (for switch command) */
|
||||
export function loadAllWorkflows(): Map<string, WorkflowConfig> {
|
||||
/**
|
||||
* Load all workflows with descriptions (for switch command).
|
||||
*
|
||||
* Priority (later entries override earlier):
|
||||
* 1. Builtin workflows
|
||||
* 2. User workflows (~/.takt/workflows/)
|
||||
* 3. Project-local workflows (.takt/workflows/)
|
||||
*/
|
||||
export function loadAllWorkflows(cwd: string): Map<string, WorkflowConfig> {
|
||||
const workflows = new Map<string, WorkflowConfig>();
|
||||
const disabled = getDisabledBuiltins();
|
||||
|
||||
// 1. Builtin workflows (lower priority — will be overridden by user)
|
||||
// 1. Builtin workflows (lowest priority)
|
||||
const lang = getLanguage();
|
||||
const builtinDir = getBuiltinWorkflowsDir(lang);
|
||||
if (existsSync(builtinDir)) {
|
||||
for (const entry of readdirSync(builtinDir)) {
|
||||
if (!entry.endsWith('.yaml') && !entry.endsWith('.yml')) continue;
|
||||
loadWorkflowsFromDir(builtinDir, workflows, disabled);
|
||||
|
||||
const entryPath = join(builtinDir, entry);
|
||||
if (statSync(entryPath).isFile()) {
|
||||
const workflowName = entry.replace(/\.ya?ml$/, '');
|
||||
if (disabled.includes(workflowName)) continue;
|
||||
try {
|
||||
workflows.set(workflowName, loadWorkflowFromFile(entryPath));
|
||||
} catch {
|
||||
// Skip invalid workflows
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 2. User workflows (higher priority — overrides builtins)
|
||||
// 2. User workflows (overrides builtins)
|
||||
const globalWorkflowsDir = getGlobalWorkflowsDir();
|
||||
if (existsSync(globalWorkflowsDir)) {
|
||||
for (const entry of readdirSync(globalWorkflowsDir)) {
|
||||
if (!entry.endsWith('.yaml') && !entry.endsWith('.yml')) continue;
|
||||
loadWorkflowsFromDir(globalWorkflowsDir, workflows);
|
||||
|
||||
const entryPath = join(globalWorkflowsDir, entry);
|
||||
if (statSync(entryPath).isFile()) {
|
||||
try {
|
||||
const workflow = loadWorkflowFromFile(entryPath);
|
||||
const workflowName = entry.replace(/\.ya?ml$/, '');
|
||||
workflows.set(workflowName, workflow);
|
||||
} catch {
|
||||
// Skip invalid workflows
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// 3. Project-local workflows (highest priority)
|
||||
const projectWorkflowsDir = join(getProjectConfigDir(cwd), 'workflows');
|
||||
loadWorkflowsFromDir(projectWorkflowsDir, workflows);
|
||||
|
||||
return workflows;
|
||||
}
|
||||
|
||||
/** List available workflows (user + builtin, excluding disabled) */
|
||||
export function listWorkflows(): string[] {
|
||||
/** Load workflow files from a directory into a Map (later calls override earlier entries) */
|
||||
function loadWorkflowsFromDir(
|
||||
dir: string,
|
||||
target: Map<string, WorkflowConfig>,
|
||||
disabled?: string[],
|
||||
): void {
|
||||
if (!existsSync(dir)) return;
|
||||
for (const entry of readdirSync(dir)) {
|
||||
if (!entry.endsWith('.yaml') && !entry.endsWith('.yml')) continue;
|
||||
const entryPath = join(dir, entry);
|
||||
if (!statSync(entryPath).isFile()) continue;
|
||||
const workflowName = entry.replace(/\.ya?ml$/, '');
|
||||
if (disabled?.includes(workflowName)) continue;
|
||||
try {
|
||||
target.set(workflowName, loadWorkflowFromFile(entryPath));
|
||||
} catch {
|
||||
// Skip invalid workflows
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* List available workflow names (builtin + user + project-local, excluding disabled).
|
||||
*
|
||||
* @param cwd Project root directory (used to scan project-local .takt/workflows/).
|
||||
*/
|
||||
export function listWorkflows(cwd: string): string[] {
|
||||
const workflows = new Set<string>();
|
||||
const disabled = getDisabledBuiltins();
|
||||
|
||||
@ -380,9 +386,53 @@ export function listWorkflows(): string[] {
|
||||
const globalWorkflowsDir = getGlobalWorkflowsDir();
|
||||
scanWorkflowDir(globalWorkflowsDir, workflows);
|
||||
|
||||
// 3. Project-local workflows
|
||||
const projectWorkflowsDir = join(getProjectConfigDir(cwd), 'workflows');
|
||||
scanWorkflowDir(projectWorkflowsDir, workflows);
|
||||
|
||||
return Array.from(workflows).sort();
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a workflow identifier looks like a file path (vs a workflow name).
|
||||
*
|
||||
* Path indicators:
|
||||
* - Starts with `/` (absolute path)
|
||||
* - Starts with `~` (home directory)
|
||||
* - Starts with `./` or `../` (relative path)
|
||||
* - Ends with `.yaml` or `.yml` (file extension)
|
||||
*/
|
||||
export function isWorkflowPath(identifier: string): boolean {
|
||||
return (
|
||||
identifier.startsWith('/') ||
|
||||
identifier.startsWith('~') ||
|
||||
identifier.startsWith('./') ||
|
||||
identifier.startsWith('../') ||
|
||||
identifier.endsWith('.yaml') ||
|
||||
identifier.endsWith('.yml')
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Load workflow by identifier (auto-detects name vs path).
|
||||
*
|
||||
* If the identifier looks like a path (see isWorkflowPath), loads from file.
|
||||
* Otherwise, loads by name with the standard priority chain:
|
||||
* project-local → user → builtin.
|
||||
*
|
||||
* @param identifier Workflow name or file path
|
||||
* @param projectCwd Project root directory (for project-local resolution and relative path base)
|
||||
*/
|
||||
export function loadWorkflowByIdentifier(
|
||||
identifier: string,
|
||||
projectCwd: string
|
||||
): WorkflowConfig | null {
|
||||
if (isWorkflowPath(identifier)) {
|
||||
return loadWorkflowFromPath(identifier, projectCwd);
|
||||
}
|
||||
return loadWorkflow(identifier, projectCwd);
|
||||
}
|
||||
|
||||
/** Scan a directory for .yaml/.yml files and add names to the set */
|
||||
function scanWorkflowDir(dir: string, target: Set<string>, disabled?: string[]): void {
|
||||
if (!existsSync(dir)) return;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user