merge: pod-cli-manifest-flags
This commit is contained in:
commit
3d0dce2a2e
|
|
@ -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 `<config_dir>/manifest.toml`
|
||||
/// (see `manifest::paths`).
|
||||
#[arg(long, value_name = "PATH")]
|
||||
user_manifest: Option<PathBuf>,
|
||||
/// Manifest TOML to use directly, without loading user, project, or
|
||||
/// overlay layers.
|
||||
#[arg(long, value_name = "PATH", conflicts_with_all = ["project", "overlay"])]
|
||||
manifest: Option<PathBuf>,
|
||||
|
||||
/// 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<SessionId>,
|
||||
}
|
||||
|
||||
async fn build_factory(cli: &Cli) -> Result<PodFactory, String> {
|
||||
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<OsString>,
|
||||
) -> 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<OsString>) -> Option<PathBuf> {
|
||||
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<PathBuf>,
|
||||
) -> Result<PodFactory, String> {
|
||||
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<PodFactory, String> {
|
|||
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());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -50,3 +50,8 @@
|
|||
- `--manifest` を `--project` / `--overlay` と併用しようとすると clap が拒否する
|
||||
- `INSOMNIA_USER_MANIFEST` がセットされた状態で `--manifest` を使おうとするとエラーになる
|
||||
- 既存の `pod --overlay <toml>` 起動(`start_pod.local.fish` 経由)が引き続き動く
|
||||
|
||||
## Review
|
||||
- 状態: Approve with follow-up
|
||||
- レビュー詳細: [./pod-cli-manifest-flags.review.md](./pod-cli-manifest-flags.review.md)
|
||||
- 日付: 2026-05-12
|
||||
|
|
|
|||
23
tickets/pod-cli-manifest-flags.review.md
Normal file
23
tickets/pod-cli-manifest-flags.review.md
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
# Review: Pod CLI: マニフェスト関連フラグの整理
|
||||
|
||||
## 前提・要件の確認
|
||||
- `--user-manifest <PATH>` の削除: 満たされている。`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 <PATH>` の単一ファイル 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 <toml>` 起動の互換: 満たされている。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` には依存していない(`/home/hare/Projects/insomnia/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 として扱う。
|
||||
Loading…
Reference in New Issue
Block a user