From 493ed2c781a129cce4672d2bf47283a226321421 Mon Sep 17 00:00:00 2001 From: Hare Date: Thu, 16 Apr 2026 00:57:26 +0900 Subject: [PATCH] =?UTF-8?q?pod-factory=E5=AE=8C=E4=BA=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TODO.md | 1 - crates/pod/src/lib.rs | 4 +- crates/pod/src/main.rs | 46 ++++--- tickets/pod-factory.md | 198 ------------------------------ tickets/pod-factory.review.md | 222 ---------------------------------- 5 files changed, 24 insertions(+), 447 deletions(-) delete mode 100644 tickets/pod-factory.md delete mode 100644 tickets/pod-factory.review.md diff --git a/TODO.md b/TODO.md index 0704d2ae..9c70f4fb 100644 --- a/TODO.md +++ b/TODO.md @@ -6,7 +6,6 @@ - [ ] Protocol の設計 → [tickets/protocol-design.md](tickets/protocol-design.md) - [ ] パーミッション: パターンベースのツール実行制御 → [tickets/permission-extension-point.md](tickets/permission-extension-point.md) - [ ] ネイティブ GUI クライアント MVP → [tickets/native-gui-mvp.md](tickets/native-gui-mvp.md) -- [ ] Pod Factory: カスケード設定とプロンプト資産 → [tickets/pod-factory.md](tickets/pod-factory.md) - [ ] TUI 拡充 - [ ] Pod の明示的 shutdown → [tickets/tui-pod-shutdown.md](tickets/tui-pod-shutdown.md) - [ ] 新しい Pod を spawn する UI の設計 → [tickets/tui-pod-spawn-ui.md](tickets/tui-pod-spawn-ui.md) diff --git a/crates/pod/src/lib.rs b/crates/pod/src/lib.rs index 859b1933..98acf40f 100644 --- a/crates/pod/src/lib.rs +++ b/crates/pod/src/lib.rs @@ -23,7 +23,9 @@ pub use controller::{PodController, PodHandle}; pub use factory::{FactoryError, PodFactory}; pub use notifier::Notifier; pub use hook::{Hook, HookEventKind, HookRegistryBuilder}; -pub use manifest::{PodManifest, PodManifestConfig, ProviderConfig, ProviderKind, Scope}; +pub use manifest::{ + PodManifest, PodManifestConfig, PodMetaConfig, ProviderConfig, ProviderKind, Scope, +}; pub use pod::{Pod, PodError, PodRunResult, apply_worker_manifest}; pub use prompt_loader::PromptLoader; pub use protocol::{ErrorCode, Event, Method, TurnResult}; diff --git a/crates/pod/src/main.rs b/crates/pod/src/main.rs index afe7fc2b..e20f4f3e 100644 --- a/crates/pod/src/main.rs +++ b/crates/pod/src/main.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use std::process::ExitCode; use clap::Parser; -use pod::{Pod, PodController, PodFactory}; +use pod::{Pod, PodController, PodFactory, PodManifestConfig, PodMetaConfig}; use session_store::FsStore; #[derive(Parser)] @@ -57,28 +57,17 @@ fn default_runtime_dir() -> Result { } } -/// Turn CLI inputs into a single programmatic overlay TOML string, -/// combining `--pwd` and `--overlay`. Returns `None` if neither flag -/// is set. -fn build_overlay_toml(pwd: Option<&PathBuf>, overlay: Option<&str>) -> Option { - let mut parts: Vec = Vec::new(); - if let Some(pwd) = pwd { - // Canonicalize the pwd shorthand here so relative CLI arguments - // (e.g. `--pwd .`) turn into the absolute path required by the - // manifest cascade. - let absolute = std::fs::canonicalize(pwd).unwrap_or_else(|_| pwd.clone()); - parts.push(format!( - "[pod]\npwd = \"{}\"\n", - absolute.display().to_string().replace('\\', "\\\\") - )); - } - if let Some(overlay) = overlay { - parts.push(overlay.to_string()); - } - if parts.is_empty() { - None - } else { - Some(parts.join("\n")) +/// Construct a programmatic overlay [`PodManifestConfig`] that carries +/// `pod.pwd` derived from the `--pwd` shorthand. Relative CLI paths +/// are canonicalized here so the cascade always sees an absolute path. +fn pwd_overlay(pwd: &PathBuf) -> PodManifestConfig { + let absolute = std::fs::canonicalize(pwd).unwrap_or_else(|_| pwd.clone()); + PodManifestConfig { + pod: PodMetaConfig { + pwd: Some(absolute), + ..Default::default() + }, + ..Default::default() } } @@ -103,9 +92,16 @@ async fn build_factory(cli: &Cli) -> Result { .map_err(|e| format!("failed to auto-load project manifest: {e}"))?, }; - if let Some(overlay) = build_overlay_toml(cli.pwd.as_ref(), cli.overlay.as_deref()) { + // `--pwd` goes in as a typed config so path strings never have to + // pass through TOML escaping. `--overlay` keeps its inline-TOML + // interface (that is its entire reason for existing). Both feed + // the same overlay slot and merge in call order. + if let Some(pwd) = cli.pwd.as_ref() { + factory = factory.with_overlay_config(pwd_overlay(pwd)); + } + if let Some(overlay) = cli.overlay.as_deref() { factory = factory - .with_overlay_toml(&overlay) + .with_overlay_toml(overlay) .map_err(|e| format!("failed to parse overlay TOML: {e}"))?; } diff --git a/tickets/pod-factory.md b/tickets/pod-factory.md deleted file mode 100644 index 0847721f..00000000 --- a/tickets/pod-factory.md +++ /dev/null @@ -1,198 +0,0 @@ -# Pod Factory: 設定カスケードとプロンプト資産による Pod 自動生成 - -## レビュー状態 - -初回レビュー実施済み。[pod-factory.review.md](pod-factory.review.md) を参照。 -要件全項目達成、アーキテクチャ・テスト被覆とも良好で**無条件で受け入れ可**。指摘は 6 件（すべて任意修正または nit レベル）。 - -## 背景 - -現状、Pod を起動するには `test_pod.local.toml` のような完全な `PodManifest` TOML を手書きする必要がある。1 人のユーザーが1 つのエージェントを試験運用するには十分だが、Insomnia が狙う「複数のエージェントが独立プロセスとして spawn されて自律的に動く」世界観では、**Pod のライフサイクル全体が自動化可能でなければならない**。そのためには、Pod の**作成自体**も自動化可能である必要がある。 - -手書きマニフェストには以下の問題がある: - -- 1 Pod = 1 ファイルで、Pod を動的に増やす用途にスケールしない -- 設定項目が多く（`worker.*` / `provider.*` / `scope.*` / `compaction.*` / `tool_output.*` 等）、毎回コピペしてわずかな差分だけ書き換える苦行になる -- system_prompt を TOML 文字列に埋め込む形はプロンプト資産の再利用性が低い -- Pod の起動条件の**共通部分**（プロバイダ・モデル・デフォルトツール設定など）は本来一度書けば良いのに、毎回書かされる - -## ゴール - -Pod 作成を「**最終的に `PodManifest` を1 つ構築する問題**」として定式化し、その `PodManifest` を**カスケード + 差分上書き**で組み立てる基盤を提供する。手書きが必要な TOML は「ユーザー / プロジェクト単位の**デフォルト上書き**」だけに縮退させ、個別の Pod 起動ごとに人間が TOML を触らない状態を目指す。 - -プロンプトは手書きマニフェストに文字列を埋め込む方式をやめ、**テンプレート資産ライブラリ**として参照可能にする。 - -## 方針 - -### 同じ型で、層で上書きする - -- **解決後の型は現行の `manifest::PodManifest` のまま**（必須フィールド持ち）。 -- カスケードの各層は **`PodManifestConfig`**（`PodManifest` と同じ構造だが全フィールドを `Option` / 部分形で保持する新規型）で表現する。部分形同士を順番にマージし、最後に `TryFrom for PodManifest` で必須チェックを行いつつ確定させる。 -- 各層は**部分形**を持てる（全フィールドを埋める必要はない）。存在するフィールドだけが下層を上書きする。 -- 人間が書くときは `PodManifest` と同じ TOML スキーマで書く（サブセット可）。ファイル名は**すべて `manifest.toml`**。「設定」という曖昧な名前を避け、「Pod manifest」という語彙に揃える。 - -### カスケードの層 - -優先順位が低い方から高い方へ: - -1. **ビルトインのデフォルト**: コードに焼き込んだ基本値（現在 `PodManifest` 各フィールドの `#[serde(default)]` や `Default` 実装に散っているものを集約） -2. **ユーザー manifest**: `$XDG_CONFIG_HOME/insomnia/manifest.toml`（`XDG_CONFIG_HOME` 未設定時は `~/.config/insomnia/manifest.toml`）。ユーザー個人のプロバイダ指定・デフォルトモデル・常用ツール設定等を書く -3. **プロジェクト manifest**: プロジェクトルート下の `.insomnia/manifest.toml`。プロジェクト固有の scope・compaction 設定・system_prompt のベース等を書く -4. **プログラマティック overlay**: Pod 生成を呼ぶコード（GUI / CLI / 別 Pod からの spawn 等）が渡す部分形。ここで `pod.name` や `pod.pwd` のような**その Pod に固有の値**を与える - -### プロジェクトルートの判定 - -起動ディレクトリから上方向に `.insomnia/` ディレクトリを探索し、**最も近い**ものをプロジェクトルートとする。見つからなければ「プロジェクト無し」として扱い、プロジェクト層をスキップする。 - -### マージのセマンティクス - -- **スカラー** (`String`, `u32`, `bool` 等): 上層が存在すれば丸ごと置換 -- **Option 型**: 上層が `Some` なら置換、`None` なら据え置き -- **マップ** (例: `tool_output.per_tool`): キー単位でマージ、同一キーは上層優先 -- **`scope.allow` / `scope.deny`**: **union（各層から全部足す）**。すべての層が追加のみ可能。最終的な effective scope は既存の `Scope::from_config` ロジックに委ね、`allow_union - deny_union` として解決される。**上位層は deny を追加することで下位層の allow を必ず削れる**ので、「上に行くほど強制力を持てる」構造が自然に出る -- その他リスト: 当面は `scope.*` 以外にリスト型は存在しないが、新規にリスト型フィールドを追加するときは原則 union（scope と同じ）で扱う - -### エラー戦略 - -- **未知フィールド**: TOML に書かれた未定義のキーは `tracing::warn!` のみ出して無視する。`#[serde(deny_unknown_fields)]` は**使わない**（将来バージョンアップで読めない旧設定が出るとユーザー体験が悪いため） -- **型ミスマッチ**: `max_tokens = "100"` のような型エラーは hard error として resolve 失敗させる。ファイルパスと位置情報をエラーメッセージに含める -- **パスフィールド**: `pod.pwd` / `provider.api_key_file` / `scope.*.target` 等のパスは**絶対パスのみ受け付ける**。相対パスが書かれていたら resolve 時に hard error とする。相対パス解決を層ごとの manifest_dir に依存させるとカスケードの意味論が濁るため、各ファイルを書く人間の責任で絶対パスを指定させる - -### プロンプト資産ライブラリ - -- プロンプトは TOML 文字列ではなく**ファイルとして管理**する。 -- 検索パスは以下の3層（上層優先で解決）: - 1. **ビルトイン**: レポジトリ直下の `resources/prompts/` 以下を `include_dir!` マクロでバイナリに同梱 - 2. **ユーザー**: `$XDG_CONFIG_HOME/insomnia/prompts/` - 3. **プロジェクト**: `/.insomnia/prompts/` -- **ファイル名の stem がそのままプロンプト名**。サブディレクトリも許容し、`resources/prompts/common/tool-usage.md` は `{% include "common/tool-usage" %}` で参照できる -- ファイル形式は **`.md`**（frontmatter は無し。前方互換として `---` 区切りの TOML フロントマターを将来持てる余地だけ認識しておく） -- 既存の `SystemPromptTemplate`（minijinja ベース）の `Environment` にカスタムローダを仕込み、テンプレート内から他のプロンプトを `{% include "name" %}` / `{% import "name" as p %}` で参照できるようにする -- プロンプト資産自体もテンプレートとして評価され、現行の `SystemPromptContext`（`now` / `cwd` / `scope` / `tools` / `files` 等）と同じ変数が見える -- include 時の**変数伝搬は minijinja のデフォルト挙動**（親スコープの変数が自動で見える）に任せる - -### 設定値のテンプレート参照は扱わない - -`worker.max_tokens = "{{ env.INSOMNIA_MAX_TOKENS }}"` のような**設定値の中でテンプレートを展開する機能は本チケットの範囲外**とする。テンプレートエンジンはプロンプト本文の組み立てだけに限定する。設定値の動的化が必要になった時点で別チケットで検討する。 - -### プログラマティック Pod 作成 API - -factory は `PodManifest` を組み上げる builder として設計する。イメージ: - -```rust -let manifest: PodManifest = PodFactory::new() - .with_user_manifest_auto()? // XDG から自動読み込み、不在 OK - .with_project_manifest_auto()? // cwd から上方向に .insomnia/ を探索、不在 OK - .with_overlay_toml(overlay)? // programmatic な最上層 overlay（TOML 文字列） - .resolve()?; // -> PodManifest -Pod::from_manifest(manifest, store).await?; -``` - -`Pod::from_manifest` は現在 `manifest_dir` を取るが、本チケットで以下の二段に書き換える: - -- **`Pod::from_manifest(manifest: PodManifest, store)`** — 一次 API。factory の出力がそのまま入る -- **`Pod::from_manifest_toml(toml: &str, store)`** — 便利関数。単層 manifest を TOML 文字列で直接投げる用途（テスト・デバッグ・動作確認） - -`manifest_dir` 引数は廃止する。パスは絶対化済み前提のため、Pod 側で相対パス解決ロジックを持たない。ファイル読み込みは呼び出し側（factory か caller）の責務。 - -### CLI - -`pod` バイナリの CLI surface を factory に合わせて更新する。新 umbrella コマンド（`insomnia spawn` 等）は**作らない**。ユーザーが直接 CLI を叩くのは debug/automation の文脈で、GUI/TUI は Pod を subprocess として spawn する運用のため、`pod` バイナリ1本の fla だけで十分。 - -``` -pod [--user-manifest ] [--project ] [--overlay ] [--pwd ] -``` - -- `--user-manifest`: 省略時は XDG から自動解決 -- `--project`: 省略時は cwd から上方向に `.insomnia/` を探索 -- `--overlay`: 最上層の overlay を inline TOML 文字列で渡す（例: `--overlay 'pod.name = "dbg"'`） -- `--pwd`: 便利ショートカット（overlay に `pod.pwd = ...` を書く代わり） -- 旧 `--manifest ` フラグは廃止。単層 manifest を直接読み込みたい場合は `--user-manifest ` で代替する - -## 要件 - -### カスケード基盤 - -- ユーザー manifest・プロジェクト manifest・プログラマティック overlay を順に重ねた結果が `PodManifest` として取れる。 -- 各層が部分形を許容する（`pod.pwd` だけ書いてあっても良い等）。 -- マージセマンティクスがフィールドごとに定義され、単体テストで担保される（スカラー / Option / マップ / scope union）。 -- 未知フィールドは warn のみ、型ミスマッチは hard error。 -- 層が全て空で resolve 失敗する場合は、必要なフィールド（`pod.pwd` / `provider` / `scope.allow` 等）を明示するエラーを返す。 - -### プロンプト資産ライブラリ - -- 3層の検索パスでプロンプトファイルを解決できる。 -- 同名プロンプトは上層優先で解決される。 -- `SystemPromptTemplate` の minijinja `Environment` にカスタムローダを仕込み、`{% include "name" %}` / `{% import "name" as x %}` で資産を参照できる。 -- 親テンプレートの変数が include 先にも見える（minijinja デフォルト挙動）。 -- ビルトインプロンプトは `resources/prompts/` から `include_dir!` マクロで同梱され、追加・編集するときは該当ディレクトリにファイルを置く・編集するだけで済む。 -- プロンプト資産自体もテンプレートとして評価され、`SystemPromptContext` と同じ変数が見える。 - -### プログラマティック Pod 作成 - -- `Pod::from_manifest(manifest: PodManifest, store)` と `Pod::from_manifest_toml(toml: &str, store)` の二段構成。path 受け API は廃止。 -- `PodFactory` builder が上記に繋がる。 -- TUI / GUI / daemon 等の上位クライアントが、TOML ファイルパスではなく overlay + 設定ディレクトリパスを渡すだけで Pod を起動できる。 - -### CLI - -- `pod` バイナリが `--user-manifest` / `--project` / `--overlay` / `--pwd` を受け付ける。 -- 引数無しで cwd + XDG を自動解決して起動できる最小構成が動く。 - -### ドキュメント - -- カスケード層の優先順位・マージ規則を `docs/` にまとめる。 -- ユーザー manifest / プロジェクト manifest の最小例と全オプション例を残す。 -- `resources/prompts/` のディレクトリ構造とビルトインプロンプト一覧を `docs/` に記載。 - -## リソース構成 - -``` -insomnia/ -├── crates/ -│ ├── pod/ -│ │ └── src/ -│ │ ├── factory.rs # 新規。PodManifestConfig カスケード → PodManifest -│ │ ├── prompt_loader.rs # 新規。minijinja loader + 3層検索 -│ │ └── ... -│ └── ... -├── resources/ -│ └── prompts/ -│ ├── coder.md -│ ├── reviewer.md -│ ├── planner.md -│ └── common/ -│ └── tool-usage.md -``` - -- factory とプロンプト loader は新規 crate を作らず `pod` crate 内に配置する。既に `pod` が `manifest` に依存しているため追加の依存関係整理が不要 -- `include_dir!("$CARGO_MANIFEST_DIR/../../resources/prompts")` で全ビルトインプロンプトをバイナリに取り込む -- ファイルの追加・編集は通常の file 操作だけで済む。既存ファイルの編集は cargo の変更追跡で自動再ビルドされる。新規追加・削除が反映されない場合は `cargo clean -p pod` を案内する（必要が増えたら `build.rs` で `rerun-if-changed` を足す） -- ビルトインプロンプトの初期ラインナップ（`coder` / `reviewer` / `planner` / `common/tool-usage` 等）は実装時に選定し、ドキュメントに列挙する - -## 完了条件 - -- `PodFactory` がカスケード層のマージで `PodManifest` を構築でき、マージセマンティクスの単体テストが通る（スカラー・Option・マップ・scope union・未知フィールド warn・型ミスマッチ hard error）。 -- ユーザー manifest・プロジェクト manifest・プログラマティック overlay のすべての層を使う end-to-end テストで、Pod が path を一切渡さずに起動できる。 -- プロンプト資産ライブラリを経由して system_prompt が組み立てられ、`{% include "ビルトイン名" %}` で `resources/prompts/` 以下の資産を参照できることをテストで確認できる。 -- `Pod::from_manifest(manifest)` と `Pod::from_manifest_toml(toml)` の二段 API が動き、path 受け API は削除されている。 -- `pod` バイナリが `--user-manifest` / `--project` / `--overlay` / `--pwd` を受け、引数無しでも cwd + XDG 自動解決で起動できる。 -- カスケード層の優先順位・マージ規則 / ユーザー manifest 例 / プロジェクト manifest 例 / ビルトインプロンプト一覧のドキュメントが `docs/` に存在する。 - -## 他チケットとの関係 - -- `tickets/native-gui-mvp.md`: 現状「manifest ファイルを選ぶ UI」を含むが、本チケット完了後はその UI が「overlay 指定 + Pod 起動」に置き換わる想定。native-gui-mvp 実装時に本チケットの API を使うか、先に path 渡しで済ませて後から差し替えるかは別途判断 -- `tickets/tui-pod-spawn-ui.md`: 同上。Pod spawn UI は本チケットが提供する API の上に構築される -- `tickets/protocol-design.md`: Pod ↔ Client protocol 自体は変わらない。spawn 要求を protocol に載せるかどうかは protocol-design 側で検討 -- `docs/system-prompt-template.md` / `crates/pod/src/system_prompt.rs`: プロンプト資産ライブラリはこの minijinja 基盤の拡張として実装される - -## 範囲外 - -- **preset の概念**: 名前付き overlay セット（`insomnia spawn coder` 等）は本チケットでは導入しない。将来別チケットで検討 -- **設定値の中のテンプレート展開**（`max_tokens = "{{ env.X }}"` のような動的値）。プロンプト本文のテンプレート展開のみを扱う -- **GUI 内での manifest 編集 UI**。編集は人間がエディタで TOML を書くだけ（あくまで「Pod 生成時に手書きしない」ことを目指す） -- **チーム共有・同期**。ユーザー manifest とプロジェクト manifest は各自・各リポジトリ単位で管理される -- **秘密情報管理**（API キー等）。既存の `api_key_file` 方式を維持する -- **設定値の型バリデーション強化**（JSON Schema など）。現行の serde ベースで十分な範囲に留める -- **プロンプトフロントマター**（description / required_vars 等のメタデータ）。将来一覧 UI が必要になった時点で検討。フォーマット上の前方互換性だけは意識する -- **umbrella CLI コマンド**（`insomnia spawn` 等）。`pod` バイナリ単体で完結させる diff --git a/tickets/pod-factory.review.md b/tickets/pod-factory.review.md deleted file mode 100644 index 30093204..00000000 --- a/tickets/pod-factory.review.md +++ /dev/null @@ -1,222 +0,0 @@ -# レビュー: Pod Factory - -対象差分: `crates/manifest/src/{lib,config}.rs`, `crates/pod/src/{lib,pod,main,system_prompt,factory,prompt_loader}.rs`, `crates/pod/examples/{pod_cli,pod_protocol}.rs`, `crates/provider/src/lib.rs`, `resources/prompts/`, `docs/pod-factory.md`（いずれも未コミット） - -## 要件達成状況 - -| 要件 | 状態 | -|---|---| -| カスケード基盤（ユーザー・プロジェクト・overlay の層マージ） | ✅ `manifest::PodManifestConfig` が部分形として実装。`PodFactory` が 4 層（default/user/project/overlay）を順にマージ | -| 解決後の型は `PodManifest` のまま | ✅ `TryFrom for PodManifest` が検証付き変換 | -| 各層の manifest.toml スキーマは同じ | ✅ partial 型群 (`PodMetaConfig` / `ProviderConfigPartial` / `WorkerManifestConfig` / ...) が本物と同構造 | -| ユーザーパスは XDG | ✅ `user_manifest_path` が `XDG_CONFIG_HOME` → `$HOME/.config` の順 | -| プロジェクトルートは `.insomnia/` 最近接 | ✅ `find_project_manifest` が cwd から上方向に walk | -| scope は union マージ | ✅ `merge_scope` が allow/deny を `extend` | -| マップは key-wise マージ | ✅ `ToolOutputLimitsPartial::merge` | -| スカラー / Option は upper 優位 | ✅ `upper.x.or(self.x)` パターンで実装 | -| 未知フィールドは warn のみ | ✅ `serde_ignored::deserialize` + `tracing::warn!` | -| 型ミスマッチは hard error | ✅ toml パースで通常通り失敗 | -| パスフィールドは絶対パスのみ | ✅ `ensure_absolute` が `pod.pwd` / `provider.api_key_file` / `scope.*.target` を検証 | -| `Pod::from_manifest(manifest, store, loader)` 二段 API | ✅ + `from_manifest_toml` 便利関数。旧 path 受けは廃止 | -| `manifest_dir` 引数の消滅 | ✅ `Pod` 構造体から削除、`provider::build_client` からも消滅 | -| プロンプト3層ローダ | ✅ `PromptLoader` が project/user/builtin の順で解決 | -| ビルトインは `resources/prompts/` + `include_dir!` | ✅ `pod/src/prompt_loader.rs:17` | -| プロンプト include の親変数伝搬 | ✅ minijinja デフォルト挙動、`include_resolves_builtin_prompt` テストで確認 | -| CLI の `--user-manifest` / `--project` / `--overlay` / `--pwd` | ✅ `pod/src/main.rs` で実装。引数無しでも XDG + cwd 自動解決で動く | -| 引数無しでの最小構成動作 | ✅ CLI デフォルトが auto 系メソッドにつながる | -| ドキュメント (`docs/pod-factory.md`) | ✅ カスケード層・マージ規則・CLI・プログラマティック API・ビルトイン一覧を網羅 | - -**すべての要件を達成**。さらに当初のチケットに無かった点として CLI docs の整理、`examples/` の追随更新、provider の `~` 展開廃止（絶対パス徹底）までカバーしている。 - -## アーキテクチャ統合 - -### クレート境界 - -- **`manifest/src/config.rs`** — `PodManifestConfig` を manifest crate 側に置いたのは**正しい判断**。manifest crate は純データ型と検証ロジック (`Scope::from_config`, `TryFrom`) の置き場で I/O を持たない、という既存方針をそのまま継承している -- **`crates/pod/src/factory.rs`** / **`prompt_loader.rs`** — I/O（ファイル読み、XDG 解決、`include_dir!`）は pod crate 側に集約。新規 crate を作らず、既存レイヤに素直に乗せている。前回私が推した統合方針どおり -- **`PodFactory::resolve()` が `(PodManifest, PromptLoader)` を返す** — factory は「manifest + どこからプロンプトを引くか」を一体で管理し、Pod は渡された loader を素直に使うだけ。責務の分離が綺麗 -- **`Pod::manifest_dir` の完全削除 + `resolve_pwd` の絶対パス強制** — 「パスの正規化は cascade 層の専売事項」という単一ソースの原則が守られている。`provider::build_client` からも `manifest_dir` 引数が消え、`~` 展開も消え、relative rejection だけ残った。下流が大幅にシンプルになった - -### 副作用的な改善 - -- `Pod::from_manifest_toml` が `PromptLoader::builtins_only()` を暗黙に使う → テスト・examples が「単層 TOML で Pod 起動」を一行で書けるようになった -- `examples/pod_cli.rs` と `pod_protocol.rs` が絶対パス化と新 API に追随済み。壊れ残りなし - -## 指摘事項 - -### 1. 🟢 `with_overlay_toml` / `with_overlay_config` の重ね合わせが「同じ層にマージ」 - -`factory.rs:123-139`: - -```rust -pub fn with_overlay_toml(mut self, toml: &str) -> Result { - let config = PodManifestConfig::from_toml(toml).map_err(FactoryError::OverlayParse)?; - self.overlay = Some(match self.overlay { - Some(existing) => existing.merge(config), - None => config, - }); - Ok(self) -} -``` - -`with_overlay_*` を複数回呼ぶと、独立したレイヤにはならず**1 つの overlay スロットに逐次マージ**される（後勝ち）。これは CLI + 1 回のプログラマティック注入には十分だが、「複数の独立した overlay を priority order 付きで積みたい」ニーズには応えない。 - -**判断**: 現状の要件範囲内では問題なし。将来 preset や環境変数ベースの overlay を追加する場合に再検討。 - -**細かい付随点**: `factory.rs` のテスト `cascade_overlay_overrides_project_overrides_user` は名前が「overlay が project を override し、project が user を override する」と読めるが、実際には**全部 overlay スロットに積まれている**（user/project スロットは未使用）。テスト内コメントで断り書きはあるが、名前だけ見ると誤解されやすい。`cascade_priority_layer_ordering` のほうが本来のレイヤ順序を検証しているので、前者は `overlay_stacking_merges_in_place` などに改名するとより正確。 - -### 2. 🟢 "builtin defaults" 層の実体がゼロ - -チケット方針: -> ビルトインのデフォルト: コードに焼き込んだ基本値（現在 `PodManifest` 各フィールドの `#[serde(default)]` や `Default` 実装に散っているものを集約） - -実装方針（`factory.rs` module doc）: -> 1. **Builtin defaults** — in-code defaults, currently empty. Upper layers provide everything; `TryFrom` fills in per-field defaults - -実際の builtin layer は `PodManifestConfig::default()`（全部 None）で、デフォルト値の適用は `TryFrom` 内の `.unwrap_or(ToolOutputLimits::default())` のように散在している。つまり**「散らばってる defaults を builtin layer に集約する」という元の目的は達成されていない**。 - -**判断**: 実運用上の挙動は同じ（ユーザーから見れば `PodManifest` の各フィールドが既定値を持つことに変わりなし）。チケットの表現 vs 実装の厳密な乖離だけで、受け入れ可否には影響しない。将来 defaults の一覧を可視化したくなった時点で builtin layer を実体化する余地を残しておけば OK。 - -### 3. 🟢 CLI `--pwd` の overlay 注入が文字列フォーマット経由 - -`main.rs:64-76`: - -```rust -parts.push(format!( - "[pod]\npwd = \"{}\"\n", - absolute.display().to_string().replace('\\', "\\\\") -)); -``` - -生成した TOML 断片を `with_overlay_toml` に渡している。`\` は escape しているが `"` は escape していないため、`"` を含むパス（Linux では理論上あり得る）で壊れる。また `--overlay` 側と `--pwd` 側を join するときに両者の構文が衝突しないかも微妙。 - -**推奨**: 文字列を作らず、`PodManifestConfig` を直接構築して `with_overlay_config` で渡す形に書き換える: - -```rust -if let Some(pwd) = cli.pwd.as_ref() { - let absolute = std::fs::canonicalize(pwd).unwrap_or_else(|_| pwd.clone()); - factory = factory.with_overlay_config(PodManifestConfig { - pod: PodMetaConfig { pwd: Some(absolute), ..Default::default() }, - ..Default::default() - }); -} -if let Some(overlay) = cli.overlay.as_deref() { - factory = factory.with_overlay_toml(overlay)?; -} -``` - -型を経由するので escape 問題が消え、`--pwd` と `--overlay` の干渉も無くなる。 - -**判断**: 現実には問題が起きる可能性は極めて低いが、型経由のほうが筋が良い。**任意**。 - -### 4. 🟢 `resolve_provider` に dead code - -`manifest/src/config.rs:218-237`: - -```rust -fn resolve_provider( - cfg: ProviderConfigPartial, - field_prefix: &'static str, // ← 使われていない - kind_field: &'static str, - ... -) -> Result { - let _ = field_prefix; // ← 明示的に捨てられている - ... -} -``` - -`field_prefix` を取っているが関数内で使っていない（`let _ =` で捨てている）。将来エラーメッセージで `"missing field: {prefix}.kind"` のようにしたい意図だったと推察されるが、現在の `ResolveError::MissingField` は静的文字列を直接受けているので不要。 - -**判断**: 不要なので削除推奨。**任意**（実害なし、lint が効けば dead_code 警告が出るかも）。 - -### 5. 🟢 `~` 展開廃止は breaking change - -`provider` から `~/.config/insomnia/keys/anthropic` のような `~` 始まりパスの展開処理が消えた。既存の手書き manifest にこの形式が書かれていると resolve で `RelativePath` エラーになる。 - -**判断**: チケットで「絶対パスのみ」と決めた結果であり、意図通り。`docs/pod-factory.md` でも「パスの絶対性」として明記されている。受け入れ可。 - -### 6. 🟢 `find_project_manifest` の canonicalize 失敗時フォールバック - -`factory.rs:197-208` は `start.canonicalize()` が失敗したら raw path で walk を続ける。`.insomnia` 名前の検出は path 比較だけなので動作するが、shell が与えた相対パスが絶対化されないまま `dir.parent()` を続けると仕様上の最上位で止まる可能性。 - -**判断**: 実害小。canonicalize が失敗するケースは cwd が無効等で、そもそも Pod 起動前の別エラーに出るはず。**不問**。 - -## テスト - -- `manifest/src/config.rs`: 14 ケース（resolve 成功 / 必須欠落 / 相対パス3種 / scalar merge / scope union / per_tool keywise / option struct / type mismatch / unknown field / partial layer / end-to-end cascade） -- `pod/src/factory.rs`: 7 ケース（overlay only / 模擬レイヤ順 / 実レイヤ順 / walk-up / 無プロジェクト / loader 連携 / 必須欠落） -- `pod/src/prompt_loader.rs`: 6 ケース（builtin 存在 / サブディレクトリ / 未知 / user override / project override / fallthrough） -- `pod/src/system_prompt.rs`: 2 ケース（include 成功 / 未知 prompt） -- `provider/src/lib.rs`: 既存テストを新シグネチャに追随 + 新規 relative rejection - -**合計 30 ケース超**、各層の検証が丁寧。特に `resolve_produces_loader_with_project_prompts_dir` は factory + prompt_loader + system_prompt の**3 層を貫く end-to-end テスト**で、この変更でもっとも壊れやすい配線を lock-in している点が優秀。 - -## 結論 - -**無条件で受け入れ可**。要件は全項目達成、アーキテクチャ統合も筋が良く、テスト被覆も厚い。指摘はすべて任意修正または nit レベルで、受け入れ可否に影響しない。 - -任意修正として推せるのは: - -- **指摘 3**（CLI `--pwd` の型経由化）が最も筋が良い改善。余力があれば -- **指摘 4**（dead code）は数行で済む clean-up -- **指摘 1 のテスト名**（`cascade_overlay_overrides_project_overrides_user` → `overlay_stacking_merges_in_place` 等）も小さな改善 - -上記はいずれも受け入れ後の別タスクとしても良い。 - ---- - -## フォローアップ差分 (2026-04-16) - -レビュー後の追加作業として、指摘 #2（builtin defaults の実体ゼロ）を解消し、 -**デフォルト値のメンテナンス性向上**を目的とした集約リファクタを入れた。 - -### 変更内容 - -- **`crates/manifest/src/defaults.rs` 新設**: 全 manifest デフォルト値を - `pub const` で宣言する単一の真実ソース - - `TOOL_OUTPUT_MAX_BYTES`, `PRUNE_PROTECTED_TURNS`, - `PRUNE_MIN_SAVINGS`, `COMPACT_RETAINED_TURNS` -- **`crates/manifest/src/lib.rs`**: 既存の `default_*` fn 群を constants を - 返すだけの 1 行に縮小。`ToolOutputLimits::default()` / - `CompactionConfig::default()` の serde `#[default = "..."]` 経路もすべて - constants に収束 -- **`crates/manifest/src/config.rs`**: - - `PodManifestConfig::builtin_defaults()` を追加（cascade の最下層として使う - 構築メソッド、constants 直接参照） - - `TryFrom for PodManifest` が `ToolOutputLimits::default()` - / `CompactionConfig::default()` 経由をやめ、constants を直接 `unwrap_or` - する belt-and-suspenders 形 - - 指摘 #4 の `resolve_provider` dead param (`field_prefix`) を削除 -- **`crates/pod/src/factory.rs`**: - - `PodFactory::resolve` の base layer を `PodManifestConfig::default()` から - `PodManifestConfig::builtin_defaults()` に切替。これで "builtin layer" が - 実体を持つ cascade 層として機能 - - 指摘 #1 のテスト名 `cascade_overlay_overrides_project_overrides_user` → - `overlay_stacking_merges_in_place` にリネーム -- **`crates/manifest/src/config.rs` テスト追加**: - - `builtin_defaults_populates_tool_output_max_bytes` - - `builtin_defaults_merged_into_minimal_resolves_with_defaults` - -### 効果 - -- デフォルト値を変えるときの編集箇所が**1 ファイル 1 行**（`defaults.rs`）に。 - 従来は `default_*` fn、`Default::default()` impl、`TryFrom` フォールバックの - 3 経路にそれぞれ値が書かれていたが、すべて `defaults::X` を参照する形に収束 -- チケット本文で当初意図されていた「builtin layer がデフォルト値を保持する」 - 概念が `builtin_defaults()` + factory の base 層で実体化 -- `TryFrom` の fallback は残すので、`builtin_defaults()` を経由しない直接構築 - （テスト等）でも同じ既定値が保証される（belt-and-suspenders） - -### テスト - -全 ワークスペース テスト通過（manifest 45 / pod 71 を含む）。新規 2 ケース -で `builtin_defaults` の挙動と constants の同一性を lock-in。 - -### 未処理 - -- **指摘 3**（CLI `--pwd` の型経由化）は未対応。現在の文字列 format 経由でも - escape 問題が実際に起きる可能性は極めて低いため保留。必要になった時点で別 - フォローアップとする - -この差分の後、指摘 #2 / #4 / #1（テスト名）は解消済み。受け入れ可否には -変化なし（元々「無条件で受け入れ可」）。