diff --git a/Cargo.lock b/Cargo.lock index dd045070..7ef32e44 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1255,6 +1255,25 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "include_dir" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "923d117408f1e49d914f1a379a309cffe4f18c05cf4e3d12e613a15fc81bd0dd" +dependencies = [ + "include_dir_macros", +] + +[[package]] +name = "include_dir_macros" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cab85a7ed0bd5f0e76d93846e0147172bed2e2d3f859bcc33a8d9699cad1a75" +dependencies = [ + "proc-macro2", + "quote", +] + [[package]] name = "indexmap" version = "2.13.1" @@ -1489,9 +1508,11 @@ name = "manifest" version = "0.1.0" dependencies = [ "serde", + "serde_ignored", "tempfile", "thiserror 2.0.18", "toml", + "tracing", ] [[package]] @@ -1873,6 +1894,7 @@ dependencies = [ "clap", "dotenv", "futures", + "include_dir", "llm-worker", "manifest", "minijinja", @@ -2417,6 +2439,16 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "serde_ignored" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "115dffd5f3853e06e746965a20dcbae6ee747ae30b543d91b0e089668bb07798" +dependencies = [ + "serde", + "serde_core", +] + [[package]] name = "serde_json" version = "1.0.149" diff --git a/crates/manifest/Cargo.toml b/crates/manifest/Cargo.toml index 0e396240..1887d236 100644 --- a/crates/manifest/Cargo.toml +++ b/crates/manifest/Cargo.toml @@ -6,8 +6,10 @@ license.workspace = true [dependencies] serde = { version = "1.0.228", features = ["derive"] } +serde_ignored = "0.1.14" thiserror = "2.0.18" toml = "1.1.2" +tracing = "0.1.44" [dev-dependencies] tempfile = "3.27.0" diff --git a/crates/manifest/src/config.rs b/crates/manifest/src/config.rs new file mode 100644 index 00000000..1edce78e --- /dev/null +++ b/crates/manifest/src/config.rs @@ -0,0 +1,670 @@ +//! Partial-form of [`crate::PodManifest`] used as cascade layers. +//! +//! `PodManifestConfig` mirrors `PodManifest` but every field is optional +//! so individual layers (builtin defaults, user manifest, project +//! manifest, programmatic overlay) can be partial. Layers are combined +//! via [`PodManifestConfig::merge`] and the final config is converted to +//! a validated [`PodManifest`] via `TryFrom`. + +use std::collections::HashMap; +use std::num::NonZeroU32; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +use crate::defaults; +use crate::{ + CompactionConfig, PodManifest, PodMeta, ProviderConfig, ProviderKind, ScopeConfig, + ToolOutputLimits, WorkerManifest, +}; + +/// Partial-form Pod manifest. Every field is optional; one or more +/// instances merge via [`PodManifestConfig::merge`] before being +/// converted to a validated [`PodManifest`] via `TryFrom`. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct PodManifestConfig { + #[serde(default)] + pub pod: PodMetaConfig, + #[serde(default)] + pub provider: ProviderConfigPartial, + #[serde(default)] + pub worker: WorkerManifestConfig, + #[serde(default)] + pub scope: ScopeConfig, + #[serde(default)] + pub compaction: Option, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct PodMetaConfig { + #[serde(default)] + pub name: Option, + #[serde(default)] + pub pwd: Option, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct ProviderConfigPartial { + #[serde(default)] + pub kind: Option, + #[serde(default)] + pub model: Option, + #[serde(default)] + pub api_key_file: Option, + #[serde(default)] + pub base_url: Option, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct WorkerManifestConfig { + #[serde(default)] + pub system_prompt: Option, + #[serde(default)] + pub max_tokens: Option, + #[serde(default)] + pub max_turns: Option, + #[serde(default)] + pub temperature: Option, + #[serde(default)] + pub tool_output: ToolOutputLimitsPartial, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct ToolOutputLimitsPartial { + #[serde(default)] + pub default_max_bytes: Option, + #[serde(default)] + pub per_tool: HashMap, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct CompactionConfigPartial { + #[serde(default)] + pub prune_protected_turns: Option, + #[serde(default)] + pub prune_min_savings: Option, + #[serde(default)] + pub compact_threshold: Option, + #[serde(default)] + pub compact_retained_turns: Option, + #[serde(default)] + pub provider: Option, +} + +/// Errors raised when converting a [`PodManifestConfig`] to a validated +/// [`PodManifest`] via `TryFrom`. +#[derive(Debug, thiserror::Error)] +pub enum ResolveError { + #[error("missing required field: {0}")] + MissingField(&'static str), + #[error("path must be absolute ({field}): {}", .path.display())] + RelativePath { + field: &'static str, + path: PathBuf, + }, +} + +impl PodManifestConfig { + /// Parse a partial manifest from a TOML string. Unknown top-level or + /// nested fields emit a `tracing::warn!` and are ignored; use + /// `tracing_subscriber` with `WARN` enabled to surface them to the + /// operator. + pub fn from_toml(s: &str) -> Result { + let de = toml::Deserializer::parse(s)?; + serde_ignored::deserialize(de, |path| { + tracing::warn!("unknown field in manifest: {}", path); + }) + } + + /// Cascade layer populated with the in-code defaults listed in + /// [`crate::defaults`]. Used by [`PodFactory::resolve`] as the + /// bottom layer, so every per-field default lives at exactly one + /// call site (the `defaults` module). + /// + /// `TryFrom` also reads the same constants as a + /// belt-and-suspenders fallback, so a manually-constructed config + /// that skips this layer still resolves to the same values. + pub fn builtin_defaults() -> Self { + Self { + worker: WorkerManifestConfig { + tool_output: ToolOutputLimitsPartial { + default_max_bytes: Some(defaults::TOOL_OUTPUT_MAX_BYTES), + per_tool: HashMap::new(), + }, + ..Default::default() + }, + ..Default::default() + } + } + + /// Merge `upper` into `self`. Fields present in `upper` override + /// fields from `self`. Map entries merge key-wise with `upper` + /// winning on conflict. Scope rules from both layers accumulate + /// (see [`ScopeConfig`] semantics). + pub fn merge(self, upper: PodManifestConfig) -> Self { + Self { + pod: self.pod.merge(upper.pod), + provider: self.provider.merge(upper.provider), + worker: self.worker.merge(upper.worker), + scope: merge_scope(self.scope, upper.scope), + compaction: merge_option( + self.compaction, + upper.compaction, + CompactionConfigPartial::merge, + ), + } + } +} + +impl PodMetaConfig { + fn merge(self, upper: Self) -> Self { + Self { + name: upper.name.or(self.name), + pwd: upper.pwd.or(self.pwd), + } + } +} + +impl ProviderConfigPartial { + fn merge(self, upper: Self) -> Self { + Self { + kind: upper.kind.or(self.kind), + model: upper.model.or(self.model), + api_key_file: upper.api_key_file.or(self.api_key_file), + base_url: upper.base_url.or(self.base_url), + } + } +} + +impl WorkerManifestConfig { + fn merge(self, upper: Self) -> Self { + Self { + system_prompt: upper.system_prompt.or(self.system_prompt), + max_tokens: upper.max_tokens.or(self.max_tokens), + max_turns: upper.max_turns.or(self.max_turns), + temperature: upper.temperature.or(self.temperature), + tool_output: self.tool_output.merge(upper.tool_output), + } + } +} + +impl ToolOutputLimitsPartial { + fn merge(self, upper: Self) -> Self { + let mut per_tool = self.per_tool; + per_tool.extend(upper.per_tool); + Self { + default_max_bytes: upper.default_max_bytes.or(self.default_max_bytes), + per_tool, + } + } +} + +impl CompactionConfigPartial { + fn merge(self, upper: Self) -> Self { + Self { + prune_protected_turns: upper.prune_protected_turns.or(self.prune_protected_turns), + prune_min_savings: upper.prune_min_savings.or(self.prune_min_savings), + compact_threshold: upper.compact_threshold.or(self.compact_threshold), + compact_retained_turns: upper + .compact_retained_turns + .or(self.compact_retained_turns), + provider: merge_option(self.provider, upper.provider, ProviderConfigPartial::merge), + } + } +} + +fn merge_scope(mut lower: ScopeConfig, upper: ScopeConfig) -> ScopeConfig { + lower.allow.extend(upper.allow); + lower.deny.extend(upper.deny); + lower +} + +fn merge_option(lower: Option, upper: Option, merge: fn(T, T) -> T) -> Option { + match (lower, upper) { + (Some(l), Some(u)) => Some(merge(l, u)), + (l, u) => u.or(l), + } +} + +fn ensure_absolute(field: &'static str, path: &Path) -> Result<(), ResolveError> { + if path.is_absolute() { + Ok(()) + } else { + Err(ResolveError::RelativePath { + field, + path: path.to_path_buf(), + }) + } +} + +fn resolve_provider( + cfg: ProviderConfigPartial, + kind_field: &'static str, + model_field: &'static str, + api_key_field: &'static str, +) -> Result { + let kind = cfg.kind.ok_or(ResolveError::MissingField(kind_field))?; + let model = cfg.model.ok_or(ResolveError::MissingField(model_field))?; + if let Some(ref p) = cfg.api_key_file { + ensure_absolute(api_key_field, p)?; + } + Ok(ProviderConfig { + kind, + model, + api_key_file: cfg.api_key_file, + base_url: cfg.base_url, + }) +} + +impl TryFrom for PodManifest { + type Error = ResolveError; + + fn try_from(cfg: PodManifestConfig) -> Result { + let name = cfg + .pod + .name + .ok_or(ResolveError::MissingField("pod.name"))?; + let pwd = cfg.pod.pwd.ok_or(ResolveError::MissingField("pod.pwd"))?; + ensure_absolute("pod.pwd", &pwd)?; + + let provider = resolve_provider( + cfg.provider, + "provider.kind", + "provider.model", + "provider.api_key_file", + )?; + + let worker = WorkerManifest { + system_prompt: cfg.worker.system_prompt, + max_tokens: cfg.worker.max_tokens, + max_turns: cfg.worker.max_turns, + temperature: cfg.worker.temperature, + tool_output: ToolOutputLimits { + default_max_bytes: cfg + .worker + .tool_output + .default_max_bytes + .unwrap_or(defaults::TOOL_OUTPUT_MAX_BYTES), + per_tool: cfg.worker.tool_output.per_tool, + }, + }; + + if cfg.scope.allow.is_empty() { + return Err(ResolveError::MissingField("scope.allow")); + } + for rule in &cfg.scope.allow { + ensure_absolute("scope.allow.target", &rule.target)?; + } + for rule in &cfg.scope.deny { + ensure_absolute("scope.deny.target", &rule.target)?; + } + + let compaction = cfg + .compaction + .map(|c| -> Result { + let comp_provider = c + .provider + .map(|p| { + resolve_provider( + p, + "compaction.provider.kind", + "compaction.provider.model", + "compaction.provider.api_key_file", + ) + }) + .transpose()?; + Ok(CompactionConfig { + prune_protected_turns: c + .prune_protected_turns + .unwrap_or(defaults::PRUNE_PROTECTED_TURNS), + prune_min_savings: c + .prune_min_savings + .unwrap_or(defaults::PRUNE_MIN_SAVINGS), + compact_threshold: c.compact_threshold, + compact_retained_turns: c + .compact_retained_turns + .unwrap_or(defaults::COMPACT_RETAINED_TURNS), + provider: comp_provider, + }) + }) + .transpose()?; + + Ok(PodManifest { + pod: PodMeta { name, pwd }, + provider, + worker, + scope: cfg.scope, + compaction, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{Permission, ScopeRule}; + + fn abs(path: &str) -> PathBuf { + PathBuf::from(format!("/tmp/insomnia-test{path}")) + } + + fn minimal_valid() -> PodManifestConfig { + PodManifestConfig { + pod: PodMetaConfig { + name: Some("test".into()), + pwd: Some(abs("/pod")), + }, + provider: ProviderConfigPartial { + kind: Some(ProviderKind::Anthropic), + model: Some("claude-sonnet-4-20250514".into()), + ..Default::default() + }, + worker: WorkerManifestConfig::default(), + scope: ScopeConfig { + allow: vec![ScopeRule { + target: abs("/pod"), + permission: Permission::Write, + recursive: true, + }], + deny: Vec::new(), + }, + compaction: None, + } + } + + #[test] + fn resolve_minimal_succeeds() { + let manifest: PodManifest = minimal_valid().try_into().unwrap(); + assert_eq!(manifest.pod.name, "test"); + assert_eq!(manifest.pod.pwd, abs("/pod")); + assert_eq!(manifest.provider.kind, ProviderKind::Anthropic); + } + + #[test] + fn resolve_rejects_relative_pwd() { + let mut cfg = minimal_valid(); + cfg.pod.pwd = Some(PathBuf::from("./rel")); + let err = PodManifest::try_from(cfg).unwrap_err(); + assert!(matches!( + err, + ResolveError::RelativePath { field: "pod.pwd", .. } + )); + } + + #[test] + fn resolve_rejects_relative_api_key_file() { + let mut cfg = minimal_valid(); + cfg.provider.api_key_file = Some(PathBuf::from("~/.config/key")); + let err = PodManifest::try_from(cfg).unwrap_err(); + assert!(matches!( + err, + ResolveError::RelativePath { + field: "provider.api_key_file", + .. + } + )); + } + + #[test] + fn resolve_rejects_relative_scope_target() { + let mut cfg = minimal_valid(); + cfg.scope.allow[0].target = PathBuf::from("./docs"); + let err = PodManifest::try_from(cfg).unwrap_err(); + assert!(matches!( + err, + ResolveError::RelativePath { + field: "scope.allow.target", + .. + } + )); + } + + #[test] + fn resolve_rejects_missing_pod_name() { + let mut cfg = minimal_valid(); + cfg.pod.name = None; + let err = PodManifest::try_from(cfg).unwrap_err(); + assert!(matches!(err, ResolveError::MissingField("pod.name"))); + } + + #[test] + fn resolve_rejects_empty_scope() { + let mut cfg = minimal_valid(); + cfg.scope.allow.clear(); + let err = PodManifest::try_from(cfg).unwrap_err(); + assert!(matches!(err, ResolveError::MissingField("scope.allow"))); + } + + #[test] + fn merge_scalar_upper_wins() { + let lower = PodManifestConfig { + pod: PodMetaConfig { + name: Some("lower".into()), + pwd: Some(abs("/lower")), + }, + ..Default::default() + }; + let upper = PodManifestConfig { + pod: PodMetaConfig { + name: Some("upper".into()), + pwd: None, + }, + ..Default::default() + }; + let merged = lower.merge(upper); + assert_eq!(merged.pod.name.as_deref(), Some("upper")); + // pwd not present in upper — retain lower + assert_eq!(merged.pod.pwd, Some(abs("/lower"))); + } + + #[test] + fn merge_scope_accumulates_allow_and_deny() { + let lower = PodManifestConfig { + scope: ScopeConfig { + allow: vec![ScopeRule { + target: abs("/a"), + permission: Permission::Read, + recursive: true, + }], + deny: Vec::new(), + }, + ..Default::default() + }; + let upper = PodManifestConfig { + scope: ScopeConfig { + allow: vec![ScopeRule { + target: abs("/b"), + permission: Permission::Write, + recursive: true, + }], + deny: vec![ScopeRule { + target: abs("/a/secret"), + permission: Permission::Read, + recursive: false, + }], + }, + ..Default::default() + }; + let merged = lower.merge(upper); + assert_eq!(merged.scope.allow.len(), 2); + assert_eq!(merged.scope.deny.len(), 1); + } + + #[test] + fn merge_tool_output_per_tool_keywise() { + let lower = PodManifestConfig { + worker: WorkerManifestConfig { + tool_output: ToolOutputLimitsPartial { + default_max_bytes: Some(8192), + per_tool: [("Read".to_string(), 1024)].into_iter().collect(), + }, + ..Default::default() + }, + ..Default::default() + }; + let upper = PodManifestConfig { + worker: WorkerManifestConfig { + tool_output: ToolOutputLimitsPartial { + default_max_bytes: None, + per_tool: [ + ("Read".to_string(), 2048), + ("Grep".to_string(), 512), + ] + .into_iter() + .collect(), + }, + ..Default::default() + }, + ..Default::default() + }; + let merged = lower.merge(upper); + let to = &merged.worker.tool_output; + assert_eq!(to.default_max_bytes, Some(8192)); + assert_eq!(to.per_tool.get("Read"), Some(&2048)); + assert_eq!(to.per_tool.get("Grep"), Some(&512)); + } + + #[test] + fn merge_option_struct_field_wise() { + let lower = PodManifestConfig { + compaction: Some(CompactionConfigPartial { + compact_threshold: Some(50_000), + prune_protected_turns: Some(5), + ..Default::default() + }), + ..Default::default() + }; + let upper = PodManifestConfig { + compaction: Some(CompactionConfigPartial { + compact_threshold: Some(80_000), + ..Default::default() + }), + ..Default::default() + }; + let merged = lower.merge(upper); + let c = merged.compaction.unwrap(); + assert_eq!(c.compact_threshold, Some(80_000)); + // field from lower retained when upper has None + assert_eq!(c.prune_protected_turns, Some(5)); + } + + #[test] + fn from_toml_type_mismatch_is_hard_error() { + let bad = r#" +[pod] +name = "x" +pwd = "/abs" + +[worker] +max_tokens = "not-a-number" +"#; + assert!(PodManifestConfig::from_toml(bad).is_err()); + } + + #[test] + fn from_toml_accepts_unknown_field() { + // Unknown keys are warn-and-ignored, not hard errors. + let ok = r#" +[pod] +name = "x" +pwd = "/abs" + +[worker] +max_tokens = 1000 +unknown_future_field = "tolerated" +"#; + let cfg = PodManifestConfig::from_toml(ok).unwrap(); + assert_eq!(cfg.worker.max_tokens, Some(1000)); + } + + #[test] + fn from_toml_partial_layer_succeeds() { + // A project-layer manifest with only scope set must parse fine. + let toml = r#" +[[scope.allow]] +target = "/abs/project" +permission = "write" +"#; + let cfg = PodManifestConfig::from_toml(toml).unwrap(); + assert!(cfg.pod.name.is_none()); + assert_eq!(cfg.scope.allow.len(), 1); + } + + #[test] + fn builtin_defaults_populates_tool_output_max_bytes() { + let cfg = PodManifestConfig::builtin_defaults(); + assert_eq!( + cfg.worker.tool_output.default_max_bytes, + Some(defaults::TOOL_OUTPUT_MAX_BYTES) + ); + } + + #[test] + fn builtin_defaults_merged_into_minimal_resolves_with_defaults() { + // Starting from builtin_defaults and overlaying only the + // required fields must resolve to a PodManifest carrying the + // centralised default values. + let overlay = PodManifestConfig { + pod: PodMetaConfig { + name: Some("x".into()), + pwd: Some(abs("/pod")), + }, + provider: ProviderConfigPartial { + kind: Some(ProviderKind::Anthropic), + model: Some("m".into()), + ..Default::default() + }, + scope: ScopeConfig { + allow: vec![ScopeRule { + target: abs("/pod"), + permission: Permission::Write, + recursive: true, + }], + deny: Vec::new(), + }, + ..Default::default() + }; + let merged = PodManifestConfig::builtin_defaults().merge(overlay); + let manifest: PodManifest = merged.try_into().unwrap(); + assert_eq!( + manifest.worker.tool_output.default_max_bytes, + defaults::TOOL_OUTPUT_MAX_BYTES + ); + } + + #[test] + fn end_to_end_cascade() { + let builtin = PodManifestConfig::default(); + let user = PodManifestConfig::from_toml( + r#" +[provider] +kind = "anthropic" +model = "claude-sonnet-4-20250514" +"#, + ) + .unwrap(); + let project = PodManifestConfig::from_toml( + r#" +[[scope.allow]] +target = "/abs/project" +permission = "write" +"#, + ) + .unwrap(); + let overlay = PodManifestConfig::from_toml( + r#" +[pod] +name = "dbg" +pwd = "/abs/project" +"#, + ) + .unwrap(); + + let merged = builtin.merge(user).merge(project).merge(overlay); + let manifest: PodManifest = merged.try_into().unwrap(); + assert_eq!(manifest.pod.name, "dbg"); + assert_eq!(manifest.pod.pwd, PathBuf::from("/abs/project")); + assert_eq!(manifest.provider.kind, ProviderKind::Anthropic); + assert_eq!(manifest.scope.allow.len(), 1); + } +} diff --git a/crates/manifest/src/defaults.rs b/crates/manifest/src/defaults.rs new file mode 100644 index 00000000..c6b6cffb --- /dev/null +++ b/crates/manifest/src/defaults.rs @@ -0,0 +1,23 @@ +//! Single source of truth for manifest default values. +//! +//! Every default that would otherwise be duplicated between serde +//! `#[serde(default = "...")]` attributes (on [`crate::PodManifest`]) +//! and the cascade resolution in [`crate::config`] lives here as a +//! `pub const`. Both paths read from this module, so changing a +//! default requires editing exactly one line. + +/// Byte-size cap applied to any tool's `content` output when no +/// per-tool override is set. See [`crate::ToolOutputLimits`]. +pub const TOOL_OUTPUT_MAX_BYTES: usize = 16 * 1024; + +/// Number of most-recent turns protected from pruning. See +/// [`crate::CompactionConfig::prune_protected_turns`]. +pub const PRUNE_PROTECTED_TURNS: usize = 3; + +/// Minimum estimated token savings required to trigger a prune. See +/// [`crate::CompactionConfig::prune_min_savings`]. +pub const PRUNE_MIN_SAVINGS: u64 = 4096; + +/// Number of most-recent turns retained after a compact. See +/// [`crate::CompactionConfig::compact_retained_turns`]. +pub const COMPACT_RETAINED_TURNS: usize = 2; diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index 49e37480..891cde1c 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -1,5 +1,11 @@ +mod config; +pub mod defaults; mod scope; +pub use config::{ + CompactionConfigPartial, PodManifestConfig, PodMetaConfig, ProviderConfigPartial, ResolveError, + ToolOutputLimitsPartial, WorkerManifestConfig, +}; pub use scope::{Scope, ScopeError}; use std::collections::HashMap; @@ -106,7 +112,7 @@ pub struct ToolOutputLimits { } fn default_tool_output_max_bytes() -> usize { - 16 * 1024 + defaults::TOOL_OUTPUT_MAX_BYTES } impl Default for ToolOutputLimits { @@ -206,13 +212,13 @@ pub struct CompactionConfig { } fn default_prune_protected_turns() -> usize { - 3 + defaults::PRUNE_PROTECTED_TURNS } fn default_prune_min_savings() -> u64 { - 4096 + defaults::PRUNE_MIN_SAVINGS } fn default_compact_retained_turns() -> usize { - 2 + defaults::COMPACT_RETAINED_TURNS } impl Default for CompactionConfig { diff --git a/crates/pod/Cargo.toml b/crates/pod/Cargo.toml index cfe855e7..13513503 100644 --- a/crates/pod/Cargo.toml +++ b/crates/pod/Cargo.toml @@ -21,6 +21,7 @@ tracing = "0.1.44" tools = { version = "0.1.0", path = "../tools" } minijinja = "2.19.0" chrono = "0.4.44" +include_dir = "0.7.4" [dev-dependencies] async-trait = "0.1.89" diff --git a/crates/pod/examples/pod_cli.rs b/crates/pod/examples/pod_cli.rs index 83563a3a..5ccb82c6 100644 --- a/crates/pod/examples/pod_cli.rs +++ b/crates/pod/examples/pod_cli.rs @@ -14,10 +14,13 @@ use pod::{Pod, PodManifest, PodRunResult}; use session_store::FsStore; -const MANIFEST_TOML: &str = r#" +fn manifest_toml(pwd: &std::path::Path) -> String { + let pwd = pwd.display(); + format!( + r#" [pod] name = "hello-pod" -pwd = "./" +pwd = "{pwd}" [provider] kind = "anthropic" @@ -28,24 +31,29 @@ system_prompt = "You are a concise assistant. Reply in one or two sentences." max_tokens = 256 [[scope.allow]] -target = "./" +target = "{pwd}" permission = "write" -"#; +"# + ) +} #[tokio::main] async fn main() -> Result<(), Box> { dotenv::dotenv().ok(); - // 1. Parse the manifest - let manifest = PodManifest::from_toml(MANIFEST_TOML)?; - println!("Pod: {}", manifest.pod.name); + // 1. Build a manifest rooted at the current working directory. + // All paths in a manifest must be absolute — see the pod-factory ticket. + let pwd = std::env::current_dir()?; + let toml = manifest_toml(&pwd); // 2. Create a persistent store (temp dir for demo) let tmp = tempfile::tempdir()?; let store = FsStore::new(tmp.path()).await?; - // 3. Build the Pod from manifest - let mut pod = Pod::from_manifest(manifest, store, None).await?; + // 3. Build the Pod from the single-layer manifest TOML + let mut pod = Pod::from_manifest_toml(&toml, store).await?; + let manifest: &PodManifest = pod.manifest(); + println!("Pod: {}", manifest.pod.name); println!("Session: {}", pod.session_id()); // 4. Run a prompt diff --git a/crates/pod/examples/pod_protocol.rs b/crates/pod/examples/pod_protocol.rs index 8df4ecde..92f9781c 100644 --- a/crates/pod/examples/pod_protocol.rs +++ b/crates/pod/examples/pod_protocol.rs @@ -5,13 +5,16 @@ //! cargo run -p pod --example pod_protocol //! ``` -use pod::{Event, Method, PodController, PodManifest}; +use pod::{Event, Method, PodController}; use session_store::FsStore; -const MANIFEST_TOML: &str = r#" +fn manifest_toml(pwd: &std::path::Path) -> String { + let pwd = pwd.display(); + format!( + r#" [pod] name = "protocol-demo" -pwd = "./" +pwd = "{pwd}" [provider] kind = "anthropic" @@ -22,18 +25,22 @@ system_prompt = "You are a concise assistant. Reply in one or two sentences." max_tokens = 256 [[scope.allow]] -target = "./" +target = "{pwd}" permission = "write" -"#; +"# + ) +} #[tokio::main] async fn main() -> Result<(), Box> { dotenv::dotenv().ok(); - let manifest = PodManifest::from_toml(MANIFEST_TOML)?; + // All manifest paths must be absolute — see the pod-factory ticket. + let pwd = std::env::current_dir()?; + let toml = manifest_toml(&pwd); let tmp = tempfile::tempdir()?; let store = FsStore::new(tmp.path()).await?; - let pod = pod::Pod::from_manifest(manifest, store, None).await?; + let pod = pod::Pod::from_manifest_toml(&toml, store).await?; let runtime_tmp = tempfile::tempdir()?; let handle = PodController::spawn(pod, runtime_tmp.path()).await?; diff --git a/crates/pod/src/factory.rs b/crates/pod/src/factory.rs new file mode 100644 index 00000000..06de6c65 --- /dev/null +++ b/crates/pod/src/factory.rs @@ -0,0 +1,539 @@ +//! Builder that assembles a [`PodManifest`] from cascade layers. +//! +//! Layers are merged in order of increasing priority: +//! 1. **Builtin defaults** — in-code defaults, currently empty. Upper +//! layers provide everything; `TryFrom` fills in +//! per-field defaults (`ToolOutputLimits`, `CompactionConfig`, ...). +//! 2. **User manifest** — `$XDG_CONFIG_HOME/insomnia/manifest.toml` +//! (falling back to `~/.config/insomnia/manifest.toml`). +//! 3. **Project manifest** — closest `.insomnia/manifest.toml` found by +//! walking up from `cwd`. +//! 4. **Programmatic overlay** — inline TOML string or typed +//! [`PodManifestConfig`] supplied by the caller (CLI flags, GUI, +//! spawning Pod, etc.). Highest priority. + +use std::path::{Path, PathBuf}; + +use manifest::{PodManifest, PodManifestConfig, ResolveError}; + +use crate::prompt_loader::PromptLoader; + +/// Errors raised while building a [`PodManifest`] from cascade layers. +#[derive(Debug, thiserror::Error)] +pub enum FactoryError { + #[error("failed to read manifest {}: {source}", .path.display())] + Io { + path: PathBuf, + #[source] + source: std::io::Error, + }, + #[error("failed to parse manifest {}: {source}", .path.display())] + Parse { + path: PathBuf, + #[source] + source: toml::de::Error, + }, + #[error("failed to parse overlay TOML: {0}")] + OverlayParse(#[source] toml::de::Error), + #[error("failed to resolve manifest config: {0}")] + Resolve(#[source] ResolveError), + #[error("cannot locate home directory for user manifest lookup")] + HomeDirUnavailable, +} + +/// Builder that accumulates cascade layers and resolves them to a +/// validated [`PodManifest`]. +/// +/// Call order does not matter — layers are always merged in the fixed +/// priority order listed at the module level. Calling the same +/// `with_*` method twice overwrites the previous value for that slot. +#[derive(Debug, Default)] +pub struct PodFactory { + user: Option, + project: Option, + overlay: Option, + /// Directory holding the user prompts library — co-located with + /// the user manifest when loaded. `/prompts/`. + user_prompts_dir: Option, + /// `/.insomnia/prompts/` — co-located with the + /// project manifest when loaded. + project_prompts_dir: Option, +} + +impl PodFactory { + pub fn new() -> Self { + Self::default() + } + + /// Attempt to load the user manifest from the XDG config directory. + /// + /// Looks at `$XDG_CONFIG_HOME/insomnia/manifest.toml` first, then + /// falls back to `$HOME/.config/insomnia/manifest.toml`. If the + /// resolved file does not exist the call is a no-op — user + /// manifests are optional. + pub fn with_user_manifest_auto(mut self) -> Result { + let path = user_manifest_path()?; + if path.exists() { + self.user = Some(read_config_file(&path)?); + self.user_prompts_dir = path.parent().map(|p| p.join("prompts")); + } + Ok(self) + } + + /// Load the user manifest from an explicit path. The file must + /// exist; missing files are an error (unlike the `_auto` variant). + pub fn with_user_manifest(mut self, path: impl AsRef) -> Result { + let path = path.as_ref(); + self.user = Some(read_config_file(path)?); + self.user_prompts_dir = path.parent().map(|p| p.join("prompts")); + Ok(self) + } + + /// Walk up from `cwd` looking for a `.insomnia/manifest.toml` and + /// load it as the project layer. If no project root is found the + /// call is a no-op. + pub fn with_project_manifest_auto(mut self) -> Result { + let cwd = std::env::current_dir().map_err(|source| FactoryError::Io { + path: PathBuf::from("."), + source, + })?; + if let Some(path) = find_project_manifest(&cwd) { + self.project = Some(read_config_file(&path)?); + self.project_prompts_dir = path.parent().map(|p| p.join("prompts")); + } + Ok(self) + } + + /// Walk up from `start` looking for a `.insomnia/manifest.toml`. + /// Explicit variant of [`with_project_manifest_auto`] for tests. + pub fn with_project_manifest_from( + mut self, + start: impl AsRef, + ) -> Result { + if let Some(path) = find_project_manifest(start.as_ref()) { + self.project = Some(read_config_file(&path)?); + self.project_prompts_dir = path.parent().map(|p| p.join("prompts")); + } + Ok(self) + } + + /// Install a programmatic overlay parsed from a TOML string. This + /// is the highest-priority layer — use it to inject per-spawn + /// values like `pod.name` or `pod.pwd` from CLI flags. + 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) + } + + /// Install a programmatic overlay from an already-parsed config. + pub fn with_overlay_config(mut self, config: PodManifestConfig) -> Self { + self.overlay = Some(match self.overlay { + Some(existing) => existing.merge(config), + None => config, + }); + self + } + + /// Build a [`PromptLoader`] that reflects the user / project + /// prompt directories registered with this factory (a sibling of + /// each manifest file: `prompts/`). Missing directories are + /// silently skipped. + fn build_prompt_loader(&self) -> PromptLoader { + let user = self + .user_prompts_dir + .as_ref() + .filter(|p| p.is_dir()) + .cloned(); + let project = self + .project_prompts_dir + .as_ref() + .filter(|p| p.is_dir()) + .cloned(); + PromptLoader::new(user, project) + } + + /// Merge all installed layers, convert the result to a validated + /// [`PodManifest`], and return it together with a [`PromptLoader`] + /// that reflects the user / project prompt directories. The loader + /// feeds `{% include "name" %}` references in the Pod's system + /// prompt template. + /// + /// The base layer is [`PodManifestConfig::builtin_defaults`] so + /// every per-field default flows through a single source of truth + /// (see [`manifest::defaults`]). + pub fn resolve(self) -> Result<(PodManifest, PromptLoader), FactoryError> { + let loader = self.build_prompt_loader(); + let merged = PodManifestConfig::builtin_defaults(); + let merged = match self.user { + Some(user) => merged.merge(user), + None => merged, + }; + let merged = match self.project { + Some(project) => merged.merge(project), + None => merged, + }; + let merged = match self.overlay { + Some(overlay) => merged.merge(overlay), + None => merged, + }; + let manifest = PodManifest::try_from(merged).map_err(FactoryError::Resolve)?; + Ok((manifest, loader)) + } +} + +fn user_manifest_path() -> Result { + if let Ok(dir) = std::env::var("XDG_CONFIG_HOME") { + if !dir.is_empty() { + return Ok(PathBuf::from(dir).join("insomnia").join("manifest.toml")); + } + } + let home = std::env::var("HOME").map_err(|_| FactoryError::HomeDirUnavailable)?; + Ok(PathBuf::from(home) + .join(".config") + .join("insomnia") + .join("manifest.toml")) +} + +fn find_project_manifest(start: &Path) -> Option { + let start = start.canonicalize().ok().unwrap_or_else(|| start.to_path_buf()); + let mut cur: Option<&Path> = Some(start.as_path()); + while let Some(dir) = cur { + let candidate = dir.join(".insomnia").join("manifest.toml"); + if candidate.is_file() { + return Some(candidate); + } + cur = dir.parent(); + } + None +} + +fn read_config_file(path: &Path) -> Result { + let toml = std::fs::read_to_string(path).map_err(|source| FactoryError::Io { + path: path.to_path_buf(), + source, + })?; + PodManifestConfig::from_toml(&toml).map_err(|source| FactoryError::Parse { + path: path.to_path_buf(), + source, + }) +} + +#[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(); + } + + #[test] + fn resolve_overlay_only() { + let tmp = TempDir::new().unwrap(); + let pwd = tmp.path().canonicalize().unwrap(); + let overlay = format!( + r#" +[pod] +name = "solo" +pwd = "{pwd}" + +[provider] +kind = "anthropic" +model = "claude-sonnet-4-20250514" + +[[scope.allow]] +target = "{pwd}" +permission = "write" +"#, + pwd = pwd.display() + ); + let manifest = PodFactory::new() + .with_overlay_toml(&overlay) + .unwrap() + .resolve() + .unwrap(); + let manifest = manifest.0; + assert_eq!(manifest.pod.name, "solo"); + assert_eq!(manifest.pod.pwd, pwd); + } + + #[test] + fn overlay_stacking_merges_in_place() { + let tmp = TempDir::new().unwrap(); + let pwd = tmp.path().canonicalize().unwrap(); + let user_cfg = PodManifestConfig::from_toml(&format!( + r#" +[provider] +kind = "anthropic" +model = "user-model" + +[[scope.allow]] +target = "{pwd}" +permission = "read" +"#, + pwd = pwd.display() + )) + .unwrap(); + let project_cfg = PodManifestConfig::from_toml(&format!( + r#" +[provider] +model = "project-model" + +[[scope.allow]] +target = "{pwd}" +permission = "write" +"#, + pwd = pwd.display() + )) + .unwrap(); + let overlay_cfg = PodManifestConfig::from_toml(&format!( + r#" +[pod] +name = "overlay-name" +pwd = "{pwd}" +"#, + pwd = pwd.display() + )) + .unwrap(); + + let (manifest, _loader) = PodFactory::new() + .with_overlay_config(user_cfg) + .with_overlay_config(project_cfg) + .with_overlay_config(overlay_cfg) + .resolve() + .unwrap(); + + // Note: stacking via with_overlay_config merges into one + // overlay layer so later calls win. This also exercises the + // scope union across layers (two allow rules). + assert_eq!(manifest.pod.name, "overlay-name"); + assert_eq!(manifest.provider.model, "project-model"); + assert_eq!(manifest.scope.allow.len(), 2); + } + + #[test] + fn cascade_priority_layer_ordering() { + let tmp = TempDir::new().unwrap(); + let pwd = tmp.path().canonicalize().unwrap(); + + // Simulate distinct user / project / overlay layers by using + // the dedicated slots on the factory. + let user = tmp.path().join("user.toml"); + write( + &user, + &format!( + r#" +[pod] +name = "from-user" +pwd = "{pwd}" + +[provider] +kind = "anthropic" +model = "user-model" + +[[scope.allow]] +target = "{pwd}" +permission = "write" +"#, + pwd = pwd.display() + ), + ); + + let project_root = tmp.path().join("proj"); + let project_manifest = project_root.join(".insomnia").join("manifest.toml"); + write( + &project_manifest, + r#" +[provider] +model = "project-model" +"#, + ); + + let (manifest, _loader) = PodFactory::new() + .with_user_manifest(&user) + .unwrap() + .with_project_manifest_from(&project_root) + .unwrap() + .resolve() + .unwrap(); + + // project layer overrides user layer on provider.model + assert_eq!(manifest.provider.model, "project-model"); + // user layer provides the rest + assert_eq!(manifest.pod.name, "from-user"); + } + + #[test] + fn project_manifest_walks_up_from_nested_dir() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let project_manifest = root.join(".insomnia").join("manifest.toml"); + write( + &project_manifest, + &format!( + r#" +[pod] +name = "walked-up" +pwd = "{root}" + +[provider] +kind = "anthropic" +model = "claude-sonnet-4-20250514" + +[[scope.allow]] +target = "{root}" +permission = "write" +"#, + root = root.display() + ), + ); + + let nested = root.join("a").join("b").join("c"); + std::fs::create_dir_all(&nested).unwrap(); + + let manifest = PodFactory::new() + .with_project_manifest_from(&nested) + .unwrap() + .resolve() + .unwrap(); + let manifest = manifest.0; + assert_eq!(manifest.pod.name, "walked-up"); + } + + #[test] + fn missing_project_root_is_ok() { + let tmp = TempDir::new().unwrap(); + let pwd = tmp.path().canonicalize().unwrap(); + let overlay = format!( + r#" +[pod] +name = "standalone" +pwd = "{pwd}" + +[provider] +kind = "anthropic" +model = "m" + +[[scope.allow]] +target = "{pwd}" +permission = "write" +"#, + pwd = pwd.display() + ); + + // The temp dir has no .insomnia/ — walking up should skip the + // project layer silently. + let manifest = PodFactory::new() + .with_project_manifest_from(&pwd) + .unwrap() + .with_overlay_toml(&overlay) + .unwrap() + .resolve() + .unwrap(); + let manifest = manifest.0; + assert_eq!(manifest.pod.name, "standalone"); + } + + #[test] + fn resolve_produces_loader_with_project_prompts_dir() { + use crate::system_prompt::{SystemPromptContext, SystemPromptTemplate}; + use manifest::{Permission, Scope, ScopeConfig, ScopeRule}; + + let tmp = TempDir::new().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + // .insomnia/manifest.toml and .insomnia/prompts/coder.md + let manifest_path = root.join(".insomnia").join("manifest.toml"); + write( + &manifest_path, + &format!( + r#" +[pod] +name = "factory-pod" +pwd = "{root}" + +[provider] +kind = "anthropic" +model = "m" + +[[scope.allow]] +target = "{root}" +permission = "write" +"#, + root = root.display() + ), + ); + let project_prompts_dir = root.join(".insomnia").join("prompts"); + std::fs::create_dir_all(&project_prompts_dir).unwrap(); + std::fs::write( + project_prompts_dir.join("coder.md"), + "PROJECT-OVERRIDE from {{ cwd }}", + ) + .unwrap(); + + let (_manifest, loader) = PodFactory::new() + .with_project_manifest_from(&root) + .unwrap() + .resolve() + .unwrap(); + + // The loader must see the project override, not the builtin. + let source = "{% include \"coder\" %}"; + let tmpl = SystemPromptTemplate::parse_with_loader(source, loader).unwrap(); + let scope_cfg = ScopeConfig { + allow: vec![ScopeRule { + target: root.clone(), + permission: Permission::Write, + recursive: true, + }], + deny: Vec::new(), + }; + let scope = Scope::from_config(&scope_cfg, &root).unwrap(); + let ctx = SystemPromptContext { + now: chrono::Utc::now(), + cwd: &root, + scope: &scope, + tool_names: Vec::new(), + files: std::collections::BTreeMap::new(), + }; + let rendered = tmpl.render(&ctx).unwrap(); + assert!( + rendered.starts_with("PROJECT-OVERRIDE"), + "expected project override, got: {rendered}" + ); + } + + #[test] + fn resolve_fails_on_missing_required_field() { + let tmp = TempDir::new().unwrap(); + let pwd = tmp.path().canonicalize().unwrap(); + // pwd set but pod.name missing + let overlay = format!( + r#" +[pod] +pwd = "{pwd}" + +[provider] +kind = "anthropic" +model = "m" + +[[scope.allow]] +target = "{pwd}" +permission = "write" +"#, + pwd = pwd.display() + ); + let err = PodFactory::new() + .with_overlay_toml(&overlay) + .unwrap() + .resolve() + .unwrap_err(); + assert!(matches!(err, FactoryError::Resolve(_))); + } +} diff --git a/crates/pod/src/lib.rs b/crates/pod/src/lib.rs index 2d9de2d1..859b1933 100644 --- a/crates/pod/src/lib.rs +++ b/crates/pod/src/lib.rs @@ -8,8 +8,10 @@ pub mod socket_server; mod agents_md; mod compact_interceptor; mod compact_state; +mod factory; mod hook_interceptor; mod pod; +mod prompt_loader; mod prune; mod system_prompt; mod token_counter; @@ -18,10 +20,12 @@ mod usage_tracker; pub use token_counter::{EstimateSource, SplitPoint, TokenEstimate}; pub use controller::{PodController, PodHandle}; +pub use factory::{FactoryError, PodFactory}; pub use notifier::Notifier; pub use hook::{Hook, HookEventKind, HookRegistryBuilder}; -pub use manifest::{PodManifest, ProviderConfig, ProviderKind, Scope}; +pub use manifest::{PodManifest, PodManifestConfig, ProviderConfig, ProviderKind, Scope}; pub use pod::{Pod, PodError, PodRunResult, apply_worker_manifest}; +pub use prompt_loader::PromptLoader; pub use protocol::{ErrorCode, Event, Method, TurnResult}; pub use provider::{ProviderError, build_client}; pub use runtime_dir::RuntimeDir; diff --git a/crates/pod/src/main.rs b/crates/pod/src/main.rs index daae7cc0..afe7fc2b 100644 --- a/crates/pod/src/main.rs +++ b/crates/pod/src/main.rs @@ -1,18 +1,39 @@ -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::process::ExitCode; use clap::Parser; -use pod::{Pod, PodController}; +use pod::{Pod, PodController, PodFactory}; use session_store::FsStore; #[derive(Parser)] -#[command(name = "pod", about = "Run a Pod process from a manifest file")] +#[command( + name = "pod", + about = "Spawn a Pod process from cascaded manifest layers" +)] struct Cli { - /// Path to the manifest TOML file - #[arg(short, long)] - manifest: PathBuf, + /// User manifest TOML. Defaults to + /// `$XDG_CONFIG_HOME/insomnia/manifest.toml`. + #[arg(long, value_name = "PATH")] + user_manifest: Option, - /// Directory for session persistence (default: ~/.insomnia/sessions/) + /// Start the project-manifest walk from this directory. When + /// omitted, the factory walks up from the current working + /// directory looking for `.insomnia/manifest.toml`. + #[arg(long, value_name = "PATH")] + project: Option, + + /// Inline TOML string applied as the highest-priority overlay + /// layer. Example: `--overlay 'pod.name = "dbg"'`. + #[arg(long, value_name = "TOML")] + overlay: Option, + + /// Shorthand that injects `pod.pwd = ` into the overlay + /// layer. `--pwd .` uses the current working directory. + #[arg(long, value_name = "PATH")] + pwd: Option, + + /// Directory for session persistence. Defaults to + /// `~/.insomnia/sessions/`. #[arg(short, long)] store: Option, } @@ -36,30 +57,83 @@ 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")) + } +} + +async fn build_factory(cli: &Cli) -> Result { + let mut factory = PodFactory::new(); + + factory = match &cli.user_manifest { + Some(path) => factory + .with_user_manifest(path) + .map_err(|e| format!("failed to load user manifest: {e}"))?, + None => factory + .with_user_manifest_auto() + .map_err(|e| format!("failed to auto-load user manifest: {e}"))?, + }; + + factory = match &cli.project { + Some(path) => factory + .with_project_manifest_from(path) + .map_err(|e| format!("failed to load project manifest: {e}"))?, + None => factory + .with_project_manifest_auto() + .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()) { + factory = factory + .with_overlay_toml(&overlay) + .map_err(|e| format!("failed to parse overlay TOML: {e}"))?; + } + + Ok(factory) +} + #[tokio::main] async fn main() -> ExitCode { let cli = Cli::parse(); - // Read and parse the manifest - let toml_str = match tokio::fs::read_to_string(&cli.manifest).await { - Ok(s) => s, + let factory = match build_factory(&cli).await { + Ok(f) => f, Err(e) => { - eprintln!("error: failed to read manifest {:?}: {e}", cli.manifest); - return ExitCode::FAILURE; - } - }; - let manifest = match manifest::PodManifest::from_toml(&toml_str) { - Ok(m) => m, - Err(e) => { - eprintln!("error: invalid manifest: {e}"); + eprintln!("error: {e}"); return ExitCode::FAILURE; } }; - let pod_name = manifest.pod.name.clone(); + let (manifest, loader) = match factory.resolve() { + Ok(pair) => pair, + Err(e) => { + eprintln!("error: failed to resolve manifest cascade: {e}"); + return ExitCode::FAILURE; + } + }; // Initialize persistent store - let store_dir = cli.store.unwrap_or_else(|| { + let store_dir = cli.store.clone().unwrap_or_else(|| { default_store_dir().unwrap_or_else(|_| PathBuf::from(".insomnia/sessions")) }); let store = match FsStore::new(&store_dir).await { @@ -70,17 +144,14 @@ async fn main() -> ExitCode { } }; - // Build the Pod (pwd/scope derived from manifest + manifest_dir). - let manifest_dir = std::fs::canonicalize(&cli.manifest) - .ok() - .and_then(|p| p.parent().map(Path::to_path_buf)); - let pod = match Pod::from_manifest(manifest, store, manifest_dir).await { + let pod = match Pod::from_manifest(manifest, store, loader).await { Ok(p) => p, Err(e) => { eprintln!("error: failed to create pod: {e}"); return ExitCode::FAILURE; } }; + let pod_name = pod.manifest().pod.name.clone(); // Spawn the controller (starts socket server) let runtime_base = match default_runtime_dir() { @@ -113,8 +184,6 @@ async fn main() -> ExitCode { } } - // TODO: handle.shutdown().await — PodController にグレースフルシャットダウン機構を追加したら組み込む drop(handle); - ExitCode::SUCCESS } diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index 23d93b29..073c4a94 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -11,7 +11,7 @@ use session_store::{ }; use tracing::{info, warn}; -use manifest::{PodManifest, Scope, ScopeError, WorkerManifest}; +use manifest::{PodManifest, PodManifestConfig, ResolveError, Scope, ScopeError, WorkerManifest}; use crate::agents_md::read_agents_md; use crate::compact_interceptor::CompactInterceptor; @@ -22,6 +22,7 @@ use crate::hook::{ }; use crate::hook_interceptor::HookInterceptor; use crate::notifier::Notifier; +use crate::prompt_loader::PromptLoader; use crate::system_prompt::{SystemPromptContext, SystemPromptError, SystemPromptTemplate}; use crate::usage_tracker::UsageTracker; use protocol::{NotificationLevel, NotificationSource}; @@ -74,8 +75,6 @@ pub struct Pod { scope: Scope, hook_builder: HookRegistryBuilder, interceptor_installed: bool, - /// Directory containing the manifest file (needed for api_key_file resolution). - manifest_dir: Option, /// Shared compaction state (present when compact_threshold is configured). compact_state: Option>, /// Per-LLM-request Usage tracker. Always present after construction. @@ -136,7 +135,6 @@ impl Pod { scope, hook_builder: HookRegistryBuilder::new(), interceptor_installed: false, - manifest_dir: None, compact_state: None, usage_tracker: Arc::new(UsageTracker::new()), usage_history: Arc::new(Mutex::new(Vec::::new())), @@ -185,7 +183,6 @@ impl Pod { scope, hook_builder: HookRegistryBuilder::new(), interceptor_installed: false, - manifest_dir: None, compact_state: None, usage_tracker: Arc::new(UsageTracker::new()), usage_history: Arc::new(Mutex::new(state.usage_history)), @@ -807,10 +804,7 @@ impl Pod { fn build_compactor_client(&self) -> Result, PodError> { if let Some(ref compaction) = self.manifest.compaction { if let Some(ref provider_config) = compaction.provider { - let client = provider::build_client( - provider_config, - self.manifest_dir.as_deref().map(|p| p.as_ref()), - )?; + let client = provider::build_client(provider_config)?; return Ok(client); } } @@ -820,24 +814,30 @@ impl Pod { } impl Pod, St> { - /// Create a Pod entirely from a manifest. + /// Create a Pod entirely from a validated manifest. /// - /// Resolves `manifest.pod.pwd` against `manifest_dir` (or the - /// current working directory when absent), builds the [`Scope`] - /// from `manifest.scope`, and validates that the resolved pwd is - /// readable under that scope. + /// `manifest.pod.pwd` must already be an absolute path (the cascade + /// layer — `PodManifestConfig` → `PodManifest` — is the sole place + /// where path normalisation happens). The Pod builds its [`Scope`] + /// from `manifest.scope`, canonicalizes the pwd, and validates that + /// the resolved pwd is readable under that scope. + /// + /// `loader` is installed into the system-prompt template + /// environment so that `{% include "name" %}` / + /// `{% import "name" %}` references resolve against the three-layer + /// prompt asset library. pub async fn from_manifest( manifest: PodManifest, store: St, - manifest_dir: Option, + loader: PromptLoader, ) -> Result { - let pwd = resolve_pwd(&manifest.pod.pwd, manifest_dir.as_deref())?; + let pwd = resolve_pwd(&manifest.pod.pwd)?; let scope = Scope::from_config(&manifest.scope, &pwd).map_err(PodError::Scope)?; if !scope.is_readable(&pwd) { return Err(PodError::PwdOutsideScope { pwd }); } - let client = provider::build_client(&manifest.provider, manifest_dir.as_deref())?; + let client = provider::build_client(&manifest.provider)?; let mut worker = Worker::new(client); apply_worker_manifest(&mut worker, &manifest.worker); @@ -847,7 +847,7 @@ impl Pod, St> { // scope summary, ...) can be injected. let system_prompt_template = match manifest.worker.system_prompt.as_deref() { Some(source) => Some( - SystemPromptTemplate::parse(source) + SystemPromptTemplate::parse_with_loader(source, loader) .map_err(|source| PodError::InvalidSystemPromptTemplate { source })?, ), None => None, @@ -867,7 +867,6 @@ impl Pod, St> { scope, hook_builder: HookRegistryBuilder::new(), interceptor_installed: false, - manifest_dir, compact_state: None, usage_tracker: Arc::new(UsageTracker::new()), usage_history: Arc::new(Mutex::new(Vec::new())), @@ -878,6 +877,18 @@ impl Pod, St> { pod.apply_prune_from_manifest(); Ok(pod) } + + /// Convenience: build a Pod from a single-layer TOML manifest string. + /// + /// Parses the TOML into a [`PodManifestConfig`], converts to a + /// validated [`PodManifest`] via `TryFrom`, then delegates to + /// [`Pod::from_manifest`]. Useful for tests, debugging, and any + /// caller that wants to skip the cascade entirely. + pub async fn from_manifest_toml(toml: &str, store: St) -> Result { + let config = PodManifestConfig::from_toml(toml).map_err(PodError::ManifestParse)?; + let manifest = PodManifest::try_from(config).map_err(PodError::ManifestResolve)?; + Self::from_manifest(manifest, store, PromptLoader::builtins_only()).await + } } /// Apply worker-level manifest settings to a Worker. @@ -984,6 +995,15 @@ pub enum PodError { source: std::io::Error, }, + #[error("pwd must be absolute: {}", .0.display())] + PwdNotAbsolute(PathBuf), + + #[error("failed to parse manifest TOML: {0}")] + ManifestParse(#[source] toml::de::Error), + + #[error("failed to resolve manifest config: {0}")] + ManifestResolve(#[source] ResolveError), + #[error(transparent)] Provider(#[from] provider::ProviderError), @@ -1003,22 +1023,16 @@ pub enum PodError { }, } -/// Resolve the pwd declared in a manifest against `manifest_dir` (or the -/// current working directory when absent), canonicalizing symlinks. -fn resolve_pwd(pwd: &Path, manifest_dir: Option<&Path>) -> Result { - let joined = if pwd.is_absolute() { - pwd.to_path_buf() - } else { - let base = manifest_dir - .map(Path::to_path_buf) - .or_else(|| std::env::current_dir().ok()) - .unwrap_or_else(|| PathBuf::from(".")); - base.join(pwd) - }; - joined - .canonicalize() - .map_err(|source| PodError::InvalidPwd { - pwd: joined, - source, - }) +/// Canonicalize an absolute pwd (resolves symlinks and any `.`/`..` +/// components). Relative inputs are rejected — the cascade layer is +/// the sole source of path normalisation and must hand off an absolute +/// path. +fn resolve_pwd(pwd: &Path) -> Result { + if !pwd.is_absolute() { + return Err(PodError::PwdNotAbsolute(pwd.to_path_buf())); + } + pwd.canonicalize().map_err(|source| PodError::InvalidPwd { + pwd: pwd.to_path_buf(), + source, + }) } diff --git a/crates/pod/src/prompt_loader.rs b/crates/pod/src/prompt_loader.rs new file mode 100644 index 00000000..7ae14795 --- /dev/null +++ b/crates/pod/src/prompt_loader.rs @@ -0,0 +1,137 @@ +//! Three-layer prompt asset loader used by [`crate::SystemPromptTemplate`]. +//! +//! Layers (highest priority first): +//! 1. **Project prompts** — `/.insomnia/prompts/` +//! 2. **User prompts** — `$XDG_CONFIG_HOME/insomnia/prompts/` +//! 3. **Builtin prompts** — baked into the binary from `resources/prompts/` +//! via [`include_dir!`]. +//! +//! A prompt name is its path stem without the `.md` extension. +//! Subdirectories are supported: `common/tool-usage` maps to +//! `common/tool-usage.md` under whichever layer provides it first. + +use std::path::{Path, PathBuf}; + +use include_dir::{Dir, include_dir}; + +static BUILTIN_PROMPTS: Dir<'static> = + include_dir!("$CARGO_MANIFEST_DIR/../../resources/prompts"); + +/// Lookup table for prompt assets across the three cascade layers. +#[derive(Debug, Clone)] +pub struct PromptLoader { + user_dir: Option, + project_dir: Option, +} + +impl PromptLoader { + /// Builtins-only loader. Used for direct `Pod::from_manifest` + /// calls that skip the factory cascade (tests, examples, simple + /// callers). + pub fn builtins_only() -> Self { + Self { + user_dir: None, + project_dir: None, + } + } + + /// Loader with optional user and project prompts directories. Both + /// are consulted before falling back to builtins; `None` on either + /// skips that layer. + pub fn new(user_dir: Option, project_dir: Option) -> Self { + Self { + user_dir, + project_dir, + } + } + + /// Look up the raw template source for `name`. Returns `None` if + /// no layer provides it. + pub fn lookup(&self, name: &str) -> Option { + if let Some(ref dir) = self.project_dir { + if let Some(s) = read_from_dir(dir, name) { + return Some(s); + } + } + if let Some(ref dir) = self.user_dir { + if let Some(s) = read_from_dir(dir, name) { + return Some(s); + } + } + read_from_include_dir(&BUILTIN_PROMPTS, name) + } +} + +fn read_from_dir(dir: &Path, name: &str) -> Option { + let path = dir.join(format!("{name}.md")); + std::fs::read_to_string(path).ok() +} + +fn read_from_include_dir(dir: &Dir<'static>, name: &str) -> Option { + let path = format!("{name}.md"); + dir.get_file(&path) + .and_then(|f| f.contents_utf8()) + .map(|s| s.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn builtin_coder_prompt_present() { + let loader = PromptLoader::builtins_only(); + let coder = loader.lookup("coder").expect("coder builtin missing"); + assert!(coder.contains("software engineering agent")); + } + + #[test] + fn builtin_subdirectory_lookup() { + let loader = PromptLoader::builtins_only(); + let tu = loader + .lookup("common/tool-usage") + .expect("common/tool-usage missing"); + assert!(tu.contains("tool")); + } + + #[test] + fn unknown_name_returns_none() { + let loader = PromptLoader::builtins_only(); + assert!(loader.lookup("definitely-not-a-prompt").is_none()); + } + + #[test] + fn user_layer_overrides_builtin() { + let tmp = TempDir::new().unwrap(); + let user_dir = tmp.path().to_path_buf(); + std::fs::write(user_dir.join("coder.md"), "user-coder").unwrap(); + + let loader = PromptLoader::new(Some(user_dir), None); + assert_eq!(loader.lookup("coder").as_deref(), Some("user-coder")); + } + + #[test] + fn project_layer_overrides_user_and_builtin() { + let tmp = TempDir::new().unwrap(); + let user_dir = tmp.path().join("user"); + let project_dir = tmp.path().join("project"); + std::fs::create_dir_all(&user_dir).unwrap(); + std::fs::create_dir_all(&project_dir).unwrap(); + std::fs::write(user_dir.join("coder.md"), "user-coder").unwrap(); + std::fs::write(project_dir.join("coder.md"), "project-coder").unwrap(); + + let loader = PromptLoader::new(Some(user_dir), Some(project_dir)); + assert_eq!(loader.lookup("coder").as_deref(), Some("project-coder")); + } + + #[test] + fn falls_through_to_builtin_when_user_missing_name() { + let tmp = TempDir::new().unwrap(); + let user_dir = tmp.path().to_path_buf(); + // user layer only defines "only-user", not "coder" + std::fs::write(user_dir.join("only-user.md"), "x").unwrap(); + let loader = PromptLoader::new(Some(user_dir), None); + assert!(loader.lookup("coder").is_some()); // from builtin + } +} diff --git a/crates/pod/src/system_prompt.rs b/crates/pod/src/system_prompt.rs index 6d0a3cbe..2d2ed385 100644 --- a/crates/pod/src/system_prompt.rs +++ b/crates/pod/src/system_prompt.rs @@ -14,9 +14,11 @@ use std::sync::Arc; use chrono::{DateTime, SecondsFormat, Utc}; use manifest::Scope; use minijinja::value::Value; -use minijinja::{Environment, UndefinedBehavior}; +use minijinja::{Environment, ErrorKind, UndefinedBehavior}; use thiserror::Error; +use crate::prompt_loader::PromptLoader; + const TEMPLATE_NAME: &str = "system_prompt"; #[derive(Debug, Error)] @@ -35,11 +37,31 @@ pub struct SystemPromptTemplate { } impl SystemPromptTemplate { - /// Parse a template source. Performs syntax validation only — no - /// variable resolution is attempted here. + /// Parse a template source with a builtins-only prompt loader. + /// Convenience wrapper for callers that do not need user/project + /// prompt layers — see [`SystemPromptTemplate::parse_with_loader`] + /// for the factory-driven path. pub fn parse(source: impl Into) -> Result { + Self::parse_with_loader(source, PromptLoader::builtins_only()) + } + + /// Parse a template source with a custom prompt loader installed. + /// The loader resolves `{% include "name" %}` / `{% import "name" %}` + /// references by consulting the cascade layers (project → user → + /// builtin) before reporting a missing template. + pub fn parse_with_loader( + source: impl Into, + loader: PromptLoader, + ) -> Result { let mut env = Environment::new(); env.set_undefined_behavior(UndefinedBehavior::Strict); + env.set_loader(move |name| match loader.lookup(name) { + Some(source) => Ok(Some(source)), + None => Err(minijinja::Error::new( + ErrorKind::TemplateNotFound, + format!("prompt asset '{name}' not found"), + )), + }); env.add_template_owned(TEMPLATE_NAME, source.into()) .map_err(|e| SystemPromptError::Parse(e.to_string()))?; Ok(Self { env: Arc::new(env) }) @@ -230,6 +252,45 @@ mod tests { assert!(rendered.contains(&dir.path().canonicalize().unwrap().display().to_string())); } + #[test] + fn include_resolves_builtin_prompt() { + // User-supplied source pulls in a builtin via the loader. + let source = "HEAD\n{% include \"common/tool-usage\" %}"; + let tmpl = SystemPromptTemplate::parse_with_loader( + source, + PromptLoader::builtins_only(), + ) + .unwrap(); + let dir = TempDir::new().unwrap(); + let scope = build_scope(dir.path()); + let rendered = tmpl + .render(&ctx( + dir.path(), + &scope, + vec!["Read".into(), "Edit".into()], + )) + .unwrap(); + assert!(rendered.starts_with("HEAD")); + // The common/tool-usage builtin references {{ tools | join(", ") }} + // so including it must have resolved that expression with the + // parent scope's variables. + assert!(rendered.contains("Read")); + assert!(rendered.contains("Edit")); + } + + #[test] + fn include_unknown_prompt_fails_at_render() { + let tmpl = SystemPromptTemplate::parse_with_loader( + "{% include \"nonexistent-prompt\" %}", + PromptLoader::builtins_only(), + ) + .unwrap(); + let dir = TempDir::new().unwrap(); + let scope = build_scope(dir.path()); + let err = tmpl.render(&ctx(dir.path(), &scope, vec![])).unwrap_err(); + assert!(matches!(err, SystemPromptError::Render(_))); + } + #[test] fn files_reserved_namespace_is_empty() { let t = SystemPromptTemplate::parse( diff --git a/crates/provider/src/lib.rs b/crates/provider/src/lib.rs index 7666b17d..ffc10915 100644 --- a/crates/provider/src/lib.rs +++ b/crates/provider/src/lib.rs @@ -1,5 +1,3 @@ -use std::path::{Path, PathBuf}; - use llm_worker::llm_client::client::LlmClient; use llm_worker::llm_client::providers::anthropic::AnthropicClient; use llm_worker::llm_client::providers::gemini::GeminiClient; @@ -22,22 +20,23 @@ pub enum ProviderError { /// /// Resolution order: /// 1. Environment variable `INSOMNIA_API_KEY_{KIND}` -/// 2. File specified by `api_key_file` (trimmed) +/// 2. File specified by `api_key_file` (must be an absolute path; the +/// cascade layer is responsible for normalisation) /// 3. `None` -fn resolve_api_key( - config: &ProviderConfig, - manifest_dir: Option<&Path>, -) -> Result, ProviderError> { - // 1. Convention-based environment variable +fn resolve_api_key(config: &ProviderConfig) -> Result, ProviderError> { let env_name = config.kind.env_var_name(); if let Ok(val) = std::env::var(&env_name) { return Ok(Some(val)); } - // 2. File - if let Some(ref raw_path) = config.api_key_file { - let path = expand_key_path(raw_path, manifest_dir)?; - let contents = std::fs::read_to_string(&path).map_err(|e| { + if let Some(ref path) = config.api_key_file { + if !path.is_absolute() { + return Err(ProviderError::Config(format!( + "api_key_file must be absolute: {}", + path.display() + ))); + } + let contents = std::fs::read_to_string(path).map_err(|e| { ProviderError::Config(format!( "failed to read api_key_file {}: {e}", path.display() @@ -49,38 +48,13 @@ fn resolve_api_key( Ok(None) } -/// Expand `~` and resolve relative paths against `manifest_dir`. -fn expand_key_path(raw: &Path, manifest_dir: Option<&Path>) -> Result { - let path = if raw.starts_with("~") { - let home = std::env::var("HOME") - .map_err(|_| ProviderError::Config("HOME is not set for ~ expansion".into()))?; - PathBuf::from(home).join(raw.strip_prefix("~").unwrap()) - } else { - raw.to_path_buf() - }; - - if path.is_relative() { - match manifest_dir { - Some(dir) => Ok(dir.join(&path)), - None => Err(ProviderError::Config(format!( - "relative api_key_file '{}' requires a manifest directory", - path.display() - ))), - } - } else { - Ok(path) - } -} - /// Build an [`LlmClient`] from a [`ProviderConfig`]. /// -/// Resolves the API key from `INSOMNIA_API_KEY_{KIND}` env var or `api_key_file`. -/// `manifest_dir` is used to resolve relative `api_key_file` paths. -pub fn build_client( - config: &ProviderConfig, - manifest_dir: Option<&Path>, -) -> Result, ProviderError> { - let api_key = resolve_api_key(config, manifest_dir)?; +/// `api_key_file` (if set) must already be an absolute path — relative +/// paths are rejected because cascade resolution is the sole source of +/// path normalisation. +pub fn build_client(config: &ProviderConfig) -> Result, ProviderError> { + let api_key = resolve_api_key(config)?; match config.kind { ProviderKind::Anthropic => { @@ -128,6 +102,7 @@ mod tests { use super::*; use serial_test::serial; use std::io::Write; + use std::path::PathBuf; fn anthropic_config() -> ProviderConfig { ProviderConfig { @@ -143,7 +118,7 @@ mod tests { fn resolve_from_env() { let env_name = ProviderKind::Anthropic.env_var_name(); unsafe { std::env::set_var(&env_name, "sk-from-env") }; - let key = resolve_api_key(&anthropic_config(), None).unwrap(); + let key = resolve_api_key(&anthropic_config()).unwrap(); unsafe { std::env::remove_var(&env_name) }; assert_eq!(key.as_deref(), Some("sk-from-env")); } @@ -160,7 +135,7 @@ mod tests { api_key_file: Some(key_path), ..anthropic_config() }; - let key = resolve_api_key(&config, None).unwrap(); + let key = resolve_api_key(&config).unwrap(); assert_eq!(key.as_deref(), Some("sk-from-file")); } @@ -178,40 +153,25 @@ mod tests { api_key_file: Some(key_path), ..anthropic_config() }; - let key = resolve_api_key(&config, None).unwrap(); + let key = resolve_api_key(&config).unwrap(); unsafe { std::env::remove_var(&env_name) }; assert_eq!(key.as_deref(), Some("sk-from-env")); } #[test] - fn relative_path_resolved_against_manifest_dir() { - let dir = tempfile::tempdir().unwrap(); - let key_path = dir.path().join("keys").join("anthropic"); - std::fs::create_dir_all(key_path.parent().unwrap()).unwrap(); - std::fs::write(&key_path, "sk-relative").unwrap(); - + fn relative_api_key_file_is_rejected() { let config = ProviderConfig { api_key_file: Some(PathBuf::from("keys/anthropic")), ..anthropic_config() }; - let key = resolve_api_key(&config, Some(dir.path())).unwrap(); - assert_eq!(key.as_deref(), Some("sk-relative")); - } - - #[test] - fn relative_path_without_manifest_dir_errors() { - let config = ProviderConfig { - api_key_file: Some(PathBuf::from("keys/anthropic")), - ..anthropic_config() - }; - let err = resolve_api_key(&config, None).unwrap_err(); + let err = resolve_api_key(&config).unwrap_err(); assert!(matches!(err, ProviderError::Config(_))); } #[test] fn missing_key_returns_api_key_missing() { let config = anthropic_config(); - let result = build_client(&config, None); + let result = build_client(&config); assert!(matches!(result, Err(ProviderError::ApiKeyMissing { .. }))); } @@ -223,6 +183,6 @@ mod tests { api_key_file: None, base_url: None, }; - assert!(build_client(&config, None).is_ok()); + assert!(build_client(&config).is_ok()); } } diff --git a/resources/prompts/coder.md b/resources/prompts/coder.md new file mode 100644 index 00000000..3be58733 --- /dev/null +++ b/resources/prompts/coder.md @@ -0,0 +1,7 @@ +You are a focused software engineering agent operating in {{ cwd }}. + +Today is {{ date }}. Stay precise, edit code directly when asked, and +avoid speculative refactoring. Explain what you changed in one short +paragraph at the end of each turn. + +{% include "common/tool-usage" %} diff --git a/resources/prompts/common/tool-usage.md b/resources/prompts/common/tool-usage.md new file mode 100644 index 00000000..330121e4 --- /dev/null +++ b/resources/prompts/common/tool-usage.md @@ -0,0 +1,11 @@ +## Tool usage + +You have access to these tools: {{ tools | join(", ") }}. + +Prefer the most specific tool for the job. When reading files you already +know the path of, use the file-read tool directly instead of searching. +When searching, use grep/glob primitives rather than shell pipelines. + +Only touch paths inside your scope. Your scope is: + +{{ scope.summary }} diff --git a/resources/prompts/planner.md b/resources/prompts/planner.md new file mode 100644 index 00000000..ac3090f1 --- /dev/null +++ b/resources/prompts/planner.md @@ -0,0 +1,7 @@ +You are a planning agent operating in {{ cwd }}. + +Today is {{ date }}. Produce a concise, implementation-ready plan: list +the files to touch, the order of changes, and the risks. Do not write +code unless asked. + +{% include "common/tool-usage" %} diff --git a/resources/prompts/reviewer.md b/resources/prompts/reviewer.md new file mode 100644 index 00000000..c281e54e --- /dev/null +++ b/resources/prompts/reviewer.md @@ -0,0 +1,7 @@ +You are a code reviewer operating in {{ cwd }}. + +Today is {{ date }}. Read the diff or the requested files, then report +problems grouped by severity (blocking / recommended / nit). Quote +specific lines. Do not rewrite the code unless asked. + +{% include "common/tool-usage" %} diff --git a/tickets/pod-factory.md b/tickets/pod-factory.md index cc42f46a..0847721f 100644 --- a/tickets/pod-factory.md +++ b/tickets/pod-factory.md @@ -1,5 +1,10 @@ # 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 の**作成自体**も自動化可能である必要がある。 @@ -21,39 +26,50 @@ Pod 作成を「**最終的に `PodManifest` を1 つ構築する問題**」と ### 同じ型で、層で上書きする -- **解決後の型は現行の `manifest::PodManifest` のまま**。Pod 側の契約（`Pod::from_manifest`）は変更しない。 -- カスケードは `PodManifest` より上の「部分的な `PodManifest` を層ごとに保持し、順番にマージして最終形を作る」層として設計する。 +- **解決後の型は現行の `manifest::PodManifest` のまま**（必須フィールド持ち）。 +- カスケードの各層は **`PodManifestConfig`**（`PodManifest` と同じ構造だが全フィールドを `Option` / 部分形で保持する新規型）で表現する。部分形同士を順番にマージし、最後に `TryFrom for PodManifest` で必須チェックを行いつつ確定させる。 - 各層は**部分形**を持てる（全フィールドを埋める必要はない）。存在するフィールドだけが下層を上書きする。 +- 人間が書くときは `PodManifest` と同じ TOML スキーマで書く（サブセット可）。ファイル名は**すべて `manifest.toml`**。「設定」という曖昧な名前を避け、「Pod manifest」という語彙に揃える。 ### カスケードの層 優先順位が低い方から高い方へ: 1. **ビルトインのデフォルト**: コードに焼き込んだ基本値（現在 `PodManifest` 各フィールドの `#[serde(default)]` や `Default` 実装に散っているものを集約） -2. **ユーザー設定**: `~/.config/insomnia/config.toml` など。ユーザー個人のプロバイダ指定・デフォルトモデル・常用ツール設定等を書く -3. **プロジェクト設定**: プロジェクト直下の `.insomnia/config.toml` など。プロジェクト固有の scope・compaction 設定・system_prompt のベース等を書く -4. **プログラマティック上書き**: Pod 生成を呼ぶコード（GUI / CLI / 別 Pod からの spawn 等）が渡す `PodManifestOverlay` 的な部分形。ここで `pod.name` や `pod.pwd` のような**その Pod に固有の値**を与える +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 に固有の値**を与える -各層とも人間が書くときは `PodManifest` と同じ TOML スキーマで書く（サブセット可）。 +### プロジェクトルートの判定 + +起動ディレクトリから上方向に `.insomnia/` ディレクトリを探索し、**最も近い**ものをプロジェクトルートとする。見つからなければ「プロジェクト無し」として扱い、プロジェクト層をスキップする。 ### マージのセマンティクス - **スカラー** (`String`, `u32`, `bool` 等): 上層が存在すれば丸ごと置換 - **Option 型**: 上層が `Some` なら置換、`None` なら据え置き - **マップ** (例: `tool_output.per_tool`): キー単位でマージ、同一キーは上層優先 -- **リスト** (例: `scope.allow` / `scope.deny`): **原則置換**（append にすると下層の意図しないルールが漏れる危険）。ただし append したいケースはあるので、設計時に decoration の形式（例: `scope.allow_extra` など）を別途検討 -- 未知フィールドは manifest エラーにせずログ警告 +- **`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. **ビルトイン**: バイナリに同梱されたデフォルトプロンプト（`coder` / `reviewer` / `planner` 等、設計時に選定） - 2. **ユーザー**: `~/.config/insomnia/prompts/*.md` 等 - 3. **プロジェクト**: `.insomnia/prompts/*.md` 等 -- 同名があれば**上層が優先**。 -- 既存の `SystemPromptTemplate`（minijinja ベース）のローダを拡張し、テンプレート内から他のプロンプトを `{% include "coder" %}` / `{% import "planner" as p %}` のように参照できるようにする。 -- 層の異なる同名プロンプトを合成するための include 先解決は上記優先順位に従う。 +- 検索パスは以下の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 のデフォルト挙動**（親スコープの変数が自動で見える）に任せる ### 設定値のテンプレート参照は扱わない @@ -61,80 +77,122 @@ Pod 作成を「**最終的に `PodManifest` を1 つ構築する問題**」と ### プログラマティック Pod 作成 API -- `Pod::from_manifest(path)` の隣に、カスケード解決を経由する生成経路を追加する。イメージ: +factory は `PodManifest` を組み上げる builder として設計する。イメージ: - ```rust - // 層を明示的に指定して最終形を得る - let manifest = PodFactory::new() - .with_user_config(user_config_path)? // absent OK - .with_project_config(project_root)? // absent OK - .with_overlay(overlay_toml_or_struct) // programmatic - .resolve()?; // -> PodManifest - Pod::from_manifest(manifest, store).await?; - ``` +```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?; +``` -- 細かい形状は設計時に詰める（builder 型 vs 関数型、`overlay` を TOML 文字列か型付きか等）。 -- CLI からは `insomnia spawn --overlay '...'` 相当で同じ経路を叩く想定。 +`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 ` で代替する ## 要件 ### カスケード基盤 -- ユーザー設定・プロジェクト設定・プログラマティック overlay を順に重ねた結果が `PodManifest` として取れる。 +- ユーザー manifest・プロジェクト manifest・プログラマティック overlay を順に重ねた結果が `PodManifest` として取れる。 - 各層が部分形を許容する（`pod.pwd` だけ書いてあっても良い等）。 -- マージセマンティクスが**フィールドごとに定義**され、テストされる（スカラー / Option / マップ / リスト）。 -- 層が全て空でも**ビルトインデフォルト単体で有効な manifest にならない**（少なくとも `pod.pwd` と `provider` と `scope.allow` は上位層で与える必要がある）。その旨を resolve 時のエラーで明示する。 +- マージセマンティクスがフィールドごとに定義され、単体テストで担保される（スカラー / Option / マップ / scope union）。 +- 未知フィールドは warn のみ、型ミスマッチは hard error。 +- 層が全て空で resolve 失敗する場合は、必要なフィールド（`pod.pwd` / `provider` / `scope.allow` 等）を明示するエラーを返す。 ### プロンプト資産ライブラリ - 3層の検索パスでプロンプトファイルを解決できる。 - 同名プロンプトは上層優先で解決される。 - `SystemPromptTemplate` の minijinja `Environment` にカスタムローダを仕込み、`{% include "name" %}` / `{% import "name" as x %}` で資産を参照できる。 -- プロンプト資産自体もテンプレートとして評価され、現行の `SystemPromptContext`（`now` / `cwd` / `scope` / `tools` / `files` 等）と同じ変数が見える。 +- 親テンプレートの変数が include 先にも見える（minijinja デフォルト挙動）。 +- ビルトインプロンプトは `resources/prompts/` から `include_dir!` マクロで同梱され、追加・編集するときは該当ディレクトリにファイルを置く・編集するだけで済む。 +- プロンプト資産自体もテンプレートとして評価され、`SystemPromptContext` と同じ変数が見える。 ### プログラマティック Pod 作成 -- 既存の `Pod::from_manifest` を壊さず、追加経路として `PodFactory` 系の API を提供する。 -- TUI / GUI / daemon 等の上位クライアントが、TOML ファイルパスではなく**オーバーレイ + 設定ディレクトリパス**を渡すだけで 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/` に記載。 -## 設計で決めること +## リソース構成 -- **ユーザー設定のパス**: `~/.config/insomnia/config.toml` か、XDG 非準拠のパスも許容するか。環境変数で上書きできるか。 -- **プロジェクト設定の場所**: プロジェクトルートの `.insomnia/config.toml` か、別の命名か。サブディレクトリから起動したときの discovery（上位ディレクトリ探索）の挙動。 -- **プロジェクトルートの判定**: 明示指定 vs `.git` や `.insomnia/` で自動検出 -- **preset の概念を入れるか**: 名前付きの overlay セット（例: `insomnia spawn coder`）を導入するか。導入する場合、preset はユーザー設定内に `[preset.coder]` として持つか、個別ファイル `~/.config/insomnia/presets/coder.toml` として持つか -- **リストフィールドのマージ方針**: 置換 only にするか、append 用の別フィールド (`scope.allow_extra` 等) を用意するか -- **ビルトインプロンプトの初期ラインナップ**: どの役割をデフォルトで同梱するか、どこに置くか（`crates/pod/assets/prompts/*.md` を `include_str!` で埋め込む等） -- **プロンプト資産のファイル形式**: `.md` か `.txt` か、拡張子省略可能にするか、フロントマター（YAML/TOML）で引数デフォルトを持たせるか -- **プロンプト include 時の context 伝搬**: 親テンプレートの変数を include 先でも見えるようにするか、明示的に `with` で渡させるか -- **エラー戦略**: 上層で書かれた未知フィールドや型ミスマッチをどこまで寛容に扱うか -- **既存の `Pod::from_manifest(path)` とのインターフェース整理**: 廃止するか、内部的に PodFactory に委譲するか -- **CLI コマンド名**: `insomnia spawn` / `insomnia pod new` / その他 +``` +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` 等）は実装時に選定し、ドキュメントに列挙する ## 完了条件 -- `PodManifest` の最終形を層のマージで構築する `PodFactory`（または同等の仕組み）が実装され、マージセマンティクスの単体テストが通る。 -- ユーザー設定・プロジェクト設定・プログラマティック overlay のすべての層を使う end-to-end テストで、Pod が TOML ファイルパスを一切渡さずに起動できる。 -- プロンプト資産ライブラリを経由して system_prompt が組み立てられ、`{% include "ビルトイン名" %}` で同梱プロンプトを参照できることをテストで確認できる。 -- ユーザー設定ファイル / プロジェクト設定ファイルのドキュメントが `docs/` に存在する。 -- 既存の `Pod::from_manifest(path)` 経路が動き続ける（回帰させない）。 +- `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 が「preset 選択 + overlay 入力」に置き換わる想定。native-gui-mvp 実装時に本チケットの API を使うか、先に文字列パス渡しで済ませて後から差し替えるかは別途判断 +- `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 内での設定ファイル編集 UI**。編集は人間がエディタで TOML を書くだけ（あくまで「Pod 生成時に手書きしない」ことを目指す） -- **チーム共有・同期**。ユーザー設定とプロジェクト設定は各自・各リポジトリ単位で管理される +- **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 new file mode 100644 index 00000000..30093204 --- /dev/null +++ b/tickets/pod-factory.review.md @@ -0,0 +1,222 @@ +# レビュー: 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（テスト名）は解消済み。受け入れ可否には +変化なし（元々「無条件で受け入れ可」）。