diff --git a/TODO.md b/TODO.md index b1a949cc..ac8b6460 100644 --- a/TODO.md +++ b/TODO.md @@ -3,9 +3,10 @@ - [ ] Bash ツール (Permission 層と統合) → [tickets/bash-tool.md](tickets/bash-tool.md) - [ ] Compact の改善(要約品質 + 挙動詳細) → [tickets/compact-improvements.md](tickets/compact-improvements.md) - [ ] Protocol の設計 → [tickets/protocol-design.md](tickets/protocol-design.md) +- [ ] Manifest のパス解決: cwd ベース + manifest ファイル相対 → [tickets/manifest-path-resolution.md](tickets/manifest-path-resolution.md) - [ ] パーミッション: パターンベースのツール実行制御 → [tickets/permission-extension-point.md](tickets/permission-extension-point.md) - [ ] Pod オーケストレーション - - [ ] Pod 間コールバック通知 → [tickets/pod-callback.md](tickets/pod-callback.md) + - [ ] Pod の上流イベント報告 (子 → 親) → [tickets/pod-upstream-events.md](tickets/pod-upstream-events.md) - [ ] 動的 Scope 変更 → [tickets/dynamic-scope.md](tickets/dynamic-scope.md) - [ ] ネイティブ GUI クライアント MVP → [tickets/native-gui-mvp.md](tickets/native-gui-mvp.md) - [ ] TUI 拡充 diff --git a/crates/manifest/src/config.rs b/crates/manifest/src/config.rs index 246b0d5c..f6fddcf8 100644 --- a/crates/manifest/src/config.rs +++ b/crates/manifest/src/config.rs @@ -39,8 +39,6 @@ pub struct PodManifestConfig { pub struct PodMetaConfig { #[serde(default)] pub name: Option, - #[serde(default)] - pub pwd: Option, } #[derive(Debug, Clone, Default, Serialize, Deserialize)] @@ -137,6 +135,40 @@ impl PodManifestConfig { } } + /// Resolve every relative path inside this partial config against + /// `base` (assumed absolute). Paths that are already absolute are + /// left untouched. This is the only place per-layer path resolution + /// happens — cascade merge runs against fully absolute paths so + /// rules from different layers do not accidentally inherit another + /// layer's base. + /// + /// Affected fields: `provider.api_key_file`, + /// `scope.allow[].target`, `scope.deny[].target`, + /// `compaction.provider.api_key_file`. + pub fn resolve_paths(mut self, base: &Path) -> Self { + debug_assert!( + base.is_absolute(), + "resolve_paths base must be absolute: {}", + base.display() + ); + if let Some(ref mut p) = self.provider.api_key_file { + *p = join_if_relative(base, p); + } + for rule in &mut self.scope.allow { + rule.target = join_if_relative(base, &rule.target); + } + for rule in &mut self.scope.deny { + rule.target = join_if_relative(base, &rule.target); + } + if let Some(ref mut compaction) = self.compaction + && let Some(ref mut cp) = compaction.provider + && let Some(ref mut p) = cp.api_key_file + { + *p = join_if_relative(base, p); + } + self + } + /// 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 @@ -160,7 +192,6 @@ impl PodMetaConfig { fn merge(self, upper: Self) -> Self { Self { name: upper.name.or(self.name), - pwd: upper.pwd.or(self.pwd), } } } @@ -226,6 +257,18 @@ fn merge_option(lower: Option, upper: Option, merge: fn(T, T) -> T) -> } } +fn join_if_relative(base: &Path, p: &Path) -> PathBuf { + if p.is_absolute() { + p.to_path_buf() + } else { + base.join(p) + } +} + +/// Invariant check: every path in a fully-resolved [`PodManifestConfig`] +/// must be absolute. Relative paths are resolved per-layer via +/// [`PodManifestConfig::resolve_paths`]; if one reaches `TryFrom` it +/// indicates a caller skipped the per-layer resolve step. fn ensure_absolute(field: &'static str, path: &Path) -> Result<(), ResolveError> { if path.is_absolute() { Ok(()) @@ -264,8 +307,6 @@ impl TryFrom for PodManifest { .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, @@ -333,7 +374,7 @@ impl TryFrom for PodManifest { .transpose()?; Ok(PodManifest { - pod: PodMeta { name, pwd }, + pod: PodMeta { name }, provider, worker, scope: cfg.scope, @@ -355,7 +396,6 @@ mod tests { PodManifestConfig { pod: PodMetaConfig { name: Some("test".into()), - pwd: Some(abs("/pod")), }, provider: ProviderConfigPartial { kind: Some(ProviderKind::Anthropic), @@ -379,25 +419,54 @@ mod tests { 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() { + fn resolve_paths_joins_relative_api_key_file() { 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", .. } - )); + cfg.provider.api_key_file = Some(PathBuf::from("keys/anthropic")); + let resolved = cfg.resolve_paths(Path::new("/home/user/.config/insomnia")); + assert_eq!( + resolved.provider.api_key_file.as_deref(), + Some(Path::new("/home/user/.config/insomnia/keys/anthropic")) + ); } #[test] - fn resolve_rejects_relative_api_key_file() { + fn resolve_paths_leaves_absolute_paths_untouched() { let mut cfg = minimal_valid(); - cfg.provider.api_key_file = Some(PathBuf::from("~/.config/key")); + cfg.provider.api_key_file = Some(PathBuf::from("/etc/already/abs")); + let resolved = cfg.resolve_paths(Path::new("/home/user")); + assert_eq!( + resolved.provider.api_key_file.as_deref(), + Some(Path::new("/etc/already/abs")) + ); + } + + #[test] + fn resolve_paths_joins_relative_scope_targets() { + let mut cfg = minimal_valid(); + cfg.scope.allow[0].target = PathBuf::from("."); + cfg.scope.deny.push(ScopeRule { + target: PathBuf::from("secrets"), + permission: Permission::Write, + recursive: true, + }); + let resolved = cfg.resolve_paths(Path::new("/workspace/proj")); + assert_eq!(resolved.scope.allow[0].target, Path::new("/workspace/proj")); + assert_eq!( + resolved.scope.deny[0].target, + Path::new("/workspace/proj/secrets") + ); + } + + #[test] + fn try_from_invariant_rejects_lingering_relative_api_key_file() { + let mut cfg = minimal_valid(); + cfg.provider.api_key_file = Some(PathBuf::from("keys/relative")); + // Skipping resolve_paths on purpose: TryFrom must catch the + // invariant violation. let err = PodManifest::try_from(cfg).unwrap_err(); assert!(matches!( err, @@ -409,9 +478,9 @@ mod tests { } #[test] - fn resolve_rejects_relative_scope_target() { + fn try_from_invariant_rejects_lingering_relative_scope_target() { let mut cfg = minimal_valid(); - cfg.scope.allow[0].target = PathBuf::from("./docs"); + cfg.scope.allow[0].target = PathBuf::from("docs"); let err = PodManifest::try_from(cfg).unwrap_err(); assert!(matches!( err, @@ -443,21 +512,23 @@ mod tests { let lower = PodManifestConfig { pod: PodMetaConfig { name: Some("lower".into()), - pwd: Some(abs("/lower")), + }, + provider: ProviderConfigPartial { + model: Some("lower-model".into()), + ..Default::default() }, ..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"))); + // model not present in upper — retain lower + assert_eq!(merged.provider.model.as_deref(), Some("lower-model")); } #[test] @@ -556,7 +627,6 @@ mod tests { let bad = r#" [pod] name = "x" -pwd = "/abs" [worker] max_tokens = "not-a-number" @@ -567,10 +637,13 @@ max_tokens = "not-a-number" #[test] fn from_toml_accepts_unknown_field() { // Unknown keys are warn-and-ignored, not hard errors. + // `pod.pwd` specifically is silently dropped after the + // path-resolution ticket — keep it in the fixture to exercise + // that code path. let ok = r#" [pod] name = "x" -pwd = "/abs" +pwd = "/obsolete" [worker] max_tokens = 1000 @@ -610,7 +683,6 @@ permission = "write" let overlay = PodManifestConfig { pod: PodMetaConfig { name: Some("x".into()), - pwd: Some(abs("/pod")), }, provider: ProviderConfigPartial { kind: Some(ProviderKind::Anthropic), @@ -658,7 +730,6 @@ permission = "write" r#" [pod] name = "dbg" -pwd = "/abs/project" "#, ) .unwrap(); @@ -666,7 +737,6 @@ pwd = "/abs/project" 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/lib.rs b/crates/manifest/src/lib.rs index 8595b1e4..f839de24 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -17,7 +17,9 @@ use serde::{Deserialize, Serialize}; /// Declarative configuration for a Pod. /// /// Parsed from a TOML manifest file. Describes the provider, model, -/// system prompt, and directory scope (required). +/// system prompt, and directory scope (required). The Pod's working +/// directory is **not** part of the manifest — it is the process's +/// `std::env::current_dir()` at construction time. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PodManifest { pub pod: PodMeta, @@ -32,9 +34,6 @@ pub struct PodManifest { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PodMeta { pub name: String, - /// Working directory for the Pod. Relative paths are resolved against - /// the directory containing the manifest file. - pub pwd: PathBuf, } /// LLM provider configuration. @@ -163,8 +162,9 @@ pub struct ScopeConfig { /// A single allow or deny rule inside [`ScopeConfig`]. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ScopeRule { - /// Target path. Relative paths are resolved against the Pod's pwd - /// when [`Scope::from_config`] runs. + /// Target path. Must be absolute by the time [`Scope::from_config`] + /// runs — relative paths are resolved per-layer against the manifest + /// file's directory (cwd for overlay layers) before cascade merge. pub target: PathBuf, /// Permission level this rule grants (allow) or caps strictly below /// (deny). @@ -256,7 +256,6 @@ mod tests { const MINIMAL_REQUIRED: &str = r#" [pod] name = "test-agent" -pwd = "./" [provider] kind = "anthropic" @@ -265,7 +264,7 @@ model = "claude-sonnet-4-20250514" [worker] [[scope.allow]] -target = "./" +target = "/abs/scope" permission = "write" "#; @@ -273,7 +272,6 @@ permission = "write" fn parse_minimal_manifest() { let manifest = PodManifest::from_toml(MINIMAL_REQUIRED).unwrap(); assert_eq!(manifest.pod.name, "test-agent"); - assert_eq!(manifest.pod.pwd, PathBuf::from("./")); assert_eq!(manifest.provider.kind, ProviderKind::Anthropic); assert_eq!(manifest.provider.model, "claude-sonnet-4-20250514"); assert!(manifest.provider.api_key_file.is_none()); @@ -287,12 +285,11 @@ permission = "write" let toml = r#" [pod] name = "code-reviewer" -pwd = "./src" [provider] kind = "anthropic" model = "claude-sonnet-4-20250514" -api_key_file = "~/.config/insomnia/keys/anthropic" +api_key_file = "/abs/keys/anthropic" [worker] instruction = "$user/reviewer" @@ -300,24 +297,23 @@ max_tokens = 4096 temperature = 0.3 [[scope.allow]] -target = "./" +target = "/abs/project" permission = "write" [[scope.allow]] -target = "../docs" +target = "/abs/docs" permission = "read" recursive = false [[scope.deny]] -target = "./secrets.rs" +target = "/abs/project/secrets.rs" permission = "write" "#; let manifest = PodManifest::from_toml(toml).unwrap(); assert_eq!(manifest.pod.name, "code-reviewer"); - assert_eq!(manifest.pod.pwd, PathBuf::from("./src")); assert_eq!( manifest.provider.api_key_file.as_deref(), - Some(std::path::Path::new("~/.config/insomnia/keys/anthropic")) + Some(std::path::Path::new("/abs/keys/anthropic")) ); assert_eq!(manifest.worker.instruction, "$user/reviewer"); assert_eq!(manifest.worker.max_tokens, Some(4096)); @@ -337,7 +333,6 @@ permission = "write" let toml = r#" [pod] name = "missing-scope" -pwd = "./" [provider] kind = "anthropic" @@ -348,25 +343,6 @@ model = "claude-sonnet-4-20250514" assert!(PodManifest::from_toml(toml).is_err()); } - #[test] - fn reject_missing_pwd() { - let toml = r#" -[pod] -name = "missing-pwd" - -[provider] -kind = "anthropic" -model = "claude-sonnet-4-20250514" - -[worker] - -[[scope.allow]] -target = "./" -permission = "write" -"#; - assert!(PodManifest::from_toml(toml).is_err()); - } - #[test] fn parse_max_turns() { let toml = MINIMAL_REQUIRED.replace("[worker]\n", "[worker]\nmax_turns = 50\n"); diff --git a/crates/manifest/src/scope.rs b/crates/manifest/src/scope.rs index 8a64e364..0532dc87 100644 --- a/crates/manifest/src/scope.rs +++ b/crates/manifest/src/scope.rs @@ -1,9 +1,10 @@ //! Runtime representation of a Pod's access scope. //! -//! Built from [`crate::ScopeConfig`] via [`Scope::from_config`] once the -//! Pod's pwd (working directory) has been resolved to an absolute path. -//! All rule `target` paths inside the [`Scope`] are absolute and lexically -//! stable, so access checks are pure path comparisons. +//! Built from [`crate::ScopeConfig`] via [`Scope::from_config`]. Every +//! rule `target` must already be an absolute path — per-layer path +//! resolution runs earlier, inside [`crate::PodManifestConfig::resolve_paths`]. +//! All rule `target` paths inside the [`Scope`] are canonicalised (where +//! possible) so access checks are pure path comparisons. use std::ffi::OsString; use std::path::{Path, PathBuf}; @@ -33,8 +34,8 @@ struct ResolvedRule { pub enum ScopeError { #[error("scope must declare at least one [[scope.allow]] rule")] EmptyAllow, - #[error("scope base path must be absolute: {}", .0.display())] - BaseNotAbsolute(PathBuf), + #[error("scope target must be absolute: {}", .0.display())] + RelativeTarget(PathBuf), #[error("failed to resolve scope target {}: {source}", .path.display())] ResolveTarget { path: PathBuf, @@ -44,25 +45,26 @@ pub enum ScopeError { } impl Scope { - /// Build a [`Scope`] from a declarative [`ScopeConfig`], resolving - /// relative `target` paths against `base` (conventionally the Pod's - /// absolute pwd). - pub fn from_config(config: &ScopeConfig, base: &Path) -> Result { - if !base.is_absolute() { - return Err(ScopeError::BaseNotAbsolute(base.to_path_buf())); - } + /// Build a [`Scope`] from a declarative [`ScopeConfig`]. + /// + /// Every `target` in `config` must already be absolute — per-layer + /// resolution happens upstream in + /// [`crate::PodManifestConfig::resolve_paths`] so that cascade merge + /// operates on fully-qualified paths. A lingering relative target + /// here signals an upstream bug and is rejected. + pub fn from_config(config: &ScopeConfig) -> Result { if config.allow.is_empty() { return Err(ScopeError::EmptyAllow); } let allow = config .allow .iter() - .map(|r| resolve_rule(r, base)) + .map(resolve_rule) .collect::, _>>()?; let deny = config .deny .iter() - .map(|r| resolve_rule(r, base)) + .map(resolve_rule) .collect::, _>>()?; Ok(Self { allow, deny }) } @@ -221,13 +223,11 @@ impl ResolvedRule { } } -fn resolve_rule(rule: &ScopeRule, base: &Path) -> Result { - let joined = if rule.target.is_absolute() { - rule.target.clone() - } else { - base.join(&rule.target) - }; - let target = resolve_path(&joined).ok_or_else(|| ScopeError::ResolveTarget { +fn resolve_rule(rule: &ScopeRule) -> Result { + if !rule.target.is_absolute() { + return Err(ScopeError::RelativeTarget(rule.target.clone())); + } + let target = resolve_path(&rule.target).ok_or_else(|| ScopeError::ResolveTarget { path: rule.target.clone(), source: std::io::Error::new(std::io::ErrorKind::Other, "could not absolutize target"), })?; @@ -307,7 +307,7 @@ mod tests { allow: vec![allow_rule(dir.path(), Permission::Write)], deny: Vec::new(), }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let f = dir.path().join("a.txt"); assert_eq!(scope.permission_at(&f), Some(Permission::Write)); } @@ -319,7 +319,7 @@ mod tests { allow: vec![allow_rule(dir.path(), Permission::Read)], deny: Vec::new(), }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let f = dir.path().join("a.txt"); assert_eq!(scope.permission_at(&f), Some(Permission::Read)); assert!(scope.is_readable(&f)); @@ -335,7 +335,7 @@ mod tests { allow: vec![allow_rule(dir.path(), Permission::Write)], deny: vec![allow_rule(&sub, Permission::Write)], }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let f = sub.join("a.txt"); assert_eq!(scope.permission_at(&f), Some(Permission::Read)); // outside the deny, still writable. @@ -354,7 +354,7 @@ mod tests { allow: vec![allow_rule(dir.path(), Permission::Write)], deny: vec![allow_rule(&secret, Permission::Read)], }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); assert_eq!(scope.permission_at(&secret), None); } @@ -370,7 +370,7 @@ mod tests { ], deny: Vec::new(), }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); assert_eq!( scope.permission_at(&dir.path().join("a.txt")), Some(Permission::Read) @@ -394,22 +394,35 @@ mod tests { }], deny: Vec::new(), }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); assert!(scope.is_writable(&dir.path().join("top.txt"))); assert!(!scope.is_writable(&nested.join("deep.txt"))); } #[test] fn empty_allow_rejected() { - let dir = TempDir::new().unwrap(); let cfg = ScopeConfig { allow: Vec::new(), deny: Vec::new(), }; - let err = Scope::from_config(&cfg, dir.path()).unwrap_err(); + let err = Scope::from_config(&cfg).unwrap_err(); assert!(matches!(err, ScopeError::EmptyAllow)); } + #[test] + fn relative_target_rejected_as_invariant_violation() { + let cfg = ScopeConfig { + allow: vec![ScopeRule { + target: PathBuf::from("relative/path"), + permission: Permission::Read, + recursive: true, + }], + deny: Vec::new(), + }; + let err = Scope::from_config(&cfg).unwrap_err(); + assert!(matches!(err, ScopeError::RelativeTarget(_))); + } + #[test] fn rejects_traversal_attack() { let dir = TempDir::new().unwrap(); @@ -430,7 +443,7 @@ mod tests { ], deny: Vec::new(), }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let summary = scope.summary(); assert!(summary.contains("Readable:")); assert!(summary.contains("Writable:")); @@ -448,7 +461,7 @@ mod tests { allow: vec![allow_rule(dir.path(), Permission::Write)], deny: vec![allow_rule(&secret, Permission::Read)], }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let summary = scope.summary(); assert!(!summary.contains("secret")); } @@ -473,7 +486,7 @@ mod tests { ], deny: Vec::new(), }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let summary = scope.summary(); let docs_canon = docs.canonicalize().unwrap().display().to_string(); let dir_canon = dir.path().canonicalize().unwrap().display().to_string(); @@ -500,7 +513,7 @@ mod tests { ], deny: Vec::new(), }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let readable: Vec<_> = scope.readable_paths().collect(); let writable: Vec<_> = scope.writable_paths().collect(); assert_eq!(readable.len(), 2); diff --git a/crates/pod/src/factory.rs b/crates/pod/src/factory.rs index 46fbf12d..d91decdd 100644 --- a/crates/pod/src/factory.rs +++ b/crates/pod/src/factory.rs @@ -11,6 +11,18 @@ //! 4. **Programmatic overlay** — inline TOML string or typed //! [`PodManifestConfig`] supplied by the caller (CLI flags, GUI, //! spawning Pod, etc.). Highest priority. +//! +//! Path resolution happens **before** merge. Each layer is resolved +//! against its own base directory so that a relative `target = "."` +//! in the project manifest means "project directory" regardless of +//! how the user or overlay layers lay out their own paths: +//! +//! - user manifest: base = the directory holding the manifest file +//! - project manifest: base = the directory holding the manifest file +//! (i.e. `/.insomnia/`) +//! - overlay: base = the process's `current_dir()` at the time the +//! overlay is installed, since an inline TOML string has no file +//! location of its own use std::path::{Path, PathBuf}; @@ -49,8 +61,15 @@ pub enum FactoryError { /// `with_*` method twice overwrites the previous value for that slot. #[derive(Debug, Default)] pub struct PodFactory { - user: Option, - project: Option, + /// User layer paired with the directory the manifest lives in + /// (base for resolving its relative paths). + user: Option<(PodManifestConfig, PathBuf)>, + /// Project layer paired with the directory the manifest lives in. + project: Option<(PodManifestConfig, PathBuf)>, + /// Programmatic overlays are resolved against the process's + /// `current_dir()` at the time each call arrives, then merged into + /// this slot. Storing a pre-resolved (absolute-paths) config means + /// later overlay calls from a different cwd still work correctly. overlay: Option, /// Directory holding the user prompts library — co-located with /// the user manifest when loaded. `/prompts/`. @@ -74,8 +93,9 @@ impl PodFactory { 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")); + let base = manifest_base(&path)?; + self.user = Some((read_config_file(&path)?, base.clone())); + self.user_prompts_dir = Some(base.join("prompts")); } Ok(self) } @@ -84,8 +104,9 @@ impl PodFactory { /// 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")); + let base = manifest_base(path)?; + self.user = Some((read_config_file(path)?, base.clone())); + self.user_prompts_dir = Some(base.join("prompts")); Ok(self) } @@ -98,8 +119,9 @@ impl PodFactory { 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")); + let base = manifest_base(&path)?; + self.project = Some((read_config_file(&path)?, base.clone())); + self.project_prompts_dir = Some(base.join("prompts")); } Ok(self) } @@ -111,31 +133,28 @@ impl PodFactory { 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")); + let base = manifest_base(&path)?; + self.project = Some((read_config_file(&path)?, base.clone())); + self.project_prompts_dir = Some(base.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. + /// Install a programmatic overlay parsed from a TOML string. Any + /// relative paths in the overlay are resolved against the process's + /// current working directory at the time of this call — an inline + /// TOML string has no file location of its own. 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, - }); + self.overlay = Some(resolve_and_merge_overlay(self.overlay, 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 + /// Behaves like [`Self::with_overlay_toml`] regarding relative paths. + pub fn with_overlay_config(mut self, config: PodManifestConfig) -> Result { + self.overlay = Some(resolve_and_merge_overlay(self.overlay, config)?); + Ok(self) } /// Build a [`PromptLoader`] that reflects the user / project @@ -162,6 +181,11 @@ impl PodFactory { /// feeds `{% include "name" %}` references in the Pod's system /// prompt template. /// + /// Each layer is resolved to absolute paths against its own base + /// (see module docs) **before** merge, so scope rules and + /// `api_key_file` paths from different layers do not accidentally + /// inherit another layer's base. + /// /// The base layer is [`PodManifestConfig::builtin_defaults`] so /// every per-field default flows through a single source of truth /// (see [`manifest::defaults`]). @@ -169,11 +193,11 @@ impl PodFactory { let loader = self.build_prompt_loader(); let merged = PodManifestConfig::builtin_defaults(); let merged = match self.user { - Some(user) => merged.merge(user), + Some((user, base)) => merged.merge(user.resolve_paths(&base)), None => merged, }; let merged = match self.project { - Some(project) => merged.merge(project), + Some((project, base)) => merged.merge(project.resolve_paths(&base)), None => merged, }; let merged = match self.overlay { @@ -185,6 +209,43 @@ impl PodFactory { } } +fn manifest_base(path: &Path) -> Result { + let parent = path.parent().ok_or_else(|| FactoryError::Io { + path: path.to_path_buf(), + source: std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "manifest path has no parent directory", + ), + })?; + // Absolutise against cwd so later path joins produce absolute + // results regardless of whether the caller passed a relative + // manifest path. + if parent.is_absolute() { + Ok(parent.to_path_buf()) + } else { + let cwd = std::env::current_dir().map_err(|source| FactoryError::Io { + path: PathBuf::from("."), + source, + })?; + Ok(cwd.join(parent)) + } +} + +fn resolve_and_merge_overlay( + existing: Option, + incoming: PodManifestConfig, +) -> Result { + let cwd = std::env::current_dir().map_err(|source| FactoryError::Io { + path: PathBuf::from("."), + source, + })?; + let resolved = incoming.resolve_paths(&cwd); + Ok(match existing { + Some(prev) => prev.merge(resolved), + None => resolved, + }) +} + fn user_manifest_path() -> Result { if let Ok(dir) = std::env::var("XDG_CONFIG_HOME") { if !dir.is_empty() { @@ -242,7 +303,6 @@ mod tests { r#" [pod] name = "solo" -pwd = "{pwd}" [provider] kind = "anthropic" @@ -261,7 +321,6 @@ permission = "write" .unwrap(); let manifest = manifest.0; assert_eq!(manifest.pod.name, "solo"); - assert_eq!(manifest.pod.pwd, pwd); } #[test] @@ -293,20 +352,21 @@ permission = "write" pwd = pwd.display() )) .unwrap(); - let overlay_cfg = PodManifestConfig::from_toml(&format!( + let overlay_cfg = PodManifestConfig::from_toml( r#" [pod] name = "overlay-name" -pwd = "{pwd}" "#, - pwd = pwd.display() - )) + ) .unwrap(); let (manifest, _loader) = PodFactory::new() .with_overlay_config(user_cfg) + .unwrap() .with_overlay_config(project_cfg) + .unwrap() .with_overlay_config(overlay_cfg) + .unwrap() .resolve() .unwrap(); @@ -332,7 +392,6 @@ pwd = "{pwd}" r#" [pod] name = "from-user" -pwd = "{pwd}" [provider] kind = "anthropic" @@ -381,7 +440,6 @@ model = "project-model" r#" [pod] name = "walked-up" -pwd = "{root}" [provider] kind = "anthropic" @@ -415,7 +473,6 @@ permission = "write" r#" [pod] name = "standalone" -pwd = "{pwd}" [provider] kind = "anthropic" @@ -441,6 +498,77 @@ permission = "write" assert_eq!(manifest.pod.name, "standalone"); } + #[test] + fn user_manifest_relative_paths_resolve_against_its_directory() { + // user manifest at /cfg/manifest.toml with a relative + // scope target `./workspace` must resolve to /cfg/workspace. + let tmp = TempDir::new().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let cfg_dir = root.join("cfg"); + std::fs::create_dir_all(&cfg_dir).unwrap(); + let workspace = cfg_dir.join("workspace"); + std::fs::create_dir_all(&workspace).unwrap(); + + let user = cfg_dir.join("manifest.toml"); + write( + &user, + r#" +[pod] +name = "rel-user" + +[provider] +kind = "anthropic" +model = "m" + +[[scope.allow]] +target = "./workspace" +permission = "write" +"#, + ); + + let (manifest, _loader) = PodFactory::new() + .with_user_manifest(&user) + .unwrap() + .resolve() + .unwrap(); + assert_eq!(manifest.scope.allow[0].target, workspace); + } + + #[test] + fn project_manifest_relative_paths_resolve_against_insomnia_dir() { + // per ticket: base is the directory holding the manifest — + // `.insomnia/` for a project manifest. `target = "."` inside + // a project manifest therefore points at `.insomnia/`, not at + // the project root. + let tmp = TempDir::new().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let insomnia_dir = root.join(".insomnia"); + std::fs::create_dir_all(&insomnia_dir).unwrap(); + let project_manifest = insomnia_dir.join("manifest.toml"); + write( + &project_manifest, + r#" +[pod] +name = "rel-project" + +[provider] +kind = "anthropic" +model = "m" + +[[scope.allow]] +target = "." +permission = "read" +"#, + ); + + let (manifest, _loader) = PodFactory::new() + .with_project_manifest_from(&root) + .unwrap() + .resolve() + .unwrap(); + assert_eq!(manifest.scope.allow[0].target, insomnia_dir); + } + #[test] fn resolve_produces_loader_with_workspace_prompts_dir() { use crate::system_prompt::{SystemPromptContext, SystemPromptTemplate}; @@ -456,7 +584,6 @@ permission = "write" r#" [pod] name = "factory-pod" -pwd = "{root}" [provider] kind = "anthropic" @@ -493,7 +620,7 @@ permission = "write" }], deny: Vec::new(), }; - let scope = Scope::from_config(&scope_cfg, &root).unwrap(); + let scope = Scope::from_config(&scope_cfg).unwrap(); let ctx = SystemPromptContext { now: chrono::Utc::now(), cwd: &root, @@ -512,12 +639,9 @@ permission = "write" fn resolve_fails_on_missing_required_field() { let tmp = TempDir::new().unwrap(); let pwd = tmp.path().canonicalize().unwrap(); - // pwd set but pod.name missing + // pod.name missing — resolver must reject. let overlay = format!( r#" -[pod] -pwd = "{pwd}" - [provider] kind = "anthropic" model = "m" diff --git a/crates/pod/src/main.rs b/crates/pod/src/main.rs index 4b71737a..22428c03 100644 --- a/crates/pod/src/main.rs +++ b/crates/pod/src/main.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use std::process::ExitCode; use clap::Parser; -use pod::{Pod, PodController, PodFactory, PodManifestConfig, PodMetaConfig}; +use pod::{Pod, PodController, PodFactory}; use session_store::FsStore; #[derive(Parser)] @@ -27,11 +27,6 @@ struct Cli { #[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)] @@ -68,20 +63,6 @@ fn default_runtime_dir() -> Result { } } -/// Construct a programmatic overlay [`PodManifestConfig`] that carries -/// `pod.pwd` derived from the `--pwd` shorthand. Relative CLI paths -/// are canonicalized here so the cascade always sees an absolute path. -fn pwd_overlay(pwd: &PathBuf) -> PodManifestConfig { - let absolute = std::fs::canonicalize(pwd).unwrap_or_else(|_| pwd.clone()); - PodManifestConfig { - pod: PodMetaConfig { - pwd: Some(absolute), - ..Default::default() - }, - ..Default::default() - } -} - async fn build_factory(cli: &Cli) -> Result { let mut factory = PodFactory::new(); @@ -103,13 +84,6 @@ async fn build_factory(cli: &Cli) -> Result { .map_err(|e| format!("failed to auto-load project manifest: {e}"))?, }; - // `--pwd` goes in as a typed config so path strings never have to - // pass through TOML escaping. `--overlay` keeps its inline-TOML - // interface (that is its entire reason for existing). Both feed - // the same overlay slot and merge in call order. - if let Some(pwd) = cli.pwd.as_ref() { - factory = factory.with_overlay_config(pwd_overlay(pwd)); - } if let Some(overlay) = cli.overlay.as_deref() { factory = factory .with_overlay_toml(overlay) diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index cac8eab1..f8eb284e 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -874,11 +874,12 @@ impl Pod { impl Pod, St> { /// Create a Pod entirely from a validated manifest. /// - /// `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. + /// The Pod's working directory is captured once here from the + /// process's `std::env::current_dir()` — callers that want a + /// different cwd must `cd` before constructing the Pod (e.g. the + /// `SpawnPod` tool sets `Command::current_dir` on the child). The + /// captured pwd is canonicalised and validated against + /// `manifest.scope`. /// /// `loader` is installed into the system-prompt template /// environment so that `{% include "name" %}` / @@ -889,8 +890,8 @@ impl Pod, St> { store: St, loader: PromptLoader, ) -> Result { - let pwd = resolve_pwd(&manifest.pod.pwd)?; - let scope = Scope::from_config(&manifest.scope, &pwd).map_err(PodError::Scope)?; + let pwd = current_pwd()?; + let scope = Scope::from_config(&manifest.scope).map_err(PodError::Scope)?; if !scope.is_readable(&pwd) { return Err(PodError::PwdOutsideScope { pwd }); } @@ -967,8 +968,8 @@ impl Pod, St> { loader: PromptLoader, callback_socket: PathBuf, ) -> Result { - let pwd = resolve_pwd(&manifest.pod.pwd)?; - let scope = Scope::from_config(&manifest.scope, &pwd).map_err(PodError::Scope)?; + let pwd = current_pwd()?; + let scope = Scope::from_config(&manifest.scope).map_err(PodError::Scope)?; if !scope.is_readable(&pwd) { return Err(PodError::PwdOutsideScope { pwd }); } @@ -1127,9 +1128,6 @@ 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), @@ -1158,16 +1156,17 @@ pub enum PodError { ScopeLock(#[from] ScopeLockError), } -/// 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(), +/// Snapshot the process's current working directory as the Pod's pwd, +/// canonicalising symlinks and any `.`/`..` components. The Pod keeps +/// this value for its lifetime; changes to the process-wide cwd after +/// construction do not affect scope checks or the system prompt. +fn current_pwd() -> Result { + let cwd = std::env::current_dir().map_err(|source| PodError::InvalidPwd { + pwd: PathBuf::from("."), + source, + })?; + cwd.canonicalize().map_err(|source| PodError::InvalidPwd { + pwd: cwd, source, }) } diff --git a/crates/pod/src/spawn_pod.rs b/crates/pod/src/spawn_pod.rs index b60d8482..819e01da 100644 --- a/crates/pod/src/spawn_pod.rs +++ b/crates/pod/src/spawn_pod.rs @@ -174,16 +174,15 @@ impl Tool for SpawnPodTool { // it back — even if later steps (Method::Run delivery, record // write) fail, the child is running and will release its own // entry on exit. - let overlay_toml = - match build_overlay_toml(&input.name, &self.spawner_pwd, &instruction, &scope_allow) { - Ok(s) => s, - Err(e) => { - self.release_reservation(&lock_path, &input.name); - return Err(ToolError::ExecutionFailed(format!( - "overlay serialisation: {e}" - ))); - } - }; + let overlay_toml = match build_overlay_toml(&input.name, &instruction, &scope_allow) { + Ok(s) => s, + Err(e) => { + self.release_reservation(&lock_path, &input.name); + return Err(ToolError::ExecutionFailed(format!( + "overlay serialisation: {e}" + ))); + } + }; let start_outcome = self.exec_child(&overlay_toml, &predicted_socket).await; if let Err(e) = start_outcome { @@ -231,6 +230,7 @@ impl SpawnPodTool { .arg(&self.callback_socket) .arg("--overlay") .arg(overlay_toml) + .current_dir(&self.spawner_pwd) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -281,16 +281,18 @@ fn parse_scope(rules: &[ScopeRuleInput]) -> Result, ToolError> { /// Serialise the overlay TOML that gets handed to the child `pod` /// binary via `--overlay`. `PodManifestConfig`'s `Serialize` impl is /// the single source of truth for the on-disk manifest format. +/// +/// The child's working directory is set separately via +/// `Command::current_dir` (see [`SpawnPodTool::exec_child`]) — it is +/// not part of the manifest. fn build_overlay_toml( name: &str, - pwd: &Path, instruction: &str, scope_allow: &[ScopeRule], ) -> Result { let overlay = PodManifestConfig { pod: PodMetaConfig { name: Some(name.to_string()), - pwd: Some(pwd.to_path_buf()), }, worker: WorkerManifestConfig { instruction: Some(instruction.to_string()), diff --git a/crates/pod/src/system_prompt.rs b/crates/pod/src/system_prompt.rs index 63c625b8..8f65e129 100644 --- a/crates/pod/src/system_prompt.rs +++ b/crates/pod/src/system_prompt.rs @@ -229,7 +229,7 @@ mod tests { }], deny: Vec::new(), }; - Scope::from_config(&cfg, dir).unwrap() + Scope::from_config(&cfg).unwrap() } fn ctx<'a>( diff --git a/crates/tools/src/glob.rs b/crates/tools/src/glob.rs index edda5f92..514a45f6 100644 --- a/crates/tools/src/glob.rs +++ b/crates/tools/src/glob.rs @@ -267,7 +267,7 @@ mod tests { recursive: true, }], }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let fs = ScopedFs::new(scope, dir.path().to_path_buf()); let def = glob_tool(fs); diff --git a/crates/tools/src/grep.rs b/crates/tools/src/grep.rs index 9eda3824..65556691 100644 --- a/crates/tools/src/grep.rs +++ b/crates/tools/src/grep.rs @@ -513,7 +513,7 @@ mod tests { recursive: true, }], }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let scoped = ScopedFs::new(scope, dir.path().to_path_buf()); let def = grep_tool(scoped); diff --git a/crates/tools/src/scoped_fs.rs b/crates/tools/src/scoped_fs.rs index cce29d9a..76db5bfe 100644 --- a/crates/tools/src/scoped_fs.rs +++ b/crates/tools/src/scoped_fs.rs @@ -266,7 +266,7 @@ mod tests { recursive: true, }], }; - let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let scope = Scope::from_config(&cfg).unwrap(); let scoped = ScopedFs::new(scope, dir.path().to_path_buf()); let err = scoped.write(&sub.join("locked.txt"), b"x").unwrap_err(); assert!( diff --git a/tickets/manifest-path-resolution.md b/tickets/manifest-path-resolution.md new file mode 100644 index 00000000..6f709f2b --- /dev/null +++ b/tickets/manifest-path-resolution.md @@ -0,0 +1,84 @@ +# Manifest のパス解決: cwd ベース + manifest ファイル相対 + +## 背景 + +現状 manifest 内のパス(`pod.pwd` / `provider.api_key_file` / `scope.allow.target` / `scope.deny.target` / `compaction.provider.api_key_file`)は全て絶対必須で、相対パスは `ResolveError::RelativePath` で弾かれる。 + +これは 4 層(builtin / user / project / overlay)のカスケードで「相対の基準点が層ごとに違う」曖昧さを避けるための制約だったが、次の点で歪みを生んでいる: + +- `pod.pwd` フィールドが Unix 慣習(cwd はプロセス状態、config には書かない)と乖離 +- project manifest で `scope.allow = [{ target = "" }]` のような絶対パスを強いられ、プロジェクトをどこに置いても動くはずの設定が壊れる +- user manifest で `api_key_file = "~/.config/insomnia/keys/anthropic"` が書けない(`~` 展開もしていない) + +cargo / pyproject / npm などに倣い「相対パスは manifest ファイルの位置基準」に切り替える。合わせて `pod.pwd` を廃止し、プロセスの cwd を使う。 + +## 新しい解決規則 + +- `pod.pwd` フィールドは削除。Pod の作業ディレクトリ = プロセスの cwd +- 相対パスは **manifest ファイルがあるディレクトリ** を基準に解決 + - user manifest (`~/.config/insomnia/manifest.toml`) の相対 = そのディレクトリ基準 + - project manifest (`/.insomnia/manifest.toml`) の相対 = そのディレクトリ基準 +- overlay(インライン TOML、ファイル位置なし)の相対パスは **プロセスの cwd** 基準 +- builtin 層には manifest を埋め込んでいないので対象外 + +## 解決のタイミング + +各層を**マージする前に絶対化**する。層をまたいだ相対パス合成は行わない。 + +``` +user.toml (partial) → resolve_paths(base=user_dir) → absolute partial + │ +project.toml (partial) → resolve_paths(base=prj_dir) ─┤ + │── merge → PodManifestConfig +overlay (partial) → resolve_paths(base=cwd) ──────────┤ + │ +builtin defaults → ────────────────────────────────── ┘ +``` + +`TryFrom` の時点では全パスが絶対になっているので、`ensure_absolute` は不変条件のチェックとしてのみ残す。 + +## 影響範囲 + +### `crates/manifest` + +- `PodManifestConfig` から `pod.pwd` を削除 +- 各 partial config を「ベースパス付き」で解決するヘルパーを追加(関数シグネチャ案: `fn resolve_partial(cfg: PodManifestConfigPartial, base: &Path) -> PodManifestConfigPartial`) +- 対象フィールド: `provider.api_key_file` / `scope.allow.target` / `scope.deny.target` / `compaction.provider.api_key_file` + +### `crates/pod/src/factory.rs` + +- `with_user_manifest` / `with_project_manifest_from` は渡された manifest ファイルの親ディレクトリを base として保存、解決時に使う +- `with_overlay_toml` / `with_overlay_config` はプロセス cwd を base として使う +- マージ順は現状のまま(overlay が最優先) + +### `crates/pod/src/pod.rs` 他 + +- `pod.pwd` を参照している箇所を `std::env::current_dir()` に置き換え +- `Pod::from_manifest` / `from_manifest_spawned` のシグネチャから pwd 関連を削除 + +### `crates/pod/src/spawn_pod.rs` + +- overlay TOML 構築から `pod.pwd` を消す +- 子プロセス起動時に `Command::current_dir(spawner_pwd)` で cwd を明示 + (現状の「spawner の pwd を子に引き継ぐ」挙動を維持するため) +- 将来、LLM が子の cwd を明示的に指定したくなったら `SpawnPod` の入力に `cwd` を足す(本チケット範囲外) + +### `crates/pod/src/main.rs` + +- `--pwd` フラグは削除(cwd が代替) +- 起動スクリプトや TUI 側で `cd` してから `pod` を起動する運用に変更 + +## 完了条件 + +- `pod.pwd` フィールドの削除 +- 各層のパスが manifest ファイル基準(overlay は cwd 基準)で解決される +- マージは絶対化後の値で行う +- 既存テストが通る / 相対パスを使った manifest で起動可能 +- `api_key_file = "keys/anthropic"` が user manifest 内で動作 +- project manifest で `scope.allow = [{ target = "." }]` が動作 + +## 範囲外 + +- `~` 展開(`dirs::home_dir()` ベースの展開は別途。まずは `./keys/anthropic` のような単純相対のみ) +- `SpawnPod` の入力に子 cwd の指定を追加(cwd 明示は別チケット) +- `pod` CLI の `--pwd` 廃止後の移行期間対応(一発破壊的変更で行く) diff --git a/tickets/pod-callback.md b/tickets/pod-callback.md deleted file mode 100644 index a5d3726e..00000000 --- a/tickets/pod-callback.md +++ /dev/null @@ -1,80 +0,0 @@ -# Pod 間コールバック通知 - -## 背景 - -spawned Pod がイベント(ターン完了、エラー、終了、scope 又貸し)を発生させたとき、spawner に自動通知する仕組みが必要。`Method::Notify`(実装済み)が受け口となり、spawned Pod がそこにコールバックを送る。 - -## 依存 - -- `tickets/spawn-pod-tool.md`: spawn 記録と callback address の受け渡し -- `Method::Notify`: 実装済み。コールバック受信 → LLM への system message 注入 - -## 仕組み - -### callback address - -spawn 時に spawner が自身の callback address を spawned Pod に渡す: -- ローカル: spawner の unix socket path -- リモート: `insomnia@host:pod-name` 形式(将来。`docs/network-peering.md` 参照) - -spawned Pod はこの address を保持し、イベント発生時に一発接続して通知を送り、即切断する(webhook モデル)。 - -### 通知の種類 - -| 通知 | タイミング | 含まれるデータ | -|---|---|---| -| ターン完了 | spawned Pod の1ターンが終了 | name | -| エラー | spawned Pod でエラー発生 | name, error_message | -| 終了 | spawned Pod が停止 | name(scope 返却のトリガー) | -| scope 又貸し | spawned Pod が自身の scope を別 Pod に委譲 | name, sub_name, sub_pod_socket, delegated_scope | - -### 通知は「シグナル」のみ - -通知にはイベント種類と最小限のメタデータだけを含める。応答テキスト全体は含まない。spawner が内容を知りたければ `ReadPodOutput` で取りに行く。 - -### spawner 側の処理 - -1. callback を受信 -2. `pod.push_notification(source, formatted_message)` でバッファに追加 -3. `PodInterceptor::pre_llm_request` が次の LLM リクエスト時に context に注入 -4. IDLE なら `Method::Notify` の IDLE パスで自動ターン起動 - -### scope 又貸し通知の特殊性 - -spawned Pod (B) が孫 Pod (D) に scope を又貸しした場合: -- B は spawner (A) に通知: 「`/src/core` を D に委譲した。D の socket は ...」 -- A は D の存在と address をこの通知で知る(= 親からの紹介) -- A の spawn 記録に D が追加される -- B が死亡しても A は D を直接把握しており、scope lock file の stale 回収で `delegated_from` が A に付け替わる - -### コールバック送信の実装箇所 - -spawned Pod 側に callback 送信の仕組みが必要: -- **ターン完了**: Controller の `RunEnd` イベント発行時に callback を fire -- **エラー**: Controller の `Error` イベント発行時に callback を fire -- **終了**: Controller の shutdown シーケンス内で callback を fire -- **scope 又貸し**: `SpawnPod` ツールの実行後に callback を fire - -### ポーリングでの代替 - -コールバックが失敗しても、spawner は `ListPods` のポーリングで状態を拾える。コールバックは最適化であり、唯一の手段ではない。 - -## 設計で決めること - -- **callback の message format**: JSON? protocol crate の型を再利用? 独自の軽量フォーマット? -- **callback 送信の非同期性**: ターン完了時に callback 送信を await するか、fire-and-forget で spawn するか -- **callback 失敗時のリトライ**: リトライするか、ログだけ出して諦めるか -- **callback address の更新**: spawner が再起動して socket path が変わった場合の再登録手順 - -## 完了条件 - -- spawned Pod のターン完了・エラー・終了時に spawner の callback address に通知が送信される -- scope 又貸し時に spawner に通知が送信され、spawner が孫 Pod の存在を把握する -- callback 受信が `Method::Notify` パイプラインに乗り、spawner の LLM に system message として伝わる -- callback 送信が失敗しても spawned Pod は続行する -- 単体テストで各通知種別の送受信が検証される - -## 範囲外 - -- リモートの callback(SSH 越し)。ローカル unix socket のみ -- callback の配信保証(at-least-once / exactly-once)。fire-and-forget で十分 diff --git a/tickets/pod-upstream-events.md b/tickets/pod-upstream-events.md new file mode 100644 index 00000000..f6674c13 --- /dev/null +++ b/tickets/pod-upstream-events.md @@ -0,0 +1,123 @@ +# Pod の上流イベント報告 (子 → 親) + +## 背景 + +spawned Pod(子)のライフサイクルに親 Pod が反応する仕組みが必要。反応には 2 系統ある: + +1. **system 処理**: `spawn_pods.json` 更新、`scope.lock` 整合、孫 Pod の把握 +2. **LLM 通知**: 親 LLM に「子でこれが起きた、次どうする?」を考えさせる + +これを実現する通信 primitive を分離して定義する。既存の `Method::Notify`(LLM 向け自由テキスト注入)と混ぜない。 + +## 依存 + +- `tickets/spawn-pod-tool.md`: 済。callback address の受け渡しと spawn 記録 +- `tickets/scope-lock.md` 済に含まれる: allocation の delegate/reparent は既存 +- `Method::PodEvent` の追加は `protocol` crate の拡張 + +## 設計 + +### 新しい primitive + +```rust +pub enum Method { + Run { input: String }, + Notify { message: String }, // 既存: 人間・tool → LLM 文脈、副作用なし + PodEvent(PodEvent), // 新: 子 → 親、typed なライフサイクル報告 + Resume, + Cancel, + Shutdown, + GetHistory, +} + +pub enum PodEvent { + /// 子が1ターン終えて IDLE になった + TurnEnded { pod_name: String }, + + /// 子でエラーが発生した(ターンは続行されるとは限らない) + Errored { pod_name: String, message: String }, + + /// 子が停止した + ShutDown { pod_name: String }, + + /// 子が孫 Pod に scope を又貸しした + ScopeSubDelegated { + parent_pod: String, // 又貸し元(= 子自身) + sub_pod: String, // 孫 Pod の名前 + sub_socket: PathBuf, // 孫 Pod の socket path + scope: Vec, // 又貸しされた scope + }, +} +``` + +`Method::Notify` は触らない。`message: String` のまま。 + +### 子(発信側) + +子の Controller が以下のタイミングで親 socket に一発接続して `Method::PodEvent` を送信し、即切断する(fire-and-forget)。 + +| タイミング | variant | +|---|---| +| `RunEnd { result: Finished }` 発行時 | `TurnEnded` | +| `Event::Error` 発行時 | `Errored` | +| shutdown シーケンス(controller loop 終了直前) | `ShutDown` | +| `SpawnPod` tool 成功直後 | `ScopeSubDelegated` | + +送信は非同期 spawn で発射し、await しない。接続失敗はログのみで続行(親が落ちていても子は生きる)。 + +親 socket のアドレスは spawn 時に `--callback ` で受け取って保持している(既存)。 + +### 親(受信側) + +親 Controller の main loop に `Method::PodEvent` ハンドラを追加: + +```rust +Method::PodEvent(event) => { + // (1) system 処理 — variant ごとに固有 + apply_event_side_effects(&event, &spawned_registry, &lock_path).await; + + // (2) LLM 通知 — variant を文字列にレンダリングしてバッファへ + let text = render_event(&event); + pod.push_notification(text); +} +``` + +variant 別の (1) の中身: + +| variant | system 処理 | +|---|---| +| `TurnEnded` | なし | +| `Errored` | なし(LLM に判断させる) | +| `ShutDown` | `spawned_registry.remove(pod_name)`、scope lock を flock して該当 allocation を `release_pod` で解放 | +| `ScopeSubDelegated` | `spawned_registry.add(SpawnedPodRecord { sub_pod, sub_socket, scope, ... })`(孫を直接把握する。親が子を経由せず孫を管理することで、子が死んでも孫の scope 管理が維持される) | + +(2) の `render_event` は一箇所に集約し、`format!("Pod `{pod_name}` finished a turn.")` のような短い human-readable 文字列を返す。 + +### 失敗時のフォールバック + +- 子 → 親の PodEvent 送信が失敗しても諦める(再試行しない) +- 親が再起動した場合や送信漏れた場合は、親の `ListPods` ツール(既存)による health check + `scope_lock::reclaim_stale` の stale 回収で不整合を解消する +- これは「コールバックは最適化、ポーリングが真のフォールバック」という方針の継続 + +## 設計で決めること + +- **送信の接続タイムアウト**: `SpawnPod` / pod-comm-tools と揃える(5 秒想定) +- **同時多発イベントの順序保証**: `TurnEnded` 直後に `ShutDown` が起きた場合、親側で順序を保証する必要があるか。現状は fire-and-forget で並列送信されうる +- **`ScopeSubDelegated` の親連鎖**: 孫がさらに曾孫を spawn したとき、曾孫の `ScopeSubDelegated` は誰に送る?(子に送り、子が親に転送? or 最上位の root まで届ける? or 直接の親だけで十分?) + +## 完了条件 + +- `Method::PodEvent(PodEvent)` が `protocol` crate に追加され、serde round-trip テストが通る +- 子の Controller が 4 種の variant を適切なタイミングで親 socket に送信する +- 親の Controller が variant ごとの system 処理を実行し、レンダリングした文字列を LLM 通知バッファに流す +- `ScopeSubDelegated` 受信後、孫 Pod が親の `spawned_pods.json` に現れる +- `ShutDown` 受信後、該当 Pod が親の registry から消え、scope lock からも解放される +- 送信失敗しても子プロセスが続行する +- 各 variant の送受信を検証する単体テスト + +## 範囲外 + +- リモート親への送信(SSH 越し)。ローカル Unix socket のみ +- 配信保証(at-least-once / exactly-once) +- 親再起動時の「見逃したイベント」の再送。ポーリングで補う前提 +- `Method::Notify` の `source` フィールド削除(別チケット)