Skip to content

[#584] split-claude-providers#585

Merged
nrslib merged 7 commits intomainfrom
takt/584/split-claude-providers
Apr 5, 2026
Merged

[#584] split-claude-providers#585
nrslib merged 7 commits intomainfrom
takt/584/split-claude-providers

Conversation

@nrslib
Copy link
Copy Markdown
Owner

@nrslib nrslib commented Apr 4, 2026

Summary

タスク指示書: Claude プロバイダの分割(claude-sdk / ヘッドレス claude

目的

  • 現状 SDK 経路で動いている Claude 利用を provider: claude-sdk に寄せる。
  • provider: claude は、ローカルの claude CLIヘッドレス(-p を使う別プロバイダにする。
  • 移行用の別名・エイリアスは置かない。後方互換は入れない(旧 claude = SDK の意味は残さない)。
  • 未設定時のデフォルトプロバイダは claude(ヘッドレス) とする。

参照資料

  • ユーザー指定の参照資料はなし。実装・仕様の裏取りは リポジトリ内の現行コード と、必要に応じて ローカル環境の claude --help / 実際の CLI 挙動 をソースとする。

ユーザーが明示した制約・前提

  • 別名に残さないclaude を SDK のまま残す等はしない)。
  • claude はヘッドレスにしたい
  • 後方互換はない(旧設定・旧プロバイダ名の互換レイヤーは不要)。
  • 実行前提は PATH 上の claude コマンドが動くこと(ユーザーはパス差し替えを要求していない)。
  • ヘッドレス側で必須: 権限モード等のマッピングストリーミング/長時間実行
  • セッション継続(resume)・ツール/MCPできれば欲しいが、技術的に無理なら必須ではない
  • デフォルトプロバイダは claude

優先度付き作業(モジュール/ファイル観点)

  • src/infra/providers/(Provider 登録・解決)
    • プロバイダ識別子に claude-sdkclaude を正式に追加し、既存の「claude = SDK」配線を claude-sdk に付け替える。
    • claude は新規のヘッドレス実装に接続する。
  • src/infra/claude/(既存 SDK 実装の整理)
    • 現行 SDK 利用コードを claude-sdk プロバイダとして明確化(ファイル分割・リネームは計画で最適化。責務は「SDK」に限定)。
  • 新規: ヘッドレス claude プロバイダ実装(配置は計画で決定。例: src/infra/claude-headless/ または src/infra/claude/cli.ts 等)
    • claude サブプロセス起動-p を用いた非対話実行。
    • TAKT の権限モード(および既存の provider プロファイル概念)を、CLI が提供するフラグ/挙動必須でマッピングする(マッピング不能なら計画で明示し、可能な範囲で実装+残りは Open Questions に落とす)。
    • ストリーミング出力の取り込みと、長時間実行(アイドル/タイムアウト方針は Codex 等の既存パターンに倣いつつ、CLI 特性に合わせて決定)。
    • Provider / ProviderAgent 契約AgentResponse、エラー伝播、セッションキー等)に SDK プロバイダと同等の失敗時診断が出るよう整える(空の blocked 等を避ける)。
  • 設定・スキーマ・型
    • global / project 設定、Zod スキーマ、CLI の --provider 説明・列挙を claude-sdk / claude に更新。
    • デフォルト provider を claude に変更(グローバル設定テンプレ・ローダ・ドキュメント上の記述も追随)。
  • ピース/ビルトイン/エクスポート系の文言・デフォルト
    • builtins/ や skill エクスポート、E2E テストで provider: claude を前提にしている箇所を、新しい意味(ヘッドレス)claude-sdk 需要に合わせて更新。
  • テスト
    • 既存の mock/provider テストパターンに合わせ、プロバイダ名変更ヘッドレス起動・出力パース・エラーのテストを追加・更新。
    • 実 CLI に依存しすぎるテストは、固定フィクスチャ+ subprocess モック等で可能な範囲に留める(方針は計画で確定)。

  • セッション継続(resume)
    • CLI がサポートする範囲で resumeFrom 相当を配線。無理なら 未対応として明示的に挙動定義(黙って壊さない/ログに理由)。
  • ツール/MCP
    • CLI 側で成立する範囲で配線。成立しない場合はスコープ外として文書化(ユーザーは必須としていない)。

  • 運用ドキュメント(docs/
    • ユーザーはドキュメント作成を指示していない。コード上のヘルプ・設定例の整合が最優先。余力があれば最小限の追記のみ(指示書の主目的は実装)。

再現・確認方法

  1. ビルド: npm run build
  2. 単体テスト: npm test
  3. 手動スモーク(開発者環境):
    • takt --help および設定で provider claude がヘッドレス経路を選ぶこと。
    • 同一タスクを provider claude-sdk で実行し、SDK 経路が生きていること。
    • 権限モードを変えたとき、ヘッドレス側のフラグ/挙動が期待どおり変わること(具体フラグは CLI 調査結果に基づく)。
  4. E2E(該当する場合): npm run test:e2e:mock および既存の provider E2E ラベルに合わせて更新後に実行。

やらないこと(ユーザー明示のみ)

  • 後方互換・移行別名(旧 claude = SDK の維持など)は実装しない
  • ユーザーは 設定で claude バイナリパスを差し替えることを要求していない(必須スコープに含めない)。

Open Questions(技術的な不明点のみ)

  • ローカルの claude CLI が、TAKT が必要とする 権限モード・サンドボックス相当どのフラグ/組み合わせで表現できるかclaude --help と実挙動で確定すること)。
  • ストリーミングのプロトコル(行単位 JSON、SSE 等)と パース方針(既存 SDK ストリーム処理との共通化可否)。
  • 長時間実行時の アイドルタイムアウト部分出力の扱い(CLI の仕様に依存)。
  • resumeツール/MCP が CLI 経由で どこまで再現可能か(不可能なら「未対応」の定義とユーザー向けエラーメッセージ)。

Execution Report

Workflow takt-default completed successfully.

Closes #584

Copy link
Copy Markdown
Owner Author

nrslib commented Apr 4, 2026

レビュー結果です。現状のままではマージしない方がよいです。主に以下の3点を修正対象として整理するのが妥当です。

  1. headless claude が新規 session を確立・返却していない
  • src/infra/claude-headless/client.ts では、既存 sessionId があるときだけ --resume し、成功時も response.sessionId = options.sessionId のままです。
  • この実装だと新規 headless 実行後に session が保存されず、report phase や通常の resume が成立しません。
  • 既存コードでは response.sessionIdPieceEngine 側で保存され、その後 phase-runner の resume に使われる前提です。デフォルト provider を claude に変えているので、この欠落は回帰影響が大きいです。
  • 修正方針:
    • 新規 headless 実行時に session を確定できるようにする
    • 返ってきた session ID を AgentResponse.sessionId に載せる
    • --resume だけでなく、新規開始時の session 生成経路も明示する
    • 必要なら --session-id <uuid> を使って TAKT 側で一貫した session 管理を行う
  1. systemPrompt を user prompt へ連結しており、system prompt として渡していない
  • src/infra/claude-headless/client.tsbuildUserPrompt()systemPrompt を本文に連結していますが、CLI には --system-prompt があるので、これは意味が違います。
  • このままだと persona / custom agent の指示が system 階層から user 階層へ降格し、SDK 経路と挙動差が出ます。
  • 修正方針:
    • systemPrompt--system-prompt で渡す
    • user prompt への連結はやめる
    • SDK 側と headless 側で prompt 階層の意味を揃える
  1. mcpServers を headless 側で黙って捨てている
  • ProviderCallOptions には mcpServers がありますが、src/infra/providers/claude-headless.ts では ClaudeHeadlessCallOptions へ渡しておらず、CLI 引数化もされていません。
  • Claude CLI 自体は --mcp-config を持っているので、現状は「未対応」ではなく「サイレント無視」になっています。
  • 修正方針:
    • mcpServers を CLI の --mcp-config に落とし込む
    • すぐ実装しないなら、headless provider では mcpServers 指定時に明示的にエラーにする
    • 少なくとも黙って無視する挙動は避ける

優先度としては 1, 2 がマージ前必須、3 も少なくとも fail-fast 化は必要だと思います。

Copy link
Copy Markdown
Owner Author

nrslib commented Apr 4, 2026

追加で、テスト観点もコメントします。今回かなりテストは増えていますが、ブロッカーになっている論点を守るケースはまだ不足しています。

不足していると見ているのは主に以下です。

  1. headless 初回実行で新しい sessionId を確立して返すテスト
  • 現状の claude-headless-client.test.ts--resume に既存 UUID を渡すケースは見ていますが、「session なしで開始した実行が新しい sessionId を返す」ケースを守れていません。
  • ここがないと、resume/report phase の破綻をテストで検出できません。
  1. 返ってきた sessionId を使って report phase が resume できるテスト
  • phase-runnerresponse.sessionId を前提に report phase を再開する設計なので、headless claude で output contract あり movement を1回流し、同じ session を使って report phase に入れるケースを通したいです。
  • ここは unit でも integration 寄りでもよいですが、少なくとも headless claude 経路で壊れないことを自動で確認したいです。
  1. systemPrompt が本文連結ではなく CLI の --system-prompt になるテスト
  • いまのテストは prompt 本文や引数順は見ていますが、system prompt の渡し方そのものを守れていません。
  • systemPrompt があるときに --system-prompt が argv に入り、末尾 prompt には混ざらないことを明示的にテストした方がよいです。
  1. mcpServers を黙って捨てないことのテスト
  • 正式対応するなら --mcp-config への変換をテストする。
  • まだ未対応でいくなら、mcpServers 指定時に headless provider が明示的にエラーにするテストを入れる。
  • どちらにしても「サイレント無視しない」を固定したいです。

要するに、今あるテストは provider split の外形確認はできていますが、今回のレビューで問題になった headless 固有の契約までは守れていません。上の4ケースを追加すると、この PR のリスクはかなり下がると思います。

@nrslib
Copy link
Copy Markdown
Owner Author

nrslib commented Apr 4, 2026

Summary

Execution Report

Workflow review-fix-takt-default completed successfully.

Copy link
Copy Markdown
Owner Author

nrslib commented Apr 5, 2026

気になった点を2つ共有します。

  1. claude_agent / claude_skill は削除候補に見えます。
    repo 内を見た限り、実利用の痕跡はほぼ schema / runner / provider 分岐 / テストだけで、builtins/ / docs/ / e2e/ に設定例や利用例が見当たりませんでした。
    また実装上も、
  • claude_agent: built-in agent 用 system prompt を差し込んで通常実行に流す薄いラッパー
  • claude_skill: prompt 先頭に /${skillName} を付けて通常実行に流すだけ
    という形で、独立した API / 経路として持つ価値はかなり薄そうです。
    provider split 後は特にノイズになりやすいので、残す理由がなければ別 Issue / 別 PR で削除してよい気がします。
  1. headless claude の sandbox は「技術的に不可能」ではなさそうです。
    ローカルの claude --help / claude -p --help を見ると CLI フラグとして直接 --sandbox ... があるわけではありませんが、公式 docs では headless でも --settings / settings.json 経由で sandbox 設定を扱える前提になっています。
    確認できた設定は少なくとも以下です。
  • sandbox.excludedCommands
  • sandbox.allowUnsandboxedCommands
  • sandbox.filesystem.allowWrite
  • sandbox.filesystem.denyWrite
  • sandbox.filesystem.denyRead
  • sandbox.filesystem.allowRead

参考:

なので、この PR で headless claude に対して sandbox を全面的に禁止する判断は、少なくとも「CLI 的に不可能だから」という理由では弱そうです。
方向性としては、

  • 本当に未対応にするなら理由を明示する
  • そうでなければ --settings 経由で headless 側にも配線する
    のどちらかに寄せた方がよさそうです。

@nrslib
Copy link
Copy Markdown
Owner Author

nrslib commented Apr 5, 2026

追加で気になった点は2つです。

  1. sessionId 契約が他 provider と揃っていません
    現在の claude headless だけ resume 時に sessionId を UUID として検証していますが、TAKT の provider 共通契約は sessionId?: string です。Codex / OpenCode / Cursor / Copilot はいずれも opaque な文字列として扱っており、claude headless だけ実装内で追加制約を持つのは不自然です。
    新規開始時に --session-id 用の UUID を生成するのは問題ありませんが、resume に渡す既存 sessionId まで UUID 必須にする必要は薄いので、ここは他 provider と同様に単なる string として扱うほうがよいと思います。

  2. claude headless を Codex / OpenCode と同レベルまで持っていきたいです
    今回の修正で session / systemPrompt / MCP / permission / sandbox はかなり揃いましたが、まだ Codex / OpenCode に対して provider 能力の差があります。特に supportsStructuredOutput = false のままで、outputSchema を provider から下流へ渡せていません。
    claude を TAKT の第一級 provider として扱うなら、少なくとも以下は目標にしたいです。

    • sessionId を他 provider と同じ string 契約に揃える
    • outputSchema を headless 経路に実装して structured output を使えるようにする
    • provider と実装配置も、Codex / OpenCode と同様に見通しの良い構成へ寄せる

@nrslib
Copy link
Copy Markdown
Owner Author

nrslib commented Apr 5, 2026

追加で気になる点です。

  1. sessionId は他 provider と同じく opaque な string 契約に揃えてください。claude headless だけ resume 時に UUID を強制するのは provider 共通契約から外れています。

  2. claudeAgent / claudeSkill のような Claude 固有の特別扱いは不要です。Codex / OpenCode は普通に systemPrompt + prompt + options で呼べているので、Claude だけ provider interface や runner に専用分岐を持つ必要はありません。

不要なものは絶対に残さないでください。今回の整理で置き換え先ができたなら、claudeAgent / claudeSkill の定義・分岐・補助実装・関連テスト・未使用テンプレートまで含めて削除し切るべきです。中途半端に alias や dead code を残すと、Claude 系だけ構造が汚れたままになります。

@nrslib
Copy link
Copy Markdown
Owner Author

nrslib commented Apr 5, 2026

Summary

Execution Report

Workflow takt-default completed successfully.

@nrslib nrslib marked this pull request as ready for review April 5, 2026 08:57
@nrslib
Copy link
Copy Markdown
Owner Author

nrslib commented Apr 5, 2026

2点気になりました。

  1. headless の stream-json で、最終本文の抽出元は result.result に寄せた方がよさそうです。今の実装は assistant/message.content と最終 result.result の両方を集約対象に見えるので、成功系 stream で両方に同じ本文が出る場合は二重連結になる余地があります。message.content はリアルタイム表示用、最終レスポンスは result.result 用に分離したいです。

  2. 成功/失敗判定も本文の有無ではなく最終イベントで決めるべきだと思います。今は本文を抽出できれば done になりますが、headless の stream-json では result.is_error のような状態フィールドを見る方が自然です。本文が返っていても、最終イベントが error なら TAKT 側も error にしたいです。

@nrslib
Copy link
Copy Markdown
Owner Author

nrslib commented Apr 5, 2026

Summary

Execution Report

Workflow takt-default completed successfully.

@nrslib nrslib merged commit 569ecda into main Apr 5, 2026
3 of 4 checks passed
@nrslib nrslib deleted the takt/584/split-claude-providers branch April 5, 2026 23:43
@nrslib nrslib mentioned this pull request Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

タスク指示書: Claude プロバイダの分割(claude-sdk / ヘッドレス claude

1 participant