From ee0cb8e13a1d5444e3950aaf37fb68089607e693 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Thu, 5 Feb 2026 16:59:32 +0900 Subject: [PATCH] =?UTF-8?q?E2E=E3=83=86=E3=82=B9=E3=83=88=E5=9F=BA?= =?UTF-8?q?=E7=9B=A4=E3=81=AE=E8=BF=BD=E5=8A=A0=E3=83=BB=E3=83=AC=E3=83=93?= =?UTF-8?q?=E3=83=A5=E3=83=BC=E3=82=A8=E3=83=BC=E3=82=B8=E3=82=A7=E3=83=B3?= =?UTF-8?q?=E3=83=88=E6=94=B9=E5=96=84=E3=83=BBlint=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - E2Eテストのフィクスチャ、ヘルパー、スペックを追加 - mock/provider別のvitest設定を追加 - レビューエージェントのプロンプト改善 - TTY判定の共通化、list/confirmのnon-interactive対応 - eslint no-non-null-assertion を off に変更、未使用インポート削除 --- AGENTS.md | 57 ++++---- docs/testing/e2e.md | 103 ++++++++++++++ e2e/fixtures/agents/test-coder.md | 9 ++ e2e/fixtures/agents/test-reporter.md | 9 ++ e2e/fixtures/agents/test-reviewer-a.md | 8 ++ e2e/fixtures/agents/test-reviewer-b.md | 8 ++ e2e/fixtures/pieces/mock-single-step.yaml | 19 +++ e2e/fixtures/pieces/multi-step-parallel.yaml | 49 +++++++ e2e/fixtures/pieces/report-judge.yaml | 22 +++ e2e/fixtures/pieces/simple.yaml | 19 +++ e2e/fixtures/scenarios/add-task.json | 7 + e2e/fixtures/scenarios/execute-done.json | 6 + .../scenarios/multi-step-all-approved.json | 7 + .../scenarios/multi-step-needs-fix.json | 15 ++ e2e/fixtures/scenarios/report-judge.json | 17 +++ e2e/helpers/isolated-env.ts | 66 +++++++++ e2e/helpers/takt-runner.ts | 91 ++++++++++++ e2e/helpers/test-repo.ts | 109 +++++++++++++++ e2e/specs/add-and-run.e2e.ts | 72 ++++++++++ e2e/specs/add.e2e.ts | 95 +++++++++++++ e2e/specs/direct-task.e2e.ts | 56 ++++++++ e2e/specs/github-issue.e2e.ts | 102 ++++++++++++++ e2e/specs/list-non-interactive.e2e.ts | 129 ++++++++++++++++++ e2e/specs/multi-step-parallel.e2e.ts | 79 +++++++++++ e2e/specs/pipeline-skip-git.e2e.ts | 57 ++++++++ e2e/specs/pipeline.e2e.ts | 68 +++++++++ e2e/specs/report-judge.e2e.ts | 56 ++++++++ e2e/specs/watch.e2e.ts | 92 +++++++++++++ e2e/specs/worktree.e2e.ts | 51 +++++++ eslint.config.js | 2 +- package.json | 8 ++ .../agents/default/ai-antipattern-reviewer.md | 29 +++- .../agents/default/architecture-reviewer.md | 16 +++ .../agents/default/ai-antipattern-reviewer.md | 29 +++- .../agents/default/architecture-reviewer.md | 15 ++ src/__tests__/e2e-helpers.test.ts | 73 ++++++++++ src/__tests__/test-setup.ts | 3 + src/app/cli/commands.ts | 19 ++- src/app/cli/routing.ts | 2 +- src/features/tasks/list/index.ts | 107 ++++++++++++++- src/features/tasks/list/taskActions.ts | 12 +- src/infra/codex/client.ts | 2 +- src/infra/config/paths.ts | 4 +- src/infra/task/branchList.ts | 4 +- src/shared/prompt/confirm.ts | 11 ++ src/shared/prompt/select.ts | 5 +- src/shared/prompt/tty.ts | 17 +++ vitest.config.e2e.mock.ts | 26 ++++ vitest.config.e2e.provider.ts | 23 ++++ vitest.config.e2e.ts | 18 +++ vitest.config.ts | 1 + 51 files changed, 1857 insertions(+), 47 deletions(-) create mode 100644 docs/testing/e2e.md create mode 100644 e2e/fixtures/agents/test-coder.md create mode 100644 e2e/fixtures/agents/test-reporter.md create mode 100644 e2e/fixtures/agents/test-reviewer-a.md create mode 100644 e2e/fixtures/agents/test-reviewer-b.md create mode 100644 e2e/fixtures/pieces/mock-single-step.yaml create mode 100644 e2e/fixtures/pieces/multi-step-parallel.yaml create mode 100644 e2e/fixtures/pieces/report-judge.yaml create mode 100644 e2e/fixtures/pieces/simple.yaml create mode 100644 e2e/fixtures/scenarios/add-task.json create mode 100644 e2e/fixtures/scenarios/execute-done.json create mode 100644 e2e/fixtures/scenarios/multi-step-all-approved.json create mode 100644 e2e/fixtures/scenarios/multi-step-needs-fix.json create mode 100644 e2e/fixtures/scenarios/report-judge.json create mode 100644 e2e/helpers/isolated-env.ts create mode 100644 e2e/helpers/takt-runner.ts create mode 100644 e2e/helpers/test-repo.ts create mode 100644 e2e/specs/add-and-run.e2e.ts create mode 100644 e2e/specs/add.e2e.ts create mode 100644 e2e/specs/direct-task.e2e.ts create mode 100644 e2e/specs/github-issue.e2e.ts create mode 100644 e2e/specs/list-non-interactive.e2e.ts create mode 100644 e2e/specs/multi-step-parallel.e2e.ts create mode 100644 e2e/specs/pipeline-skip-git.e2e.ts create mode 100644 e2e/specs/pipeline.e2e.ts create mode 100644 e2e/specs/report-judge.e2e.ts create mode 100644 e2e/specs/watch.e2e.ts create mode 100644 e2e/specs/worktree.e2e.ts create mode 100644 src/__tests__/e2e-helpers.test.ts create mode 100644 src/__tests__/test-setup.ts create mode 100644 src/shared/prompt/tty.ts create mode 100644 vitest.config.e2e.mock.ts create mode 100644 vitest.config.e2e.provider.ts create mode 100644 vitest.config.e2e.ts diff --git a/AGENTS.md b/AGENTS.md index 5f46801..2a8f5bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,40 +1,41 @@ # Repository Guidelines -このリポジトリに貢献する際の基本的な構成と期待値をまとめています。短い説明と例で各セクションを完結に示します。 +このドキュメントは、このリポジトリに貢献するための実務的な指針をまとめたものです。短く具体的な説明と例で、作業の迷いを減らします。 -## プロジェクト構成とモジュール整理 +## Project Structure & Module Organization - 主要ソースは `src/` にあり、エントリポイントは `src/index.ts`、CLI は `src/app/cli/index.ts` です。 -- テストは `src/__tests__/` に置き、ファイル名は対象機能が一目でわかるようにします（例: `client.test.ts`）。 +- テストは `src/__tests__/` に置き、対象が明確になる名前を付けます（例: `client.test.ts`）。 - ビルド成果物は `dist/`、実行スクリプトは `bin/`、静的リソースは `resources/`、ドキュメントは `docs/` で管理します。 -- 設定やキャッシュを使う際は `~/.takt/` 以下（実行時）や `.takt/`（プロジェクト固有）を参照します。 +- 実行時の設定やキャッシュは `~/.takt/`、プロジェクト固有の設定は `.takt/` を参照します。 -## ビルド・テスト・開発コマンド -``` -npm run build # TypeScript コンパイルを実行し dist/ を生成 -npm run watch # ソース変更を監視しつつ再ビルド -npm run lint # ESLint で src/ を解析 -npm run test # Vitest で全テストを実行 -npm run test:watch # テスト実行をウォッチ -``` -- 単体テストを個別実行する例: `npx vitest run src/__tests__/client.test.ts`。 +## Build, Test, and Development Commands +- `npm run build`: TypeScript をコンパイルして `dist/` を生成します。 +- `npm run watch`: ソース変更を監視しながら再ビルドします。 +- `npm run lint`: ESLint で `src/` を解析します。 +- `npm run test`: Vitest で全テストを実行します。 +- `npm run test:watch`: テストをウォッチ実行します。 +- `npx vitest run src/__tests__/client.test.ts`: 単体テストを個別実行する例です。 -## コーディングスタイルと命名 -- TypeScript + strict モードを前提にし、可読性や null 安全を優先します。 -- ESM 形式なので `import` の拡張子は `.js` に固定してください。 -- ESLint（`eslint src/`）と prettier ルールを守り、命名は camelCase（関数・変数）および PascalCase（クラス）を採用。 -- クロスファイルの共有型は `src/types/` 風に整理し、既存の命名パターンを踏襲します。 +## Coding Style & Naming Conventions +- TypeScript + strict を前提に、null 安全と可読性を優先します。 +- ESM 形式のため、`import` の拡張子は `.js` に固定してください。 +- 命名は camelCase（関数・変数）と PascalCase（クラス）を採用します。 +- 共有型は `src/types/` に整理し、既存の命名パターンに合わせます。 +- ESLint と Prettier の規約に従い、修正後は `npm run lint` を実行します。 -## テスト指針 -- テストフレームワークは Vitest（`vitest.config.ts` 参照）。全ての新機能・修正には関連テストを追加。 -- テストファイル名は `<対象>.test.ts`、あるいは `<対象>.spec.ts` で統一。 -- コンポーネント依存はモックやスタブを使い、状態を分離したシナリオを心がけます。 +## Testing Guidelines +- テストフレームワークは Vitest（`vitest.config.ts`）です。 +- 新規機能や修正には関連テストを追加します。 +- ファイル名は `<対象>.test.ts` または `<対象>.spec.ts` を使用します。 +- 依存が重い箇所はモックやスタブで状態を分離します。 -## コミットとプルリク -- 履歴は「短い要約 + 1 行」スタイル。英語・日本語混在可、目的が伝わるよう `feat:`, `fix:` 等のプレフィックスも可。 -- PR には変更概要・テスト結果・関連 Issue（あれば）を含め、小さな対象に絞ってレビュー負荷を抑えます。 -- ドキュメントや設定変更を伴う場合は `CHANGELOG.md` への追記を検討し、スクリーンショットやログがあれば添付します。 +## Commit & Pull Request Guidelines +- コミットメッセージは短い要約が中心で、日本語・英語どちらも使われています。 +- `fix:`, `hotfix:` などのプレフィックスや、`#32` のような Issue 参照が見られます。必要に応じて付けてください。 +- バージョン更新や変更履歴の更新は明示的なメッセージで行います（例: `0.5.1`, `update CHANGELOG`）。 +- PR には変更概要、テスト結果、関連 Issue を記載し、小さく分割してレビュー負荷を抑えます。UI/ログ変更がある場合はスクリーンショットやログを添付します。 -## セキュリティと設定の注意 +## Security & Configuration Tips - 脆弱性は公開 Issue ではなくメンテナへ直接報告します。 - `.takt/logs/` など機密情報を含む可能性のあるファイルは共有しないでください。 - `~/.takt/config.yaml` の `trusted` ディレクトリは最小限にし、不要なパスは登録しないでください。 -- 新しいピースを追加する場合は `~/.takt/pieces/` の既存スキーマを踏襲し、不要な拡張を避けます。 +- 新しいピースを追加する場合は `~/.takt/pieces/` の既存スキーマに合わせます。 diff --git a/docs/testing/e2e.md b/docs/testing/e2e.md new file mode 100644 index 0000000..6d386db --- /dev/null +++ b/docs/testing/e2e.md @@ -0,0 +1,103 @@ +# E2Eテスト概要 +このドキュメントは、E2Eテストの目的・前提条件・実行方法を短くまとめた索引です。 +E2Eテストを追加・変更した場合は、このドキュメントも更新してください。 + +## 前提条件 +- `gh` CLI が利用可能で、対象GitHubアカウントでログイン済みであること。 +- `takt-testing` リポジトリが対象アカウントに存在すること（E2Eがクローンして使用）。 +- 必要に応じて `TAKT_E2E_PROVIDER` を設定すること（例: `claude` / `codex`）。 +- 実行時間が長いテストがあるため、タイムアウトに注意すること。 +- E2Eは `e2e/helpers/test-repo.ts` が `gh` でリポジトリをクローンし、テンポラリディレクトリで実行する。 +- 対話UIを避けるため、E2E環境では `TAKT_NO_TTY=1` を設定してTTYを無効化する。 +- 実行ディレクトリの形式（macOSの例）: + - リポジトリクローン: `$(os.tmpdir())/takt-e2e-repo-/` + - 実行環境: `$(os.tmpdir())/takt-e2e--/` + +## 実行コマンド +- `npm run test:e2e`: E2E全体を実行。 +- `npm run test:e2e:mock`: mock固定のE2Eのみ実行。 +- `npm run test:e2e:provider`: `claude` と `codex` の両方で実行。 +- `npm run test:e2e:provider:claude`: `TAKT_E2E_PROVIDER=claude` で実行。 +- `npm run test:e2e:provider:codex`: `TAKT_E2E_PROVIDER=codex` で実行。 +- `npm run test:e2e:all`: `mock` + `provider` を通しで実行。 +- `npm run test:e2e:claude`: `test:e2e:provider:claude` の別名。 +- `npm run test:e2e:codex`: `test:e2e:provider:codex` の別名。 +- `npx vitest run e2e/specs/add-and-run.e2e.ts`: 単体実行の例。 + +## シナリオ一覧 +- Add task and run（`e2e/specs/add-and-run.e2e.ts`） + - 目的: `.takt/tasks/` にタスクYAMLを配置し、`takt run` が実行できることを確認。 + - LLM: 条件付き（`TAKT_E2E_PROVIDER` が `claude` / `codex` の場合に呼び出す） + - 手順（ユーザー行動/コマンド）: + - `.takt/tasks/e2e-test-task.yaml` にタスクを作成（`piece` は `e2e/fixtures/pieces/simple.yaml` を指定）。 + - `takt run` を実行する。 + - `README.md` に行が追加されることを確認する。 + - タスクファイルが `tasks/` から移動されることを確認する。 +- Worktree/Clone isolation（`e2e/specs/worktree.e2e.ts`） + - 目的: `--create-worktree yes` 指定で隔離環境に実行されることを確認。 + - LLM: 条件付き（`TAKT_E2E_PROVIDER` が `claude` / `codex` の場合に呼び出す） + - 手順（ユーザー行動/コマンド）: + - `takt --task 'Add a line "worktree test" to README.md' --piece e2e/fixtures/pieces/simple.yaml --create-worktree yes` を実行する。 + - コマンドが成功終了することを確認する。 +- Pipeline mode（`e2e/specs/pipeline.e2e.ts`） + - 目的: ブランチ作成→タスク実行→コミット→push→PR作成の一連フローを確認。 + - LLM: 条件付き（`TAKT_E2E_PROVIDER` が `claude` / `codex` の場合に呼び出す） + - 手順（ユーザー行動/コマンド）: + - `takt --pipeline --task 'Create a file called hello.txt with the content "Hello World"' --piece e2e/fixtures/pieces/simple.yaml --auto-pr --repo /` を実行する。 + - 出力に `completed` と `PR created` が含まれることを確認する。 + - `gh pr list --repo /` でPRが作成されていることを確認する。 +- GitHub Issue processing（`e2e/specs/github-issue.e2e.ts`） + - 目的: Issue番号からパイプラインを起動してPR作成までを確認。 + - LLM: 条件付き（`TAKT_E2E_PROVIDER` が `claude` / `codex` の場合に呼び出す） + - 手順（ユーザー行動/コマンド）: + - `gh issue create --title 'E2E Test Issue' --body 'Create a file called issue-test.txt with the content \"Issue resolved\"' --repo /` でIssueを作成する。 + - 作成したIssue番号を控える。 + - `takt --pipeline --issue --piece e2e/fixtures/pieces/simple.yaml --auto-pr --repo /` を実行する。 + - 出力に `Issue #` / `completed` / `PR created` が含まれることを確認する。 + - `gh pr list --repo /` でPR一覧にIssueタイトルがあることを確認する。 +- Direct task execution（`e2e/specs/direct-task.e2e.ts`） + - 目的: `--task` の直接実行が、プロンプトなしで完了することを確認。 + - LLM: 呼び出さない（`--provider mock` 固定） + - 手順（ユーザー行動/コマンド）: + - `takt --task 'Create a file called noop.txt' --piece e2e/fixtures/pieces/mock-single-step.yaml --create-worktree no --provider mock` を実行する。 + - `TAKT_MOCK_SCENARIO=e2e/fixtures/scenarios/execute-done.json` を設定する。 + - 出力に `Piece completed` が含まれることを確認する。 +- Pipeline mode with --skip-git（`e2e/specs/pipeline-skip-git.e2e.ts`） + - 目的: `--skip-git` 指定時にGit操作を行わずパイプラインが完了することを確認。 + - LLM: 呼び出さない（`--provider mock` 固定） + - 手順（ユーザー行動/コマンド）: + - `takt --pipeline --task 'Create a file called noop.txt' --piece e2e/fixtures/pieces/mock-single-step.yaml --skip-git --provider mock` を実行する。 + - `TAKT_MOCK_SCENARIO=e2e/fixtures/scenarios/execute-done.json` を設定する。 + - 出力に `completed` が含まれることを確認する。 +- Report + Judge phases（`e2e/specs/report-judge.e2e.ts`） + - 目的: reportフェーズとjudgeフェーズを通ることを確認（mockシナリオ）。 + - LLM: 呼び出さない（`--provider mock` 固定） + - 手順（ユーザー行動/コマンド）: + - `takt --task 'Create a short report and finish' --piece e2e/fixtures/pieces/report-judge.yaml --create-worktree no --provider mock` を実行する。 + - `TAKT_MOCK_SCENARIO=e2e/fixtures/scenarios/report-judge.json` を設定する。 + - 出力に `Piece completed` が含まれることを確認する。 +- Add task（`e2e/specs/add.e2e.ts`） + - 目的: `takt add` がIssue参照からタスクファイルを生成できることを確認。 + - LLM: 呼び出さない（`provider: mock` + `TAKT_MOCK_SCENARIO` 固定） + - 手順（ユーザー行動/コマンド）: + - `gh issue create ...` でIssueを作成する。 + - `TAKT_MOCK_SCENARIO=e2e/fixtures/scenarios/add-task.json` を設定する。 + - `takt add '#'` を実行し、`Create worktree?` に `n` で回答する。 + - `.takt/tasks/` にYAMLが生成されることを確認する。 +- Watch tasks（`e2e/specs/watch.e2e.ts`） + - 目的: `takt watch` が監視中に追加されたタスクを実行できることを確認。 + - LLM: 呼び出さない（`--provider mock` 固定） + - 手順（ユーザー行動/コマンド）: + - `takt watch --provider mock` を起動する。 + - `.takt/tasks/` にタスクYAMLを追加する（`piece` に `e2e/fixtures/pieces/mock-single-step.yaml` を指定）。 + - 出力に `Task "watch-task" completed` が含まれることを確認する。 + - `Ctrl+C` で終了する。 +- List tasks non-interactive（`e2e/specs/list-non-interactive.e2e.ts`） + - 目的: `takt list` の非対話モードでブランチ操作ができることを確認。 + - LLM: 呼び出さない（LLM不使用の操作のみ） + - 手順（ユーザー行動/コマンド）: + - `takt list --non-interactive --action delete --branch --yes` を実行する。 + - 対象ブランチが削除されることを確認する。 + - `takt list --non-interactive --action diff --branch ` で差分統計が出力されることを確認する。 + - `takt list --non-interactive --action try --branch ` で変更がステージされることを確認する。 + - `takt list --non-interactive --action merge --branch ` でブランチがマージされ削除されることを確認する。 diff --git a/e2e/fixtures/agents/test-coder.md b/e2e/fixtures/agents/test-coder.md new file mode 100644 index 0000000..bbecf2d --- /dev/null +++ b/e2e/fixtures/agents/test-coder.md @@ -0,0 +1,9 @@ +# E2E Test Coder + +You are a coder for E2E testing. + +## Instructions + +- Make the minimal changes required by the task +- Do not perform any work beyond what is explicitly requested +- After completing the task, output your work summary diff --git a/e2e/fixtures/agents/test-reporter.md b/e2e/fixtures/agents/test-reporter.md new file mode 100644 index 0000000..2992179 --- /dev/null +++ b/e2e/fixtures/agents/test-reporter.md @@ -0,0 +1,9 @@ +# E2E Test Reporter + +You are a reporter for E2E testing. + +## Instructions + +- Keep outputs short and direct +- When asked to produce a report, return a brief summary +- Do not perform any work beyond what is explicitly requested diff --git a/e2e/fixtures/agents/test-reviewer-a.md b/e2e/fixtures/agents/test-reviewer-a.md new file mode 100644 index 0000000..eb28f74 --- /dev/null +++ b/e2e/fixtures/agents/test-reviewer-a.md @@ -0,0 +1,8 @@ +# E2E Test Reviewer A + +You are an architecture reviewer for E2E testing. + +## Instructions + +- Review the code architecture +- Output your review result concisely diff --git a/e2e/fixtures/agents/test-reviewer-b.md b/e2e/fixtures/agents/test-reviewer-b.md new file mode 100644 index 0000000..6fbe378 --- /dev/null +++ b/e2e/fixtures/agents/test-reviewer-b.md @@ -0,0 +1,8 @@ +# E2E Test Reviewer B + +You are a security reviewer for E2E testing. + +## Instructions + +- Review the code for security issues +- Output your review result concisely diff --git a/e2e/fixtures/pieces/mock-single-step.yaml b/e2e/fixtures/pieces/mock-single-step.yaml new file mode 100644 index 0000000..2253c79 --- /dev/null +++ b/e2e/fixtures/pieces/mock-single-step.yaml @@ -0,0 +1,19 @@ +name: e2e-mock-single +description: Minimal mock-only piece for CLI E2E + +max_iterations: 3 + +movements: + - name: execute + edit: true + agent: ../agents/test-coder.md + allowed_tools: + - Read + - Write + - Edit + permission_mode: edit + instruction_template: | + {task} + rules: + - condition: Done + next: COMPLETE diff --git a/e2e/fixtures/pieces/multi-step-parallel.yaml b/e2e/fixtures/pieces/multi-step-parallel.yaml new file mode 100644 index 0000000..0a63f02 --- /dev/null +++ b/e2e/fixtures/pieces/multi-step-parallel.yaml @@ -0,0 +1,49 @@ +name: e2e-multi-step-parallel +description: Multi-step piece with parallel sub-movements for E2E testing + +max_iterations: 10 + +initial_movement: plan + +movements: + - name: plan + agent: ../agents/test-coder.md + edit: true + permission_mode: edit + instruction_template: | + Create a plan for the task. + rules: + - condition: "Plan complete" + next: review + + - name: review + parallel: + - name: arch-review + agent: ../agents/test-reviewer-a.md + instruction_template: | + Review the architecture. + rules: + - condition: approved + - condition: needs_fix + - name: security-review + agent: ../agents/test-reviewer-b.md + instruction_template: | + Review security. + rules: + - condition: approved + - condition: needs_fix + rules: + - condition: all("approved") + next: COMPLETE + - condition: any("needs_fix") + next: fix + + - name: fix + agent: ../agents/test-coder.md + edit: true + permission_mode: edit + instruction_template: | + Fix the issues found in review. + rules: + - condition: "Fix applied" + next: review diff --git a/e2e/fixtures/pieces/report-judge.yaml b/e2e/fixtures/pieces/report-judge.yaml new file mode 100644 index 0000000..0e24d06 --- /dev/null +++ b/e2e/fixtures/pieces/report-judge.yaml @@ -0,0 +1,22 @@ +name: e2e-report-judge +description: E2E piece that exercises report + judge phases + +max_iterations: 3 + +movements: + - name: execute + edit: true + agent: ../agents/test-reporter.md + allowed_tools: + - Read + - Write + - Edit + permission_mode: edit + report: report.md + instruction_template: | + {task} + rules: + - condition: Done + next: COMPLETE + - condition: Needs fix + next: ABORT diff --git a/e2e/fixtures/pieces/simple.yaml b/e2e/fixtures/pieces/simple.yaml new file mode 100644 index 0000000..b2a9853 --- /dev/null +++ b/e2e/fixtures/pieces/simple.yaml @@ -0,0 +1,19 @@ +name: e2e-simple +description: Minimal E2E test piece + +max_iterations: 5 + +movements: + - name: execute + edit: true + agent: ../agents/test-coder.md + allowed_tools: + - Read + - Write + - Edit + permission_mode: edit + instruction_template: | + {task} + rules: + - condition: Task completed + next: COMPLETE diff --git a/e2e/fixtures/scenarios/add-task.json b/e2e/fixtures/scenarios/add-task.json new file mode 100644 index 0000000..1fb9fa5 --- /dev/null +++ b/e2e/fixtures/scenarios/add-task.json @@ -0,0 +1,7 @@ +[ + { + "agent": "summarizer", + "status": "done", + "content": "add-task" + } +] diff --git a/e2e/fixtures/scenarios/execute-done.json b/e2e/fixtures/scenarios/execute-done.json new file mode 100644 index 0000000..aa14d6f --- /dev/null +++ b/e2e/fixtures/scenarios/execute-done.json @@ -0,0 +1,6 @@ +[ + { + "status": "done", + "content": "[EXECUTE:1]\n\nTask completed." + } +] diff --git a/e2e/fixtures/scenarios/multi-step-all-approved.json b/e2e/fixtures/scenarios/multi-step-all-approved.json new file mode 100644 index 0000000..5fc97cb --- /dev/null +++ b/e2e/fixtures/scenarios/multi-step-all-approved.json @@ -0,0 +1,7 @@ +[ + { "agent": "test-coder", "status": "done", "content": "Plan created." }, + { "agent": "test-reviewer-a", "status": "done", "content": "Architecture approved." }, + { "agent": "test-reviewer-b", "status": "done", "content": "Security approved." }, + { "agent": "conductor", "status": "done", "content": "[ARCH-REVIEW:1] [SECURITY-REVIEW:1]" }, + { "agent": "conductor", "status": "done", "content": "[ARCH-REVIEW:1] [SECURITY-REVIEW:1]" } +] diff --git a/e2e/fixtures/scenarios/multi-step-needs-fix.json b/e2e/fixtures/scenarios/multi-step-needs-fix.json new file mode 100644 index 0000000..a46b5c7 --- /dev/null +++ b/e2e/fixtures/scenarios/multi-step-needs-fix.json @@ -0,0 +1,15 @@ +[ + { "agent": "test-coder", "status": "done", "content": "Plan created." }, + + { "agent": "test-reviewer-a", "status": "done", "content": "Architecture looks good." }, + { "agent": "test-reviewer-b", "status": "done", "content": "Security issues found." }, + { "agent": "conductor", "status": "done", "content": "[ARCH-REVIEW:1] [SECURITY-REVIEW:2]" }, + { "agent": "conductor", "status": "done", "content": "[ARCH-REVIEW:1] [SECURITY-REVIEW:2]" }, + + { "agent": "test-coder", "status": "done", "content": "Fix applied." }, + + { "agent": "test-reviewer-a", "status": "done", "content": "Architecture still approved." }, + { "agent": "test-reviewer-b", "status": "done", "content": "Security now approved." }, + { "agent": "conductor", "status": "done", "content": "[ARCH-REVIEW:1] [SECURITY-REVIEW:1]" }, + { "agent": "conductor", "status": "done", "content": "[ARCH-REVIEW:1] [SECURITY-REVIEW:1]" } +] diff --git a/e2e/fixtures/scenarios/report-judge.json b/e2e/fixtures/scenarios/report-judge.json new file mode 100644 index 0000000..a73865e --- /dev/null +++ b/e2e/fixtures/scenarios/report-judge.json @@ -0,0 +1,17 @@ +[ + { + "agent": "test-reporter", + "status": "done", + "content": "Work completed." + }, + { + "agent": "test-reporter", + "status": "done", + "content": "Report summary: OK" + }, + { + "agent": "conductor", + "status": "done", + "content": "[EXECUTE:1]" + } +] diff --git a/e2e/helpers/isolated-env.ts b/e2e/helpers/isolated-env.ts new file mode 100644 index 0000000..0baf538 --- /dev/null +++ b/e2e/helpers/isolated-env.ts @@ -0,0 +1,66 @@ +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +export interface IsolatedEnv { + runId: string; + taktDir: string; + env: NodeJS.ProcessEnv; + cleanup: () => void; +} + +/** + * Create an isolated environment for E2E testing. + * + * - Sets TAKT_CONFIG_DIR to a temporary directory + * - Sets GIT_CONFIG_GLOBAL to an isolated .gitconfig file + * - Uses the real ~/.claude/ for Claude authentication + */ +export function createIsolatedEnv(): IsolatedEnv { + const runId = `e2e-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + const baseDir = mkdtempSync(join(tmpdir(), `takt-e2e-${runId}-`)); + + const taktDir = join(baseDir, '.takt'); + const gitConfigPath = join(baseDir, '.gitconfig'); + + // Create TAKT config directory and config.yaml + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + join(taktDir, 'config.yaml'), + [ + 'provider: claude', + 'language: en', + 'log_level: info', + 'default_piece: default', + ].join('\n'), + ); + + // Create isolated Git config file + writeFileSync( + gitConfigPath, + ['[user]', ' name = TAKT E2E Test', ' email = e2e@example.com'].join( + '\n', + ), + ); + + // ...process.env inherits all env vars including TAKT_OPENAI_API_KEY (for Codex) + const env: NodeJS.ProcessEnv = { + ...process.env, + TAKT_CONFIG_DIR: taktDir, + GIT_CONFIG_GLOBAL: gitConfigPath, + TAKT_NO_TTY: '1', + }; + + return { + runId, + taktDir, + env, + cleanup: () => { + try { + rmSync(baseDir, { recursive: true, force: true }); + } catch { + // Best-effort cleanup; ignore errors (e.g., already deleted) + } + }, + }; +} diff --git a/e2e/helpers/takt-runner.ts b/e2e/helpers/takt-runner.ts new file mode 100644 index 0000000..76c0b80 --- /dev/null +++ b/e2e/helpers/takt-runner.ts @@ -0,0 +1,91 @@ +import { execFileSync } from 'node:child_process'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { dirname } from 'node:path'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +export interface TaktRunOptions { + args: string[]; + cwd: string; + env: NodeJS.ProcessEnv; + input?: string; + timeout?: number; +} + +export interface TaktRunResult { + stdout: string; + stderr: string; + exitCode: number; +} + +const DEFAULT_TIMEOUT = 180_000; +const MAX_BUFFER = 10 * 1024 * 1024; + +function getTaktBinPath(): string { + return resolve(__dirname, '../../bin/takt'); +} + +/** + * Prepend --provider to args if provider is specified + * and args do not already contain --provider. + */ +export function injectProviderArgs( + args: readonly string[], + provider: string | undefined, +): string[] { + if (provider && !args.includes('--provider')) { + return ['--provider', provider, ...args]; + } + return [...args]; +} + +/** + * Run the takt CLI and return its result. + * Non-zero exit codes are returned in the result (not thrown). + * + * When TAKT_E2E_PROVIDER env var is set, it automatically prepends + * --provider to the args (unless args already contain --provider). + */ +export function runTakt(options: TaktRunOptions): TaktRunResult { + const binPath = getTaktBinPath(); + const timeout = options.timeout ?? DEFAULT_TIMEOUT; + + const args = injectProviderArgs(options.args, process.env.TAKT_E2E_PROVIDER); + + try { + const stdout = execFileSync('node', [binPath, ...args], { + cwd: options.cwd, + env: options.env, + encoding: 'utf-8', + input: options.input, + timeout, + maxBuffer: MAX_BUFFER, + }); + + return { + stdout, + stderr: '', + exitCode: 0, + }; + } catch (error: unknown) { + // execFileSync throws on non-zero exit or timeout + const err = error as { + stdout?: string; + stderr?: string; + status?: number | null; + signal?: string | null; + }; + + if (err.signal === 'SIGTERM' || err.signal === 'SIGKILL') { + throw new Error(`takt process timed out after ${timeout}ms`); + } + + return { + stdout: err.stdout ?? '', + stderr: err.stderr ?? '', + exitCode: err.status ?? 1, + }; + } +} diff --git a/e2e/helpers/test-repo.ts b/e2e/helpers/test-repo.ts new file mode 100644 index 0000000..6b07aa8 --- /dev/null +++ b/e2e/helpers/test-repo.ts @@ -0,0 +1,109 @@ +import { rmSync } from 'node:fs'; +import { mkdtempSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { execFileSync } from 'node:child_process'; + +export interface TestRepo { + path: string; + repoName: string; + branch: string; + cleanup: () => void; +} + +function getGitHubUser(): string { + const user = execFileSync('gh', ['api', 'user', '--jq', '.login'], { + encoding: 'utf-8', + }).trim(); + + if (!user) { + throw new Error( + 'Failed to get GitHub user. Make sure `gh` CLI is authenticated.', + ); + } + + return user; +} + +/** + * Clone the takt-testing repository and create a test branch. + * + * Cleanup order (important): + * 1. Delete remote branch (requires local directory to exist) + * 2. Close any PRs created during the test + * 3. Delete local directory + */ +export function createTestRepo(): TestRepo { + const user = getGitHubUser(); + const repoName = `${user}/takt-testing`; + + // Verify repository exists + try { + execFileSync('gh', ['repo', 'view', repoName], { + encoding: 'utf-8', + stdio: 'pipe', + }); + } catch { + throw new Error( + `Repository "${repoName}" not found. Please create it first:\n` + + ` gh repo create takt-testing --private --add-readme`, + ); + } + + // Clone to temporary directory + const repoPath = mkdtempSync(join(tmpdir(), 'takt-e2e-repo-')); + execFileSync('gh', ['repo', 'clone', repoName, repoPath], { + stdio: 'pipe', + }); + + // Create test branch + const testBranch = `e2e-test-${Date.now()}`; + execFileSync('git', ['checkout', '-b', testBranch], { + cwd: repoPath, + stdio: 'pipe', + }); + + return { + path: repoPath, + repoName, + branch: testBranch, + cleanup: () => { + // 1. Delete remote branch (best-effort) + try { + execFileSync( + 'git', + ['push', 'origin', '--delete', testBranch], + { cwd: repoPath, stdio: 'pipe' }, + ); + } catch { + // Branch may not have been pushed; ignore + } + + // 2. Close any PRs from this branch (best-effort) + try { + const prList = execFileSync( + 'gh', + ['pr', 'list', '--head', testBranch, '--repo', repoName, '--json', 'number', '--jq', '.[].number'], + { encoding: 'utf-8', stdio: 'pipe' }, + ).trim(); + + for (const prNumber of prList.split('\n').filter(Boolean)) { + execFileSync( + 'gh', + ['pr', 'close', prNumber, '--repo', repoName, '--delete-branch'], + { stdio: 'pipe' }, + ); + } + } catch { + // No PRs or already closed; ignore + } + + // 3. Delete local directory last + try { + rmSync(repoPath, { recursive: true, force: true }); + } catch { + // Best-effort cleanup + } + }, + }; +} diff --git a/e2e/specs/add-and-run.e2e.ts b/e2e/specs/add-and-run.e2e.ts new file mode 100644 index 0000000..08dd0c4 --- /dev/null +++ b/e2e/specs/add-and-run.e2e.ts @@ -0,0 +1,72 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { readFileSync, existsSync, mkdirSync, writeFileSync } from 'node:fs'; +import { join, resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Add task and run (takt add → takt run)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should add a task file and execute it with takt run', () => { + const piecePath = resolve(__dirname, '../fixtures/pieces/simple.yaml'); + + // Step 1: Create a task file in .takt/tasks/ (simulates `takt add`) + const tasksDir = join(testRepo.path, '.takt', 'tasks'); + mkdirSync(tasksDir, { recursive: true }); + + const taskYaml = [ + 'task: "Add a single line \\"E2E test passed\\" to README.md"', + `piece: "${piecePath}"`, + ].join('\n'); + writeFileSync(join(tasksDir, 'e2e-test-task.yaml'), taskYaml, 'utf-8'); + + // Step 2: Run `takt run` to execute the pending task + const result = runTakt({ + args: ['run'], + cwd: testRepo.path, + env: isolatedEnv.env, + timeout: 240_000, + }); + + // Task should succeed + expect(result.exitCode).toBe(0); + + // Verify task was picked up and executed + expect(result.stdout).toContain('e2e-test-task'); + + // Verify README.md was modified + const readmePath = join(testRepo.path, 'README.md'); + expect(existsSync(readmePath)).toBe(true); + + const readme = readFileSync(readmePath, 'utf-8'); + expect(readme).toContain('E2E test passed'); + + // Verify task file was moved out of tasks/ (completed or failed) + expect(existsSync(join(tasksDir, 'e2e-test-task.yaml'))).toBe(false); + }, 240_000); +}); diff --git a/e2e/specs/add.e2e.ts b/e2e/specs/add.e2e.ts new file mode 100644 index 0000000..6cb0f92 --- /dev/null +++ b/e2e/specs/add.e2e.ts @@ -0,0 +1,95 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { readFileSync, readdirSync, writeFileSync } from 'node:fs'; +import { join, dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Add task from GitHub issue (takt add)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + let issueNumber: string; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + + // Use mock provider to stabilize summarizer + writeFileSync( + join(isolatedEnv.taktDir, 'config.yaml'), + [ + 'provider: mock', + 'model: mock-model', + 'language: en', + 'log_level: info', + 'default_piece: default', + ].join('\n'), + ); + + const createOutput = execFileSync( + 'gh', + [ + 'issue', 'create', + '--title', 'E2E Add Issue', + '--body', 'Add task via issue for E2E', + '--repo', testRepo.repoName, + ], + { encoding: 'utf-8' }, + ); + + const match = createOutput.match(/\/issues\/(\d+)/); + if (!match?.[1]) { + throw new Error(`Failed to extract issue number from: ${createOutput}`); + } + issueNumber = match[1]; + }); + + afterEach(() => { + try { + execFileSync('gh', ['issue', 'close', issueNumber, '--repo', testRepo.repoName], { stdio: 'pipe' }); + } catch { + // ignore + } + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should create a task file from issue reference', () => { + const scenarioPath = resolve(__dirname, '../fixtures/scenarios/add-task.json'); + + const result = runTakt({ + args: ['add', `#${issueNumber}`], + cwd: testRepo.path, + env: { + ...isolatedEnv.env, + TAKT_MOCK_SCENARIO: scenarioPath, + }, + input: 'n\n', + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + + const tasksDir = join(testRepo.path, '.takt', 'tasks'); + const files = readdirSync(tasksDir).filter((file) => file.endsWith('.yaml')); + expect(files.length).toBe(1); + + const taskFile = join(tasksDir, files[0] ?? ''); + const content = readFileSync(taskFile, 'utf-8'); + expect(content).toContain('issue:'); + }, 240_000); +}); diff --git a/e2e/specs/direct-task.e2e.ts b/e2e/specs/direct-task.e2e.ts new file mode 100644 index 0000000..c2a3f1c --- /dev/null +++ b/e2e/specs/direct-task.e2e.ts @@ -0,0 +1,56 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Direct task execution (--task --create-worktree no)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should execute a direct task without worktree prompts', () => { + const piecePath = resolve(__dirname, '../fixtures/pieces/mock-single-step.yaml'); + const scenarioPath = resolve(__dirname, '../fixtures/scenarios/execute-done.json'); + + const result = runTakt({ + args: [ + '--task', 'Create a file called noop.txt', + '--piece', piecePath, + '--create-worktree', 'no', + '--provider', 'mock', + ], + cwd: testRepo.path, + env: { + ...isolatedEnv.env, + TAKT_MOCK_SCENARIO: scenarioPath, + }, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Piece completed'); + }, 240_000); +}); diff --git a/e2e/specs/github-issue.e2e.ts b/e2e/specs/github-issue.e2e.ts new file mode 100644 index 0000000..2ed1995 --- /dev/null +++ b/e2e/specs/github-issue.e2e.ts @@ -0,0 +1,102 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: GitHub Issue processing', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + let issueNumber: string; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + + // Create a test issue + const createOutput = execFileSync( + 'gh', + [ + 'issue', 'create', + '--title', 'E2E Test Issue', + '--body', 'Create a file called issue-test.txt with the content "Issue resolved"', + '--repo', testRepo.repoName, + ], + { encoding: 'utf-8' }, + ); + + // Extract issue number from URL (e.g., https://github.com/user/repo/issues/123) + const match = createOutput.match(/\/issues\/(\d+)/); + if (!match?.[1]) { + throw new Error(`Failed to extract issue number from: ${createOutput}`); + } + issueNumber = match[1]; + }); + + afterEach(() => { + // Close test issue (best-effort) + try { + execFileSync( + 'gh', + ['issue', 'close', issueNumber, '--repo', testRepo.repoName], + { stdio: 'pipe' }, + ); + } catch { + // ignore + } + + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should execute pipeline from GitHub issue number', () => { + const piecePath = resolve(__dirname, '../fixtures/pieces/simple.yaml'); + + const result = runTakt({ + args: [ + '--pipeline', + '--issue', issueNumber, + '--piece', piecePath, + '--auto-pr', + '--repo', testRepo.repoName, + ], + cwd: testRepo.path, + env: isolatedEnv.env, + timeout: 240_000, + }); + + // Pipeline should succeed + expect(result.exitCode).toBe(0); + + // Verify issue was fetched + expect(result.stdout).toContain('Issue #'); + + // Verify piece completion + expect(result.stdout).toContain('completed'); + + // Verify PR was created + expect(result.stdout).toContain('PR created'); + + // Verify PR exists on GitHub + const prList = execFileSync( + 'gh', + ['pr', 'list', '--repo', testRepo.repoName, '--json', 'title', '--jq', '.[].title'], + { encoding: 'utf-8' }, + ).trim(); + expect(prList).toContain('E2E Test Issue'); + }, 240_000); +}); diff --git a/e2e/specs/list-non-interactive.e2e.ts b/e2e/specs/list-non-interactive.e2e.ts new file mode 100644 index 0000000..c7b9be8 --- /dev/null +++ b/e2e/specs/list-non-interactive.e2e.ts @@ -0,0 +1,129 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: List tasks non-interactive (takt list)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should show diff for a takt branch in non-interactive mode', () => { + const branchName = 'takt/e2e-list-diff'; + + execFileSync('git', ['checkout', '-b', branchName], { cwd: testRepo.path, stdio: 'pipe' }); + writeFileSync(join(testRepo.path, 'LIST_DIFF.txt'), 'diff e2e', 'utf-8'); + execFileSync('git', ['add', 'LIST_DIFF.txt'], { cwd: testRepo.path, stdio: 'pipe' }); + execFileSync('git', ['commit', '-m', 'takt: list diff e2e'], { cwd: testRepo.path, stdio: 'pipe' }); + execFileSync('git', ['checkout', testRepo.branch], { cwd: testRepo.path, stdio: 'pipe' }); + + const result = runTakt({ + args: ['list', '--non-interactive', '--action', 'diff', '--branch', branchName], + cwd: testRepo.path, + env: isolatedEnv.env, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('LIST_DIFF.txt'); + }, 240_000); + + it('should try-merge a takt branch in non-interactive mode', () => { + const branchName = 'takt/e2e-list-try'; + + execFileSync('git', ['checkout', '-b', branchName], { cwd: testRepo.path, stdio: 'pipe' }); + writeFileSync(join(testRepo.path, 'LIST_TRY.txt'), 'try e2e', 'utf-8'); + execFileSync('git', ['add', 'LIST_TRY.txt'], { cwd: testRepo.path, stdio: 'pipe' }); + execFileSync('git', ['commit', '-m', 'takt: list try e2e'], { cwd: testRepo.path, stdio: 'pipe' }); + execFileSync('git', ['checkout', testRepo.branch], { cwd: testRepo.path, stdio: 'pipe' }); + + const result = runTakt({ + args: ['list', '--non-interactive', '--action', 'try', '--branch', branchName], + cwd: testRepo.path, + env: isolatedEnv.env, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + + const status = execFileSync('git', ['status', '--porcelain'], { + cwd: testRepo.path, + encoding: 'utf-8', + stdio: 'pipe', + }); + expect(status).toContain('LIST_TRY.txt'); + }, 240_000); + + it('should merge a takt branch in non-interactive mode', () => { + const branchName = 'takt/e2e-list-merge'; + + execFileSync('git', ['checkout', '-b', branchName], { cwd: testRepo.path, stdio: 'pipe' }); + writeFileSync(join(testRepo.path, 'LIST_MERGE.txt'), 'merge e2e', 'utf-8'); + execFileSync('git', ['add', 'LIST_MERGE.txt'], { cwd: testRepo.path, stdio: 'pipe' }); + execFileSync('git', ['commit', '-m', 'takt: list merge e2e'], { cwd: testRepo.path, stdio: 'pipe' }); + execFileSync('git', ['checkout', testRepo.branch], { cwd: testRepo.path, stdio: 'pipe' }); + + const result = runTakt({ + args: ['list', '--non-interactive', '--action', 'merge', '--branch', branchName], + cwd: testRepo.path, + env: isolatedEnv.env, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + + const merged = execFileSync('git', ['branch', '--list', branchName], { + cwd: testRepo.path, + encoding: 'utf-8', + stdio: 'pipe', + }).trim(); + expect(merged).toBe(''); + }, 240_000); + + it('should delete a takt branch in non-interactive mode', () => { + const branchName = 'takt/e2e-list-test'; + + execFileSync('git', ['checkout', '-b', branchName], { cwd: testRepo.path, stdio: 'pipe' }); + writeFileSync(join(testRepo.path, 'LIST_E2E.txt'), 'list e2e', 'utf-8'); + execFileSync('git', ['add', 'LIST_E2E.txt'], { cwd: testRepo.path, stdio: 'pipe' }); + execFileSync('git', ['commit', '-m', 'takt: list e2e'], { cwd: testRepo.path, stdio: 'pipe' }); + execFileSync('git', ['checkout', testRepo.branch], { cwd: testRepo.path, stdio: 'pipe' }); + + const result = runTakt({ + args: ['list', '--non-interactive', '--action', 'delete', '--branch', branchName, '--yes'], + cwd: testRepo.path, + env: isolatedEnv.env, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + + const remaining = execFileSync('git', ['branch', '--list', branchName], { + cwd: testRepo.path, + encoding: 'utf-8', + stdio: 'pipe', + }).trim(); + expect(remaining).toBe(''); + }, 240_000); +}); diff --git a/e2e/specs/multi-step-parallel.e2e.ts b/e2e/specs/multi-step-parallel.e2e.ts new file mode 100644 index 0000000..6ea5f39 --- /dev/null +++ b/e2e/specs/multi-step-parallel.e2e.ts @@ -0,0 +1,79 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Multi-step with parallel movements (mock)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + const piecePath = resolve(__dirname, '../fixtures/pieces/multi-step-parallel.yaml'); + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should complete plan → review (all approved) → COMPLETE', () => { + const scenarioPath = resolve(__dirname, '../fixtures/scenarios/multi-step-all-approved.json'); + + const result = runTakt({ + args: [ + '--task', 'Implement a feature', + '--piece', piecePath, + '--create-worktree', 'no', + '--provider', 'mock', + ], + cwd: testRepo.path, + env: { + ...isolatedEnv.env, + TAKT_MOCK_SCENARIO: scenarioPath, + }, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Piece completed'); + }, 240_000); + + it('should complete plan → review (needs_fix) → fix → review (all approved) → COMPLETE', () => { + const scenarioPath = resolve(__dirname, '../fixtures/scenarios/multi-step-needs-fix.json'); + + const result = runTakt({ + args: [ + '--task', 'Implement a feature with issues', + '--piece', piecePath, + '--create-worktree', 'no', + '--provider', 'mock', + ], + cwd: testRepo.path, + env: { + ...isolatedEnv.env, + TAKT_MOCK_SCENARIO: scenarioPath, + }, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Piece completed'); + }, 240_000); +}); diff --git a/e2e/specs/pipeline-skip-git.e2e.ts b/e2e/specs/pipeline-skip-git.e2e.ts new file mode 100644 index 0000000..871e5fa --- /dev/null +++ b/e2e/specs/pipeline-skip-git.e2e.ts @@ -0,0 +1,57 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Pipeline mode with --skip-git', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should execute pipeline without git operations', () => { + const piecePath = resolve(__dirname, '../fixtures/pieces/mock-single-step.yaml'); + const scenarioPath = resolve(__dirname, '../fixtures/scenarios/execute-done.json'); + + const result = runTakt({ + args: [ + '--pipeline', + '--task', 'Create a file called noop.txt', + '--piece', piecePath, + '--skip-git', + '--provider', 'mock', + ], + cwd: testRepo.path, + env: { + ...isolatedEnv.env, + TAKT_MOCK_SCENARIO: scenarioPath, + }, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('completed'); + }, 240_000); +}); diff --git a/e2e/specs/pipeline.e2e.ts b/e2e/specs/pipeline.e2e.ts new file mode 100644 index 0000000..1143953 --- /dev/null +++ b/e2e/specs/pipeline.e2e.ts @@ -0,0 +1,68 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Pipeline mode (--pipeline --auto-pr)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should execute full CI pipeline: branch → piece → commit → push → PR', () => { + const piecePath = resolve(__dirname, '../fixtures/pieces/simple.yaml'); + + const result = runTakt({ + args: [ + '--pipeline', + '--task', 'Create a file called hello.txt with the content "Hello World"', + '--piece', piecePath, + '--auto-pr', + '--repo', testRepo.repoName, + ], + cwd: testRepo.path, + env: isolatedEnv.env, + timeout: 240_000, + }); + + // Pipeline should succeed + expect(result.exitCode).toBe(0); + + // Verify piece completion message + expect(result.stdout).toContain('completed'); + + // Verify PR was created + expect(result.stdout).toContain('PR created'); + + // Verify PR exists on GitHub + const prList = execFileSync( + 'gh', + ['pr', 'list', '--repo', testRepo.repoName, '--json', 'title', '--jq', '.[].title'], + { encoding: 'utf-8' }, + ).trim(); + expect(prList).toBeTruthy(); + }, 240_000); +}); diff --git a/e2e/specs/report-judge.e2e.ts b/e2e/specs/report-judge.e2e.ts new file mode 100644 index 0000000..21158a6 --- /dev/null +++ b/e2e/specs/report-judge.e2e.ts @@ -0,0 +1,56 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Report + Judge phases (mock)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should complete with report and judge phases using mock scenario', () => { + const piecePath = resolve(__dirname, '../fixtures/pieces/report-judge.yaml'); + const scenarioPath = resolve(__dirname, '../fixtures/scenarios/report-judge.json'); + + const result = runTakt({ + args: [ + '--task', 'Create a short report and finish', + '--piece', piecePath, + '--create-worktree', 'no', + '--provider', 'mock', + ], + cwd: testRepo.path, + env: { + ...isolatedEnv.env, + TAKT_MOCK_SCENARIO: scenarioPath, + }, + timeout: 240_000, + }); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Piece completed'); + }, 240_000); +}); diff --git a/e2e/specs/watch.e2e.ts b/e2e/specs/watch.e2e.ts new file mode 100644 index 0000000..3cb2162 --- /dev/null +++ b/e2e/specs/watch.e2e.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { spawn } from 'node:child_process'; +import { mkdirSync, writeFileSync, existsSync } from 'node:fs'; +import { join, resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Watch tasks (takt watch)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should execute a task added during watch', async () => { + const binPath = resolve(__dirname, '../../bin/takt'); + const scenarioPath = resolve(__dirname, '../fixtures/scenarios/execute-done.json'); + const piecePath = resolve(__dirname, '../fixtures/pieces/mock-single-step.yaml'); + + const child = spawn('node', [binPath, 'watch', '--provider', 'mock'], { + cwd: testRepo.path, + env: { + ...isolatedEnv.env, + TAKT_MOCK_SCENARIO: scenarioPath, + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + + let stdout = ''; + child.stdout?.on('data', (chunk) => { + stdout += chunk.toString(); + }); + + const tasksDir = join(testRepo.path, '.takt', 'tasks'); + mkdirSync(tasksDir, { recursive: true }); + + const taskYaml = [ + 'task: "Add a single line \\\"watch test\\\" to README.md"', + `piece: "${piecePath}"`, + ].join('\n'); + + const taskPath = join(tasksDir, 'watch-task.yaml'); + writeFileSync(taskPath, taskYaml, 'utf-8'); + + const completed = await new Promise((resolvePromise) => { + const timeout = setTimeout(() => resolvePromise(false), 240_000); + const interval = setInterval(() => { + if (stdout.includes('Task "watch-task" completed')) { + clearTimeout(timeout); + clearInterval(interval); + resolvePromise(true); + } + }, 250); + }); + + child.kill('SIGINT'); + + await new Promise((resolvePromise) => { + const timeout = setTimeout(() => { + child.kill('SIGKILL'); + resolvePromise(); + }, 30_000); + child.on('close', () => { + clearTimeout(timeout); + resolvePromise(); + }); + }); + + expect(completed).toBe(true); + expect(existsSync(taskPath)).toBe(false); + }, 240_000); +}); diff --git a/e2e/specs/worktree.e2e.ts b/e2e/specs/worktree.e2e.ts new file mode 100644 index 0000000..d8e6b51 --- /dev/null +++ b/e2e/specs/worktree.e2e.ts @@ -0,0 +1,51 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createIsolatedEnv, type IsolatedEnv } from '../helpers/isolated-env'; +import { createTestRepo, type TestRepo } from '../helpers/test-repo'; +import { runTakt } from '../helpers/takt-runner'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// E2E更新時は docs/testing/e2e.md も更新すること +describe('E2E: Worktree/Clone isolation (--create-worktree yes)', () => { + let isolatedEnv: IsolatedEnv; + let testRepo: TestRepo; + + beforeEach(() => { + isolatedEnv = createIsolatedEnv(); + testRepo = createTestRepo(); + }); + + afterEach(() => { + try { + testRepo.cleanup(); + } catch { + // best-effort + } + try { + isolatedEnv.cleanup(); + } catch { + // best-effort + } + }); + + it('should execute task in an isolated worktree/clone', () => { + const piecePath = resolve(__dirname, '../fixtures/pieces/simple.yaml'); + + const result = runTakt({ + args: [ + '--task', 'Add a line "worktree test" to README.md', + '--piece', piecePath, + '--create-worktree', 'yes', + ], + cwd: testRepo.path, + env: isolatedEnv.env, + timeout: 240_000, + }); + + // Task should succeed + expect(result.exitCode).toBe(0); + }, 240_000); +}); diff --git a/eslint.config.js b/eslint.config.js index 9e46319..d556006 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -15,7 +15,7 @@ export default tseslint.config( '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], '@typescript-eslint/no-explicit-any': 'error', '@typescript-eslint/explicit-function-return-type': 'off', - '@typescript-eslint/no-non-null-assertion': 'warn', + '@typescript-eslint/no-non-null-assertion': 'off', }, }, { diff --git a/package.json b/package.json index 129771e..5df0be7 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,14 @@ "watch": "tsc --watch", "test": "vitest run", "test:watch": "vitest", + "test:e2e": "npm run test:e2e:all", + "test:e2e:mock": "vitest run --config vitest.config.e2e.mock.ts --reporter=verbose", + "test:e2e:all": "npm run test:e2e:mock && npm run test:e2e:provider", + "test:e2e:provider": "npm run test:e2e:provider:claude && npm run test:e2e:provider:codex", + "test:e2e:provider:claude": "TAKT_E2E_PROVIDER=claude vitest run --config vitest.config.e2e.provider.ts --reporter=verbose", + "test:e2e:provider:codex": "TAKT_E2E_PROVIDER=codex vitest run --config vitest.config.e2e.provider.ts --reporter=verbose", + "test:e2e:claude": "npm run test:e2e:provider:claude", + "test:e2e:codex": "npm run test:e2e:provider:codex", "lint": "eslint src/", "prepublishOnly": "npm run lint && npm run build && npm run test", "postversion": "git push --follow-tags" diff --git a/resources/global/en/agents/default/ai-antipattern-reviewer.md b/resources/global/en/agents/default/ai-antipattern-reviewer.md index 68fa438..818248b 100644 --- a/resources/global/en/agents/default/ai-antipattern-reviewer.md +++ b/resources/global/en/agents/default/ai-antipattern-reviewer.md @@ -186,13 +186,36 @@ function execute(task, options?: { projectCwd?: string }) { | **REJECT** | Setter/getter created "for symmetry" but never used | | **REJECT** | Interface or option prepared for future extension | | **REJECT** | Exported but grep finds no usage | +| **REJECT** | Defensive code for logically unreachable paths (see below) | | OK | Implicitly called by framework (lifecycle hooks, etc.) | | OK | Intentionally published as public package API | +**Logically dead defensive code:** + +AI tends to add "just in case" guards without analyzing caller constraints. Code that is syntactically reachable but logically unreachable through actual call paths must be removed. + +```typescript +// ❌ REJECT - All callers go through an interactive menu that requires TTY +// This function can never be called without TTY +function showFullDiff(cwd: string, branch: string): void { + const usePager = process.stdin.isTTY === true; + // usePager is always true (callers guarantee TTY) + const pager = usePager ? 'less -R' : 'cat'; // else branch is unreachable +} + +// ✅ OK - Understand caller constraints, remove unnecessary branches +function showFullDiff(cwd: string, branch: string): void { + // Only called from interactive menu, TTY is always present + spawnSync('git', ['diff', ...], { env: { GIT_PAGER: 'less -R' } }); +} +``` + **Verification approach:** -1. Verify with grep that no references exist to changed/deleted code -2. Verify that public module (index files, etc.) export lists match actual implementations -3. Check that old code corresponding to newly added code has been removed +1. When you find a defensive branch, grep all callers of that function +2. If all callers already guarantee the condition, the guard is unnecessary +3. Verify with grep that no references exist to changed/deleted code +4. Verify that public module (index files, etc.) export lists match actual implementations +5. Check that old code corresponding to newly added code has been removed ### 8. Unnecessary Backward Compatibility Code Detection diff --git a/resources/global/en/agents/default/architecture-reviewer.md b/resources/global/en/agents/default/architecture-reviewer.md index 1d50b39..538133d 100644 --- a/resources/global/en/agents/default/architecture-reviewer.md +++ b/resources/global/en/agents/default/architecture-reviewer.md @@ -260,6 +260,22 @@ function updateConfig(config: Config): Config { | Over-generalization | Variants and extension points not currently needed | | Hidden Dependencies | Child components implicitly calling APIs etc. | | Non-idiomatic | Custom implementation ignoring language/FW conventions | +| Logically dead defensive code | Guards for conditions already guaranteed by all callers | + +**Logically dead defensive code:** + +Call chain verification applies not only to "missing wiring" but also to the reverse — **unnecessary guards for conditions that callers already guarantee**. + +| Pattern | Problem | Detection | +|---------|---------|-----------| +| TTY check when all callers require TTY | Unreachable branch remains | grep all callers' preconditions | +| Null guard when callers already check null | Redundant defense | Trace caller constraints | +| Runtime type check when TypeScript types constrain | Not trusting type safety | Check TypeScript type constraints | + +**Verification:** +1. When you find a defensive branch (TTY check, null guard, etc.), grep all callers +2. If all callers already guarantee the condition, the guard is unnecessary → **REJECT** +3. If some callers don't guarantee it, keep the guard ### 6. Abstraction Level Evaluation diff --git a/resources/global/ja/agents/default/ai-antipattern-reviewer.md b/resources/global/ja/agents/default/ai-antipattern-reviewer.md index 10e21a1..c04fe9e 100644 --- a/resources/global/ja/agents/default/ai-antipattern-reviewer.md +++ b/resources/global/ja/agents/default/ai-antipattern-reviewer.md @@ -124,15 +124,38 @@ AIは自信を持って間違える——もっともらしく見えるが動か | 未使用の関数・メソッド | リファクタリング後に残った旧実装 | | 未使用の変数・定数 | 条件変更で不要になった定義 | | 到達不能コード | 早期returnの後に残った処理、常に真/偽になる条件分岐 | +| 論理的に到達不能な防御コード | 呼び出し元の制約で絶対に通らない分岐（後述） | | 未使用のインポート・依存 | 削除された機能のimport文やパッケージ依存 | | 孤立したエクスポート・公開API | 実体が消えたのにre-exportやindex登録が残っている | | 未使用のインターフェース・型定義 | 実装側が変更されたのに残った古い型 | | 無効化されたコード | コメントアウトされたまま放置されたコード | +**論理的デッドコードの検出:** + +AIは「念のため」の防御コードを追加しがちだが、呼び出し元の制約を考慮すると到達不能な場合がある。構文的には到達可能でも、呼び出しチェーンの前提条件により論理的に到達しないコードは削除する。 + +```typescript +// ❌ REJECT - 呼び出し元がTTY必須のインタラクティブメニュー経由のみ +// TTYがない環境からこの関数が呼ばれることはない +function showFullDiff(cwd: string, branch: string): void { + const usePager = process.stdin.isTTY === true; + // usePager は常に true（呼び出し元がTTYを前提としている） + const pager = usePager ? 'less -R' : 'cat'; // else節は到達不能 +} + +// ✅ OK - 呼び出し元の制約を理解し、不要な分岐を排除 +function showFullDiff(cwd: string, branch: string): void { + // インタラクティブメニューからのみ呼ばれるためTTYは常に存在する + spawnSync('git', ['diff', ...], { env: { GIT_PAGER: 'less -R' } }); +} +``` + **検証アプローチ:** -1. 変更・削除されたコードを参照している箇所がないか grep で確認 -2. 公開モジュール（index ファイル等）のエクスポート一覧と実体が一致しているか確認 -3. 新規追加されたコードに対応する古いコードが残っていないか確認 +1. 防御的な分岐を見つけたら、grep でその関数の全呼び出し元を確認 +2. 全呼び出し元が既にその条件を満たしている場合、防御は不要 +3. 変更・削除されたコードを参照している箇所がないか grep で確認 +4. 公開モジュール（index ファイル等）のエクスポート一覧と実体が一致しているか確認 +5. 新規追加されたコードに対応する古いコードが残っていないか確認 ### 7. フォールバック・デフォルト引数禁止レビュー（REJECT基準） diff --git a/resources/global/ja/agents/default/architecture-reviewer.md b/resources/global/ja/agents/default/architecture-reviewer.md index 7bde241..635f802 100644 --- a/resources/global/ja/agents/default/architecture-reviewer.md +++ b/resources/global/ja/agents/default/architecture-reviewer.md @@ -527,6 +527,21 @@ export async function executePiece(config, cwd, task, options?) { **このパターンを見つけたら REJECT。** 個々のファイルが正しくても、結合されていなければ機能しない。 +**呼び出し元の制約による論理的デッドコード:** + +呼び出しチェーンの検証は「配線漏れ」だけでなく、逆方向——**呼び出し元が既に保証している条件に対する不要な防御コード**——にも適用する。 + +| パターン | 問題 | 検出方法 | +|---------|------|---------| +| 呼び出し元がTTY必須なのに関数内でTTYチェック | 到達しない分岐が残る | grep で全呼び出し元の前提条件を確認 | +| 呼び出し元がnullチェック済みなのに再度nullガード | 冗長な防御 | 呼び出し元の制約を追跡 | +| 呼び出し元が型で制約しているのにランタイムチェック | 型安全を信頼していない | TypeScriptの型制約を確認 | + +**検証手順:** +1. 防御的な条件分岐（TTYチェック、nullガード等）を見つけたら、grep で全呼び出し元を確認 +2. 全呼び出し元がその条件を既に保証しているなら、防御は不要 → **REJECT** +3. 一部の呼び出し元が保証していない場合は、防御を残す + ### 10. 品質特性 | 特性 | 確認観点 | diff --git a/src/__tests__/e2e-helpers.test.ts b/src/__tests__/e2e-helpers.test.ts new file mode 100644 index 0000000..c94124e --- /dev/null +++ b/src/__tests__/e2e-helpers.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { injectProviderArgs } from '../../e2e/helpers/takt-runner.js'; +import { createIsolatedEnv } from '../../e2e/helpers/isolated-env.js'; + +describe('injectProviderArgs', () => { + it('should prepend --provider when provider is specified', () => { + const result = injectProviderArgs(['run', '--pipeline'], 'codex'); + expect(result).toEqual(['--provider', 'codex', 'run', '--pipeline']); + }); + + it('should not prepend --provider when args already contain --provider', () => { + const result = injectProviderArgs( + ['--provider', 'claude', 'run', '--pipeline'], + 'codex', + ); + expect(result).toEqual(['--provider', 'claude', 'run', '--pipeline']); + }); + + it('should return a copy of args when provider is undefined', () => { + const result = injectProviderArgs(['run', '--pipeline'], undefined); + expect(result).toEqual(['run', '--pipeline']); + }); + + it('should return a copy of args when provider is empty string', () => { + const result = injectProviderArgs(['run', '--pipeline'], ''); + expect(result).toEqual(['run', '--pipeline']); + }); +}); + +describe('createIsolatedEnv', () => { + const originalEnv = process.env; + let cleanups: Array<() => void> = []; + + afterEach(() => { + process.env = originalEnv; + for (const cleanup of cleanups) { + cleanup(); + } + cleanups = []; + }); + + it('should inherit TAKT_OPENAI_API_KEY from process.env', () => { + process.env = { ...originalEnv, TAKT_OPENAI_API_KEY: 'test-key-123' }; + const isolated = createIsolatedEnv(); + cleanups.push(isolated.cleanup); + + expect(isolated.env.TAKT_OPENAI_API_KEY).toBe('test-key-123'); + }); + + it('should not include TAKT_OPENAI_API_KEY when not in process.env', () => { + process.env = { ...originalEnv }; + delete process.env.TAKT_OPENAI_API_KEY; + const isolated = createIsolatedEnv(); + cleanups.push(isolated.cleanup); + + expect(isolated.env.TAKT_OPENAI_API_KEY).toBeUndefined(); + }); + + it('should override TAKT_CONFIG_DIR with isolated directory', () => { + const isolated = createIsolatedEnv(); + cleanups.push(isolated.cleanup); + + expect(isolated.env.TAKT_CONFIG_DIR).toBe(isolated.taktDir); + }); + + it('should set GIT_CONFIG_GLOBAL to isolated path', () => { + const isolated = createIsolatedEnv(); + cleanups.push(isolated.cleanup); + + expect(isolated.env.GIT_CONFIG_GLOBAL).toBeDefined(); + expect(isolated.env.GIT_CONFIG_GLOBAL).toContain('takt-e2e-'); + }); +}); diff --git a/src/__tests__/test-setup.ts b/src/__tests__/test-setup.ts new file mode 100644 index 0000000..924773e --- /dev/null +++ b/src/__tests__/test-setup.ts @@ -0,0 +1,3 @@ +if (process.env.TAKT_TEST_FLG_TOUCH_TTY !== '1') { + process.env.TAKT_NO_TTY = '1'; +} diff --git a/src/app/cli/commands.ts b/src/app/cli/commands.ts index 8ac7951..c8a6512 100644 --- a/src/app/cli/commands.ts +++ b/src/app/cli/commands.ts @@ -38,8 +38,23 @@ program program .command('list') .description('List task branches (merge/delete)') - .action(async () => { - await listTasks(resolvedCwd, resolveAgentOverrides(program)); + .option('--non-interactive', 'Run list in non-interactive mode') + .option('--action ', 'Non-interactive action (diff|try|merge|delete)') + .option('--format ', 'Output format for non-interactive list (text|json)') + .option('--yes', 'Skip confirmation prompts in non-interactive mode') + .action(async (_opts, command) => { + const opts = command.optsWithGlobals(); + await listTasks( + resolvedCwd, + resolveAgentOverrides(program), + { + enabled: opts.nonInteractive === true, + action: opts.action as string | undefined, + branch: opts.branch as string | undefined, + format: opts.format as string | undefined, + yes: opts.yes === true, + }, + ); }); program diff --git a/src/app/cli/routing.ts b/src/app/cli/routing.ts index eb99f9d..5148d6e 100644 --- a/src/app/cli/routing.ts +++ b/src/app/cli/routing.ts @@ -7,7 +7,7 @@ import { info, error } from '../../shared/ui/index.js'; import { getErrorMessage } from '../../shared/utils/index.js'; -import { resolveIssueTask, isIssueReference } from '../../infra/github/index.js'; +import { resolveIssueTask } from '../../infra/github/index.js'; import { selectAndExecuteTask, determinePiece, type SelectAndExecuteOptions } from '../../features/tasks/index.js'; import { executePipeline } from '../../features/pipeline/index.js'; import { interactiveMode } from '../../features/interactive/index.js'; diff --git a/src/features/tasks/list/index.ts b/src/features/tasks/list/index.ts index b11233b..5195654 100644 --- a/src/features/tasks/list/index.ts +++ b/src/features/tasks/list/index.ts @@ -5,6 +5,7 @@ * Individual actions (merge, delete, instruct, diff) are in taskActions.ts. */ +import { execFileSync } from 'node:child_process'; import { detectDefaultBranch, listTaktBranches, @@ -14,6 +15,7 @@ import { selectOption, confirm } from '../../../shared/prompt/index.js'; import { info } from '../../../shared/ui/index.js'; import { createLogger } from '../../../shared/utils/index.js'; import type { TaskExecutionOptions } from '../execute/types.js'; +import type { BranchListItem } from '../../../infra/task/index.js'; import { type ListAction, showFullDiff, @@ -36,12 +38,115 @@ export { const log = createLogger('list-tasks'); +export interface ListNonInteractiveOptions { + enabled: boolean; + action?: string; + branch?: string; + format?: string; + yes?: boolean; +} + +function isValidAction(action: string): action is ListAction { + return action === 'diff' || action === 'try' || action === 'merge' || action === 'delete'; +} + +function printNonInteractiveList(items: BranchListItem[], format?: string): void { + const outputFormat = format ?? 'text'; + if (outputFormat === 'json') { + console.log(JSON.stringify(items, null, 2)); + return; + } + + for (const item of items) { + const worktreeLabel = item.info.worktreePath ? ' (worktree)' : ''; + const instruction = item.originalInstruction ? ` - ${item.originalInstruction}` : ''; + console.log(`${item.info.branch}${worktreeLabel} (${item.filesChanged} files)${instruction}`); + } +} + +function showDiffStat(projectDir: string, defaultBranch: string, branch: string): void { + try { + const stat = execFileSync( + 'git', ['diff', '--stat', `${defaultBranch}...${branch}`], + { cwd: projectDir, encoding: 'utf-8', stdio: 'pipe' }, + ); + console.log(stat); + } catch { + info('Could not generate diff stat'); + } +} + +async function listTasksNonInteractive( + cwd: string, + _options: TaskExecutionOptions | undefined, + nonInteractive: ListNonInteractiveOptions, +): Promise { + const defaultBranch = detectDefaultBranch(cwd); + const branches = listTaktBranches(cwd); + + if (branches.length === 0) { + info('No tasks to list.'); + return; + } + + const items = buildListItems(cwd, branches, defaultBranch); + + if (!nonInteractive.action) { + printNonInteractiveList(items, nonInteractive.format); + return; + } + + if (!nonInteractive.branch) { + info('Missing --branch for non-interactive action.'); + process.exit(1); + } + + if (!isValidAction(nonInteractive.action)) { + info('Invalid --action. Use one of: diff, try, merge, delete.'); + process.exit(1); + } + + const item = items.find((entry) => entry.info.branch === nonInteractive.branch); + if (!item) { + info(`Branch not found: ${nonInteractive.branch}`); + process.exit(1); + } + + switch (nonInteractive.action) { + case 'diff': + showDiffStat(cwd, defaultBranch, item.info.branch); + return; + case 'try': + tryMergeBranch(cwd, item); + return; + case 'merge': + mergeBranch(cwd, item); + return; + case 'delete': + if (!nonInteractive.yes) { + info('Delete requires --yes in non-interactive mode.'); + process.exit(1); + } + deleteBranch(cwd, item); + return; + } +} + /** * Main entry point: list branch-based tasks interactively. */ -export async function listTasks(cwd: string, options?: TaskExecutionOptions): Promise { +export async function listTasks( + cwd: string, + options?: TaskExecutionOptions, + nonInteractive?: ListNonInteractiveOptions, +): Promise { log.info('Starting list-tasks'); + if (nonInteractive?.enabled) { + await listTasksNonInteractive(cwd, options, nonInteractive); + return; + } + const defaultBranch = detectDefaultBranch(cwd); let branches = listTaktBranches(cwd); diff --git a/src/features/tasks/list/taskActions.ts b/src/features/tasks/list/taskActions.ts index 617fa64..bdc1002 100644 --- a/src/features/tasks/list/taskActions.ts +++ b/src/features/tasks/list/taskActions.ts @@ -62,7 +62,11 @@ export function showFullDiff( try { const result = spawnSync( 'git', ['diff', '--color=always', `${defaultBranch}...${branch}`], - { cwd, stdio: ['inherit', 'inherit', 'inherit'], env: { ...process.env, GIT_PAGER: 'less -R' } }, + { + cwd, + stdio: 'inherit', + env: { ...process.env, GIT_PAGER: 'less -R' }, + }, ); if (result.status !== 0) { warn('Could not display diff'); @@ -148,10 +152,14 @@ export function mergeBranch(projectDir: string, item: BranchListItem): boolean { info(`${branch} is already merged, skipping merge.`); log.info('Branch already merged, cleanup only', { branch }); } else { - execFileSync('git', ['merge', branch], { + execFileSync('git', ['merge', '--no-edit', branch], { cwd: projectDir, encoding: 'utf-8', stdio: 'pipe', + env: { + ...process.env, + GIT_MERGE_AUTOEDIT: 'no', + }, }); } diff --git a/src/infra/codex/client.ts b/src/infra/codex/client.ts index 41c953f..d024fc3 100644 --- a/src/infra/codex/client.ts +++ b/src/infra/codex/client.ts @@ -42,7 +42,7 @@ export class CodexClient { ? mapToCodexSandboxMode(options.permissionMode) : 'workspace-write'; const threadOptions = { - model: options.model, + ...(options.model ? { model: options.model } : {}), workingDirectory: options.cwd, sandboxMode, }; diff --git a/src/infra/config/paths.ts b/src/infra/config/paths.ts index a4392c3..8b49b54 100644 --- a/src/infra/config/paths.ts +++ b/src/infra/config/paths.ts @@ -11,9 +11,9 @@ import { existsSync, mkdirSync } from 'node:fs'; import type { Language } from '../../core/models/index.js'; import { getLanguageResourcesDir } from '../resources/index.js'; -/** Get takt global config directory (~/.takt) */ +/** Get takt global config directory (~/.takt or TAKT_CONFIG_DIR) */ export function getGlobalConfigDir(): string { - return join(homedir(), '.takt'); + return process.env.TAKT_CONFIG_DIR || join(homedir(), '.takt'); } /** Get takt global agents directory (~/.takt/agents) */ diff --git a/src/infra/task/branchList.ts b/src/infra/task/branchList.ts index 695be50..53373c2 100644 --- a/src/infra/task/branchList.ts +++ b/src/infra/task/branchList.ts @@ -30,8 +30,8 @@ export class BranchManager { 'git', ['symbolic-ref', 'refs/remotes/origin/HEAD'], { cwd, encoding: 'utf-8', stdio: 'pipe' }, ).trim(); - const parts = ref.split('/'); - return parts[parts.length - 1] || 'main'; + const prefix = 'refs/remotes/origin/'; + return ref.startsWith(prefix) ? ref.slice(prefix.length) : ref; } catch { try { execFileSync('git', ['rev-parse', '--verify', 'main'], { diff --git a/src/shared/prompt/confirm.ts b/src/shared/prompt/confirm.ts index 6efbfd9..bb82334 100644 --- a/src/shared/prompt/confirm.ts +++ b/src/shared/prompt/confirm.ts @@ -7,12 +7,18 @@ import * as readline from 'node:readline'; import chalk from 'chalk'; +import { resolveTtyPolicy, assertTtyIfForced } from './tty.js'; /** * Prompt user for simple text input * @returns User input or null if cancelled */ export async function promptInput(message: string): Promise { + const { useTty, forceTouchTty } = resolveTtyPolicy(); + assertTtyIfForced(forceTouchTty); + if (!useTty) { + return null; + } const rl = readline.createInterface({ input: process.stdin, output: process.stdout, @@ -77,6 +83,11 @@ export function readMultilineFromStream(input: NodeJS.ReadableStream): Promise { + const { useTty, forceTouchTty } = resolveTtyPolicy(); + assertTtyIfForced(forceTouchTty); + if (!useTty) { + return defaultYes; + } const rl = readline.createInterface({ input: process.stdin, output: process.stdout, diff --git a/src/shared/prompt/select.ts b/src/shared/prompt/select.ts index 182fe85..03d7ebe 100644 --- a/src/shared/prompt/select.ts +++ b/src/shared/prompt/select.ts @@ -6,6 +6,7 @@ import chalk from 'chalk'; import { truncateText } from '../utils/index.js'; +import { resolveTtyPolicy, assertTtyIfForced } from './tty.js'; /** Option type for selectOption */ export interface SelectOptionItem { @@ -208,7 +209,9 @@ function interactiveSelect( const lines = renderMenu(currentOptions, selectedIndex, hasCancelOption, cancelLabel); process.stdout.write(lines.join('\n') + '\n'); - if (!process.stdin.isTTY) { + const { useTty, forceTouchTty } = resolveTtyPolicy(); + assertTtyIfForced(forceTouchTty); + if (!useTty) { process.stdout.write('\x1B[?7h'); resolve({ selectedIndex: initialIndex, finalOptions: currentOptions }); return; diff --git a/src/shared/prompt/tty.ts b/src/shared/prompt/tty.ts new file mode 100644 index 0000000..bc5c279 --- /dev/null +++ b/src/shared/prompt/tty.ts @@ -0,0 +1,17 @@ +export interface TtyPolicy { + useTty: boolean; + forceTouchTty: boolean; +} + +export function resolveTtyPolicy(): TtyPolicy { + const forceTouchTty = process.env.TAKT_TEST_FLG_TOUCH_TTY === '1'; + const forceNoTty = process.env.TAKT_NO_TTY === '1'; + const useTty = process.stdin.isTTY && (!forceNoTty || forceTouchTty); + return { useTty, forceTouchTty }; +} + +export function assertTtyIfForced(forceTouchTty: boolean): void { + if (forceTouchTty && !process.stdin.isTTY) { + throw new Error('TAKT_TEST_FLG_TOUCH_TTY=1 requires a TTY'); + } +} diff --git a/vitest.config.e2e.mock.ts b/vitest.config.e2e.mock.ts new file mode 100644 index 0000000..94d2c03 --- /dev/null +++ b/vitest.config.e2e.mock.ts @@ -0,0 +1,26 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + include: [ + 'e2e/specs/direct-task.e2e.ts', + 'e2e/specs/pipeline-skip-git.e2e.ts', + 'e2e/specs/report-judge.e2e.ts', + 'e2e/specs/add.e2e.ts', + 'e2e/specs/watch.e2e.ts', + 'e2e/specs/list-non-interactive.e2e.ts', + 'e2e/specs/multi-step-parallel.e2e.ts', + ], + environment: 'node', + globals: false, + testTimeout: 240000, + hookTimeout: 60000, + teardownTimeout: 30000, + pool: 'threads', + poolOptions: { + threads: { + singleThread: true, + }, + }, + }, +}); diff --git a/vitest.config.e2e.provider.ts b/vitest.config.e2e.provider.ts new file mode 100644 index 0000000..84c2932 --- /dev/null +++ b/vitest.config.e2e.provider.ts @@ -0,0 +1,23 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + include: [ + 'e2e/specs/add-and-run.e2e.ts', + 'e2e/specs/worktree.e2e.ts', + 'e2e/specs/pipeline.e2e.ts', + 'e2e/specs/github-issue.e2e.ts', + ], + environment: 'node', + globals: false, + testTimeout: 240000, + hookTimeout: 60000, + teardownTimeout: 30000, + pool: 'threads', + poolOptions: { + threads: { + singleThread: true, + }, + }, + }, +}); diff --git a/vitest.config.e2e.ts b/vitest.config.e2e.ts new file mode 100644 index 0000000..e3c715f --- /dev/null +++ b/vitest.config.e2e.ts @@ -0,0 +1,18 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + include: ['e2e/specs/**/*.e2e.ts'], + environment: 'node', + globals: false, + testTimeout: 240000, + hookTimeout: 60000, + teardownTimeout: 30000, + pool: 'threads', + poolOptions: { + threads: { + singleThread: true, + }, + }, + }, +}); diff --git a/vitest.config.ts b/vitest.config.ts index c100b89..4c0ab1b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -5,6 +5,7 @@ export default defineConfig({ include: ['src/__tests__/**/*.test.ts'], environment: 'node', globals: false, + setupFiles: ['src/__tests__/test-setup.ts'], // Ensure proper cleanup by forcing sequential execution and graceful shutdown pool: 'threads', poolOptions: {