From b67023aafceb6347a00b4d8eecf3455b786d9440 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 13 May 2026 02:57:50 +0900 Subject: [PATCH 1/2] feat: organize pod manifest cli flags --- crates/pod/src/main.rs | 207 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 186 insertions(+), 21 deletions(-) diff --git a/crates/pod/src/main.rs b/crates/pod/src/main.rs index 3acb247d..f5e4d5fb 100644 --- a/crates/pod/src/main.rs +++ b/crates/pod/src/main.rs @@ -1,21 +1,24 @@ -use std::path::PathBuf; +use std::ffi::OsString; +use std::path::{Path, PathBuf}; use std::process::ExitCode; use clap::Parser; -use manifest::paths; -use pod::{Pod, PodController, PodFactory}; +use manifest::{PodManifest, paths}; +use pod::{Pod, PodController, PodFactory, PromptLoader}; use session_store::{FsStore, SessionId}; -#[derive(Parser)] +const USER_MANIFEST_ENV: &str = "INSOMNIA_USER_MANIFEST"; + +#[derive(Debug, Parser)] #[command( name = "pod", - about = "Spawn a Pod process from cascaded manifest layers" + about = "Spawn a Pod process from manifest layers or a single manifest file" )] struct Cli { - /// User manifest TOML. Defaults to `/manifest.toml` - /// (see `manifest::paths`). - #[arg(long, value_name = "PATH")] - user_manifest: Option, + /// Manifest TOML to use directly, without loading user, project, or + /// overlay layers. + #[arg(long, value_name = "PATH", conflicts_with_all = ["project", "overlay"])] + manifest: Option, /// Start the project-manifest walk from this directory. When /// omitted, the factory walks up from the current working @@ -53,10 +56,56 @@ struct Cli { session: Option, } -async fn build_factory(cli: &Cli) -> Result { +fn resolve_manifest(cli: &Cli) -> Result<(PodManifest, PromptLoader), String> { + resolve_manifest_with_user_manifest_env(cli, std::env::var_os(USER_MANIFEST_ENV)) +} + +fn resolve_manifest_with_user_manifest_env( + cli: &Cli, + user_manifest_env: Option, +) -> Result<(PodManifest, PromptLoader), String> { + let user_manifest = user_manifest_path_from_env(user_manifest_env); + + if let Some(path) = &cli.manifest { + if user_manifest.is_some() { + return Err(format!( + "--manifest cannot be used when {USER_MANIFEST_ENV} is set" + )); + } + return load_single_manifest(path); + } + + let factory = build_factory_with_user_manifest_path(cli, user_manifest)?; + factory + .resolve() + .map_err(|e| format!("failed to resolve manifest cascade: {e}")) +} + +fn user_manifest_path_from_env(value: Option) -> Option { + value.and_then(|value| { + if value.is_empty() { + None + } else { + Some(PathBuf::from(value)) + } + }) +} + +fn load_single_manifest(path: &Path) -> Result<(PodManifest, PromptLoader), String> { + let toml = std::fs::read_to_string(path) + .map_err(|e| format!("failed to read manifest {}: {e}", path.display()))?; + let manifest = PodManifest::from_toml(&toml) + .map_err(|e| format!("failed to parse manifest {}: {e}", path.display()))?; + Ok((manifest, PromptLoader::builtins_only())) +} + +fn build_factory_with_user_manifest_path( + cli: &Cli, + user_manifest: Option, +) -> Result { let mut factory = PodFactory::new(); - factory = match &cli.user_manifest { + factory = match user_manifest { Some(path) => factory .with_user_manifest(path) .map_err(|e| format!("failed to load user manifest: {e}"))?, @@ -87,18 +136,10 @@ async fn build_factory(cli: &Cli) -> Result { async fn main() -> ExitCode { let cli = Cli::parse(); - let factory = match build_factory(&cli).await { - Ok(f) => f, - Err(e) => { - eprintln!("error: {e}"); - return ExitCode::FAILURE; - } - }; - - let (manifest, loader) = match factory.resolve() { + let (manifest, loader) = match resolve_manifest(&cli) { Ok(pair) => pair, Err(e) => { - eprintln!("error: failed to resolve manifest cascade: {e}"); + eprintln!("error: {e}"); return ExitCode::FAILURE; } }; @@ -202,3 +243,127 @@ async fn main() -> ExitCode { drop(handle); ExitCode::SUCCESS } + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + fn write(path: &Path, contents: &str) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, contents).unwrap(); + } + + fn manifest_toml(name: &str, scope: &Path) -> String { + format!( + r#" +[pod] +name = "{name}" + +[model] +scheme = "anthropic" +model_id = "test-model" + +[worker] + +[[scope.allow]] +target = "{scope}" +permission = "write" +"#, + scope = scope.display() + ) + } + + #[test] + fn user_manifest_flag_is_not_accepted() { + let err = Cli::try_parse_from(["pod", "--user-manifest", "manifest.toml"]).unwrap_err(); + assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); + } + + #[test] + fn manifest_conflicts_with_project_and_overlay() { + let project_err = + Cli::try_parse_from(["pod", "--manifest", "manifest.toml", "--project", "."]) + .unwrap_err(); + assert_eq!(project_err.kind(), clap::error::ErrorKind::ArgumentConflict); + + let overlay_err = Cli::try_parse_from([ + "pod", + "--manifest", + "manifest.toml", + "--overlay", + "pod.name = 'x'", + ]) + .unwrap_err(); + assert_eq!(overlay_err.kind(), clap::error::ErrorKind::ArgumentConflict); + } + + #[test] + fn manifest_conflicts_with_user_manifest_env_when_env_is_non_empty() { + let tmp = TempDir::new().unwrap(); + let manifest = tmp.path().join("manifest.toml"); + write(&manifest, &manifest_toml("single", tmp.path())); + let cli = Cli::try_parse_from(["pod", "--manifest", manifest.to_str().unwrap()]).unwrap(); + + let err = resolve_manifest_with_user_manifest_env(&cli, Some(OsString::from("user.toml"))) + .unwrap_err(); + + assert!(err.contains("--manifest cannot be used")); + assert!(err.contains(USER_MANIFEST_ENV)); + } + + #[test] + fn manifest_allows_empty_user_manifest_env() { + let tmp = TempDir::new().unwrap(); + let manifest = tmp.path().join("manifest.toml"); + write(&manifest, &manifest_toml("single", tmp.path())); + let cli = Cli::try_parse_from(["pod", "--manifest", manifest.to_str().unwrap()]).unwrap(); + + let (manifest, loader) = + resolve_manifest_with_user_manifest_env(&cli, Some(OsString::new())).unwrap(); + + assert_eq!(manifest.pod.name, "single"); + assert!(loader.user_dir().is_none()); + assert!(loader.workspace_dir().is_none()); + } + + #[test] + fn user_manifest_env_overrides_auto_user_manifest_path() { + let tmp = TempDir::new().unwrap(); + let user_manifest = tmp.path().join("custom-user.toml"); + write(&user_manifest, &manifest_toml("from-env", tmp.path())); + let no_project_root = tmp.path().join("no-project"); + std::fs::create_dir_all(&no_project_root).unwrap(); + let cli = + Cli::try_parse_from(["pod", "--project", no_project_root.to_str().unwrap()]).unwrap(); + + let (manifest, _loader) = resolve_manifest_with_user_manifest_env( + &cli, + Some(user_manifest.as_os_str().to_os_string()), + ) + .unwrap(); + + assert_eq!(manifest.pod.name, "from-env"); + } + + #[test] + fn manifest_mode_loads_single_file_with_minimal_prompt_loader() { + let tmp = TempDir::new().unwrap(); + let single_manifest = tmp.path().join("single.toml"); + write(&single_manifest, &manifest_toml("single-file", tmp.path())); + std::fs::create_dir_all(tmp.path().join("prompts")).unwrap(); + std::fs::create_dir_all(tmp.path().join(".insomnia").join("prompts")).unwrap(); + let cli = + Cli::try_parse_from(["pod", "--manifest", single_manifest.to_str().unwrap()]).unwrap(); + + let (manifest, loader) = resolve_manifest_with_user_manifest_env(&cli, None).unwrap(); + + assert_eq!(manifest.pod.name, "single-file"); + assert!(loader.user_dir().is_none()); + assert!(loader.workspace_dir().is_none()); + assert!(loader.user_pack_file().is_none()); + assert!(loader.workspace_pack_file().is_none()); + } +} From e001f4c3f9ca0d152885366c33426122e5c318e2 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 13 May 2026 06:30:45 +0900 Subject: [PATCH 2/2] review: pod-cli-manifest-flags --- tickets/pod-cli-manifest-flags.md | 5 +++++ tickets/pod-cli-manifest-flags.review.md | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tickets/pod-cli-manifest-flags.review.md diff --git a/tickets/pod-cli-manifest-flags.md b/tickets/pod-cli-manifest-flags.md index 7fcc2de2..87018d97 100644 --- a/tickets/pod-cli-manifest-flags.md +++ b/tickets/pod-cli-manifest-flags.md @@ -50,3 +50,8 @@ - `--manifest` を `--project` / `--overlay` と併用しようとすると clap が拒否する - `INSOMNIA_USER_MANIFEST` がセットされた状態で `--manifest` を使おうとするとエラーになる - 既存の `pod --overlay ` 起動（`start_pod.local.fish` 経由）が引き続き動く + +## Review +- 状態: Approve with follow-up +- レビュー詳細: [./pod-cli-manifest-flags.review.md](./pod-cli-manifest-flags.review.md) +- 日付: 2026-05-12 diff --git a/tickets/pod-cli-manifest-flags.review.md b/tickets/pod-cli-manifest-flags.review.md new file mode 100644 index 00000000..3ab11631 --- /dev/null +++ b/tickets/pod-cli-manifest-flags.review.md @@ -0,0 +1,23 @@ +# Review: Pod CLI: マニフェスト関連フラグの整理 + +## 前提・要件の確認 +- `--user-manifest ` の削除: 満たされている。`Cli` から `user_manifest` フィールドは消え、代わりに `manifest` / `project` / `overlay` が定義されている（`crates/pod/src/main.rs:17-32`）。`--user-manifest` が clap 上 unknown argument になるテストも追加されている（`crates/pod/src/main.rs:279-283`）。 +- `INSOMNIA_USER_MANIFEST` による user manifest override: 満たされている。`resolve_manifest` が `std::env::var_os(USER_MANIFEST_ENV)` を読み、非空の場合のみ `PathBuf` に変換して `with_user_manifest` 経路へ渡している（`crates/pod/src/main.rs:59-91`, `crates/pod/src/main.rs:102-115`）。明示 path は既存の `with_user_manifest` により missing file が error になる（`crates/pod/src/factory.rs:123-131`）。override のテストもある（`crates/pod/src/main.rs:332-349`）。 +- env 空文字列は未指定扱い: 満たされている。`OsString::is_empty()` の場合 `None` に落としている（`crates/pod/src/main.rs:84-91`）。`--manifest` との併用でも空 env は許可されるテストがある（`crates/pod/src/main.rs:317-330`）。 +- `--manifest ` の単一ファイル mode: 満たされている。`--manifest` 指定時は factory/cascade 経路へ進まず `load_single_manifest` に return し、`PodManifest::from_toml` と `PromptLoader::builtins_only()` で構築している（`crates/pod/src/main.rs:69-75`, `crates/pod/src/main.rs:94-99`）。 +- `--manifest` と `--project` / `--overlay` の併用不可: 満たされている。clap の `conflicts_with_all = ["project", "overlay"]` が付いており（`crates/pod/src/main.rs:18-21`）、conflict テストもある（`crates/pod/src/main.rs:285-301`）。 +- `--manifest` と `INSOMNIA_USER_MANIFEST` の併用不可: 満たされている。非空 env がある場合は `load_single_manifest` 前に error を返す（`crates/pod/src/main.rs:67-75`）。テストもある（`crates/pod/src/main.rs:303-315`）。 +- 既存の `--project` / `--overlay` / `--adopt` / `--callback` 挙動保持: 満たされている。cascade mode 側では従来通り project 探索と overlay 適用を行い（`crates/pod/src/main.rs:117-130`）、Pod 生成側の adopt/callback/session 分岐は flag 整理の外側で維持されている（`crates/pod/src/main.rs:173-204`）。 +- `pod --overlay ` 起動の互換: 満たされている。CLI 上 `--overlay` は残り、`build_factory_with_user_manifest_path` で従来通り `with_overlay_toml` に渡される（`crates/pod/src/main.rs:29-32`, `crates/pod/src/main.rs:126-130`）。手元の `start_pod.local.fish` は `--overlay` のみを使っており、削除された `--user-manifest` には依存していない（`/start_pod.local.fish:5`）。 + +## アーキテクチャ・スコープ +- 変更は `crates/pod/src/main.rs` の CLI entrypoint に閉じており、manifest cascade 本体や `PodFactory` の責務を拡張していない。`--manifest` は ticket 指定通り `PodManifest::from_toml` の直接経路で、cascade mode は既存 factory を使い続けるため、層境界は保たれている。 +- 単一ファイル mode の prompt loader は `PromptLoader::builtins_only()` として最小構成で組まれており、user / workspace prompt layer や pack file の自動探索を持ち込んでいない（`crates/pod/src/main.rs:94-99`）。 +- 追加依存や crate 構成変更はなく、ticket scope に対して過剰な実装は見当たらない。 + +## 指摘事項 +### Non-blocking / Follow-up +- `docs/pod-factory.md` の CLI 説明が旧仕様のまま残っている — `--user-manifest` を掲載し、`--manifest` と `INSOMNIA_USER_MANIFEST` を説明していない（`docs/pod-factory.md:300-312`）。ticket の完了条件ではないため blocking にはしないが、利用者向けドキュメントとして追随更新した方がよい。 + +## 判断 +Approve with follow-up — ticket の完了条件は満たされており blocking はないが、既存ドキュメントに旧 CLI フラグが残っているため follow-up として扱う。