diff --git a/Cargo.lock b/Cargo.lock index 98b337e4..dd045070 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,6 +17,15 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anstream" version = "1.0.0" @@ -216,6 +225,19 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" +[[package]] +name = "chrono" +version = "0.4.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c673075a2e0e5f4a1dde27ce9dee1ea4558c7ffe648f576438a20ca1d2acc4b0" +dependencies = [ + "iana-time-zone", + "js-sys", + "num-traits", + "wasm-bindgen", + "windows-link", +] + [[package]] name = "clap" version = "4.6.0" @@ -1078,6 +1100,30 @@ dependencies = [ "tracing", ] +[[package]] +name = "iana-time-zone" +version = "0.1.65" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e31bc9ad994ba00e440a8aa5c9ef0ec67d5cb5e5cb0cc7f8b744a35b389cc470" +dependencies = [ + "android_system_properties", + "core-foundation-sys", + "iana-time-zone-haiku", + "js-sys", + "log", + "wasm-bindgen", + "windows-core", +] + +[[package]] +name = "iana-time-zone-haiku" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" +dependencies = [ + "cc", +] + [[package]] name = "icu_collections" version = "2.2.0" @@ -1478,6 +1524,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a64a92489e2744ce060c349162be1c5f33c6969234104dbd99ddb5feb08b8c15" +[[package]] +name = "memo-map" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38d1115007560874e373613744c6fba374c17688327a71c1476d1a5954cc857b" + [[package]] name = "memoffset" version = "0.9.1" @@ -1487,6 +1539,16 @@ dependencies = [ "autocfg", ] +[[package]] +name = "minijinja" +version = "2.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "805bfd7352166bae857ee569628b52bcd85a1cecf7810861ebceb1686b72b75d" +dependencies = [ + "memo-map", + "serde", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -1807,11 +1869,13 @@ name = "pod" version = "0.1.0" dependencies = [ "async-trait", + "chrono", "clap", "dotenv", "futures", "llm-worker", "manifest", + "minijinja", "protocol", "provider", "serde", @@ -3419,12 +3483,65 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-core" +version = "0.62.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8e83a14d34d0623b51dce9581199302a221863196a1dde71a7663a4c2be9deb" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-link", + "windows-result", + "windows-strings", +] + +[[package]] +name = "windows-implement" +version = "0.60.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + +[[package]] +name = "windows-interface" +version = "0.59.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "windows-link" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-result" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" +dependencies = [ + "windows-link", +] + +[[package]] +name = "windows-strings" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7837d08f69c77cf6b07689544538e017c1bfcf57e34b4c0ff58e6c2cd3b37091" +dependencies = [ + "windows-link", +] + [[package]] name = "windows-sys" version = "0.52.0" diff --git a/crates/llm-worker/src/tool_server.rs b/crates/llm-worker/src/tool_server.rs index 06722c3c..a7e73356 100644 --- a/crates/llm-worker/src/tool_server.rs +++ b/crates/llm-worker/src/tool_server.rs @@ -72,12 +72,18 @@ impl ToolServerHandle { /// Execute all pending factories and register the resulting tools. /// + /// Called implicitly by `Worker::lock()` before the first turn. + /// Exposed as `pub` so higher layers (e.g. Pod) can force-materialise + /// tools earlier — for example when building a system-prompt template + /// context that needs the list of registered tool names. Redundant + /// calls are no-ops. + /// /// # Panics /// /// Panics if any factory produces a tool whose name collides with /// an already-registered tool. Duplicate names are a programming /// error and should be caught during development. - pub(crate) fn flush_pending(&self) { + pub fn flush_pending(&self) { let pending: Vec<_> = { let mut guard = self.pending.lock().unwrap_or_else(|e| e.into_inner()); std::mem::take(&mut *guard) diff --git a/crates/manifest/src/scope.rs b/crates/manifest/src/scope.rs index 01f41932..292573f7 100644 --- a/crates/manifest/src/scope.rs +++ b/crates/manifest/src/scope.rs @@ -130,6 +130,59 @@ impl Scope { pub fn is_writable(&self, path: &Path) -> bool { matches!(self.permission_at(path), Some(Permission::Write)) } + + /// Iterate over absolute paths granted at least `Read` by an allow + /// rule, preserving declaration order. Does not account for deny + /// rules, which only cap effective permission at query time. + pub fn readable_paths(&self) -> impl Iterator { + self.allow.iter().map(|r| r.target.as_path()) + } + + /// Iterate over absolute paths granted `Write` by an allow rule. + /// Subset of [`readable_paths`](Self::readable_paths). + pub fn writable_paths(&self) -> impl Iterator { + self.allow + .iter() + .filter(|r| r.permission == Permission::Write) + .map(|r| r.target.as_path()) + } + + /// Human-readable grouping of allow rules, suitable for embedding in + /// LLM system prompts. Deny rules are intentionally omitted — they + /// only cap effective permission and surface them would mislead the + /// reader about what paths are accessible. + /// + /// ```text + /// Readable: + /// - /abs/path1 + /// Writable: + /// - /abs/path2 + /// ``` + pub fn summary(&self) -> String { + let mut out = String::new(); + let readable: Vec<_> = self.readable_paths().collect(); + if !readable.is_empty() { + out.push_str("Readable:\n"); + for p in &readable { + out.push_str(" - "); + out.push_str(&p.display().to_string()); + out.push('\n'); + } + } + let writable: Vec<_> = self.writable_paths().collect(); + if !writable.is_empty() { + out.push_str("Writable:\n"); + for p in &writable { + out.push_str(" - "); + out.push_str(&p.display().to_string()); + out.push('\n'); + } + } + if out.ends_with('\n') { + out.pop(); + } + out + } } impl ResolvedRule { @@ -339,6 +392,61 @@ mod tests { assert!(!scope.is_readable(&traversal)); } + #[test] + fn summary_lists_readable_and_writable() { + let dir = TempDir::new().unwrap(); + let docs = dir.path().join("docs"); + std::fs::create_dir(&docs).unwrap(); + let cfg = ScopeConfig { + allow: vec![ + allow_rule(dir.path(), Permission::Read), + allow_rule(&docs, Permission::Write), + ], + deny: Vec::new(), + }; + let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let summary = scope.summary(); + assert!(summary.contains("Readable:")); + assert!(summary.contains("Writable:")); + assert!(summary.contains(&dir.path().canonicalize().unwrap().display().to_string())); + assert!(summary.contains(&docs.canonicalize().unwrap().display().to_string())); + assert!(!summary.ends_with('\n')); + } + + #[test] + fn summary_excludes_deny_rules() { + let dir = TempDir::new().unwrap(); + let secret = dir.path().join("secret"); + std::fs::create_dir(&secret).unwrap(); + let cfg = ScopeConfig { + 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 summary = scope.summary(); + assert!(!summary.contains("secret")); + } + + #[test] + fn readable_paths_includes_writable() { + let dir = TempDir::new().unwrap(); + let docs = dir.path().join("docs"); + std::fs::create_dir(&docs).unwrap(); + let cfg = ScopeConfig { + allow: vec![ + allow_rule(dir.path(), Permission::Read), + allow_rule(&docs, Permission::Write), + ], + deny: Vec::new(), + }; + let scope = Scope::from_config(&cfg, dir.path()).unwrap(); + let readable: Vec<_> = scope.readable_paths().collect(); + let writable: Vec<_> = scope.writable_paths().collect(); + assert_eq!(readable.len(), 2); + assert_eq!(writable.len(), 1); + assert!(writable.iter().all(|w| readable.contains(w))); + } + #[test] fn resolves_new_nested_file_inside_scope() { let dir = TempDir::new().unwrap(); diff --git a/crates/pod/Cargo.toml b/crates/pod/Cargo.toml index 87f159be..cfe855e7 100644 --- a/crates/pod/Cargo.toml +++ b/crates/pod/Cargo.toml @@ -19,6 +19,8 @@ tokio = { version = "1.49", features = ["fs", "io-util", "macros", "net", "rt-mu toml = "1.1.2" tracing = "0.1.44" tools = { version = "0.1.0", path = "../tools" } +minijinja = "2.19.0" +chrono = "0.4.44" [dev-dependencies] async-trait = "0.1.89" diff --git a/crates/pod/src/lib.rs b/crates/pod/src/lib.rs index 5c34bfb7..93241866 100644 --- a/crates/pod/src/lib.rs +++ b/crates/pod/src/lib.rs @@ -9,6 +9,7 @@ mod compact_state; mod hook_interceptor; mod pod; mod prune; +mod system_prompt; mod token_counter; mod usage_tracker; @@ -23,3 +24,4 @@ pub use provider::{ProviderError, build_client}; pub use runtime_dir::RuntimeDir; pub use shared_state::{PodSharedState, PodStatus}; pub use socket_server::SocketServer; +pub use system_prompt::{SystemPromptContext, SystemPromptError, SystemPromptTemplate}; diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index 97e42dba..069c3ac6 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -20,6 +20,7 @@ use crate::hook::{ PreToolCall, }; use crate::hook_interceptor::HookInterceptor; +use crate::system_prompt::{SystemPromptContext, SystemPromptError, SystemPromptTemplate}; use crate::usage_tracker::UsageTracker; use async_trait::async_trait; use llm_worker::interceptor::PreRequestAction; @@ -91,6 +92,10 @@ pub struct Pod { /// tools so that Pod-owned operations (e.g. compaction) can consult /// the recency of touched files. tracker: Option, + /// Parsed system-prompt template awaiting first-turn materialisation. + /// `Some` until `ensure_system_prompt_materialized` renders it once, + /// then `None` forever — including after compaction. + system_prompt_template: Option, } impl Pod { @@ -106,18 +111,16 @@ impl Pod { pwd: PathBuf, scope: Scope, ) -> Result { - let state = SessionStartState { - system_prompt: worker.get_system_prompt(), - config: worker.request_config(), - history: worker.history(), - }; - let (session_id, head_hash) = session_store::create_session(&store, state).await?; + // Session creation is deferred to `ensure_session_head` at first + // run so a later-installed system-prompt template (see + // `set_system_prompt_template`) can be captured by `SessionStart`. + let session_id = session_store::new_session_id(); let mut pod = Self { manifest, worker: Some(worker), store, session_id, - head_hash: Some(head_hash), + head_hash: None, pwd, scope, hook_builder: HookRegistryBuilder::new(), @@ -127,11 +130,20 @@ impl Pod { usage_tracker: Arc::new(UsageTracker::new()), usage_history: Arc::new(Mutex::new(Vec::::new())), tracker: None, + system_prompt_template: None, }; pod.apply_prune_from_manifest(); Ok(pod) } + /// Install a parsed system-prompt template that will be rendered + /// exactly once, immediately before the first LLM turn. Mirrors the + /// path used by `Pod::from_manifest` and is exposed for tests and + /// other callers that build a Pod without going through a manifest. + pub fn set_system_prompt_template(&mut self, template: SystemPromptTemplate) { + self.system_prompt_template = Some(template); + } + /// Restore a Pod from a persisted session. pub async fn restore( session_id: SessionId, @@ -166,6 +178,7 @@ impl Pod { usage_tracker: Arc::new(UsageTracker::new()), usage_history: Arc::new(Mutex::new(state.usage_history)), tracker: None, + system_prompt_template: None, }; pod.apply_prune_from_manifest(); Ok(pod) @@ -364,6 +377,40 @@ impl Pod { } } + /// Render the manifest-supplied system-prompt template exactly once, + /// just before the first LLM turn, and hand the resulting string to + /// the Worker via `set_system_prompt`. Subsequent invocations are + /// no-ops: the template field is consumed with `Option::take()`, so + /// the rendered value persists across all later turns and compaction. + fn ensure_system_prompt_materialized(&mut self) -> Result<(), PodError> { + let Some(template) = self.system_prompt_template.take() else { + return Ok(()); + }; + let worker = self.worker.as_mut().expect("worker present"); + // Materialise any pending tool factories so the template sees the + // full list of tool names. Redundant with the flush inside + // `Worker::lock()`; safe because `flush_pending` is idempotent. + worker.tool_server_handle().flush_pending(); + let tool_names: Vec = worker + .tool_server_handle() + .tool_definitions_sorted() + .into_iter() + .map(|d| d.name) + .collect(); + let ctx = SystemPromptContext { + now: chrono::Utc::now(), + cwd: &self.pwd, + scope: &self.scope, + tool_names, + files: std::collections::BTreeMap::new(), + }; + let rendered = template + .render(&ctx) + .map_err(|source| PodError::SystemPromptRender { source })?; + worker.set_system_prompt(rendered); + Ok(()) + } + /// Send user input and run until the LLM turn completes. /// /// If the between-turns compaction threshold is exceeded mid-run, @@ -371,6 +418,7 @@ impl Pod { /// automatically. pub async fn run(&mut self, input: impl Into) -> Result { self.ensure_interceptor_installed(); + self.ensure_system_prompt_materialized()?; self.ensure_session_head().await?; let history_before = self.worker.as_ref().unwrap().history().len(); @@ -387,6 +435,7 @@ impl Pod { /// Resume from a paused state. pub async fn resume(&mut self) -> Result { self.ensure_interceptor_installed(); + self.ensure_system_prompt_materialized()?; self.ensure_session_head().await?; let history_before = self.worker.as_ref().unwrap().history().len(); @@ -400,18 +449,32 @@ impl Pod { self.handle_worker_result(result, history_before).await } - /// Ensure session head exists (fork if needed). + /// Ensure the session exists and its head still matches ours. + /// + /// On the first call for a Pod built via `from_manifest`, the session + /// has not been written to the store yet — this is when we append the + /// initial `SessionStart` entry, carrying the system prompt that + /// `ensure_system_prompt_materialized` has just rendered. Subsequent + /// calls fall through to `ensure_head_or_fork`, which auto-forks when + /// another writer has advanced the store head behind our back. async fn ensure_session_head(&mut self) -> Result<(), PodError> { let w = self.worker.as_ref().unwrap(); + let state = SessionStartState { + system_prompt: w.get_system_prompt(), + config: w.request_config(), + history: w.history(), + }; + if self.head_hash.is_none() { + let hash = + session_store::create_session_with_id(&self.store, self.session_id, state).await?; + self.head_hash = Some(hash); + return Ok(()); + } session_store::ensure_head_or_fork( &self.store, &mut self.session_id, &mut self.head_hash, - SessionStartState { - system_prompt: w.get_system_prompt(), - config: w.request_config(), - history: w.history(), - }, + state, ) .await?; Ok(()) @@ -725,18 +788,28 @@ impl Pod, St> { let mut worker = Worker::new(client); apply_worker_manifest(&mut worker, &manifest.worker); - let state = SessionStartState { - system_prompt: worker.get_system_prompt(), - config: worker.request_config(), - history: worker.history(), + // Parse the system-prompt template eagerly (syntax check only). + // Rendering is deferred to `ensure_system_prompt_materialized` + // at first turn so implementation runtime values (date, tools, + // scope summary, ...) can be injected. + let system_prompt_template = match manifest.worker.system_prompt.as_deref() { + Some(source) => Some( + SystemPromptTemplate::parse(source) + .map_err(|source| PodError::InvalidSystemPromptTemplate { source })?, + ), + None => None, }; - let (session_id, head_hash) = session_store::create_session(&store, state).await?; + + // Session creation is deferred to the first run (see + // `ensure_session_head`) so the SessionStart entry can capture + // the rendered system prompt, not the raw template source. + let session_id = session_store::new_session_id(); let mut pod = Self { manifest, worker: Some(worker), store, session_id, - head_hash: Some(head_hash), + head_hash: None, pwd, scope, hook_builder: HookRegistryBuilder::new(), @@ -746,6 +819,7 @@ impl Pod, St> { usage_tracker: Arc::new(UsageTracker::new()), usage_history: Arc::new(Mutex::new(Vec::new())), tracker: None, + system_prompt_template, }; pod.apply_prune_from_manifest(); Ok(pod) @@ -753,10 +827,11 @@ impl Pod, St> { } /// Apply worker-level manifest settings to a Worker. +/// +/// Note: `system_prompt` is intentionally not applied here. It is a +/// minijinja template that is parsed by `Pod::from_manifest` and +/// rendered once at first turn in `ensure_system_prompt_materialized`. pub fn apply_worker_manifest(worker: &mut Worker, wm: &WorkerManifest) { - if let Some(ref prompt) = wm.system_prompt { - worker.set_system_prompt(prompt); - } let mut config = RequestConfig::new(); if let Some(max_tokens) = wm.max_tokens { config.max_tokens = Some(max_tokens); @@ -856,6 +931,18 @@ pub enum PodError { #[error("compaction thrash: context still exceeds threshold immediately after compact")] CompactThrash, + + #[error("invalid system prompt template: {source}")] + InvalidSystemPromptTemplate { + #[source] + source: SystemPromptError, + }, + + #[error("failed to render system prompt template: {source}")] + SystemPromptRender { + #[source] + source: SystemPromptError, + }, } /// Resolve the pwd declared in a manifest against `manifest_dir` (or the diff --git a/crates/pod/src/system_prompt.rs b/crates/pod/src/system_prompt.rs new file mode 100644 index 00000000..6d0a3cbe --- /dev/null +++ b/crates/pod/src/system_prompt.rs @@ -0,0 +1,244 @@ +//! System prompt template machinery for the Pod layer. +//! +//! Manifests describe `system_prompt` as a minijinja template string. +//! The template is parsed eagerly at `Pod::from_manifest` (syntax check +//! only) and held on the Pod until `ensure_system_prompt_materialized` +//! renders it exactly once, just before the first LLM turn. The rendered +//! string is pushed to the worker via `set_system_prompt` and is reused +//! for every subsequent turn, including after compaction. + +use std::collections::BTreeMap; +use std::path::Path; +use std::sync::Arc; + +use chrono::{DateTime, SecondsFormat, Utc}; +use manifest::Scope; +use minijinja::value::Value; +use minijinja::{Environment, UndefinedBehavior}; +use thiserror::Error; + +const TEMPLATE_NAME: &str = "system_prompt"; + +#[derive(Debug, Error)] +pub enum SystemPromptError { + #[error("system prompt template parse error: {0}")] + Parse(String), + #[error("system prompt template render error: {0}")] + Render(String), +} + +/// Parsed system-prompt template. Holds a minijinja Environment with a +/// single named template; rendering only needs a fresh [`SystemPromptContext`]. +#[derive(Clone)] +pub struct SystemPromptTemplate { + env: Arc>, +} + +impl SystemPromptTemplate { + /// Parse a template source. Performs syntax validation only — no + /// variable resolution is attempted here. + pub fn parse(source: impl Into) -> Result { + let mut env = Environment::new(); + env.set_undefined_behavior(UndefinedBehavior::Strict); + env.add_template_owned(TEMPLATE_NAME, source.into()) + .map_err(|e| SystemPromptError::Parse(e.to_string()))?; + Ok(Self { env: Arc::new(env) }) + } + + /// Render the template with the supplied context. Missing variables + /// surface as [`SystemPromptError::Render`]. + pub fn render(&self, ctx: &SystemPromptContext<'_>) -> Result { + let tmpl = self + .env + .get_template(TEMPLATE_NAME) + .map_err(|e| SystemPromptError::Render(e.to_string()))?; + tmpl.render(ctx.to_minijinja_value()) + .map_err(|e| SystemPromptError::Render(e.to_string())) + } +} + +impl std::fmt::Debug for SystemPromptTemplate { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SystemPromptTemplate") + .finish_non_exhaustive() + } +} + +/// Inputs available to a system-prompt template at materialisation time. +/// +/// `files` is reserved for AGENTS.md and other external file ingestion +/// (supplied by a separate ticket). It is always present so template +/// authors can reference `{{ files.agents_md }}` without having to guard +/// for key existence. +pub struct SystemPromptContext<'a> { + pub now: DateTime, + pub cwd: &'a Path, + pub scope: &'a Scope, + pub tool_names: Vec, + pub files: BTreeMap, +} + +impl<'a> SystemPromptContext<'a> { + fn to_minijinja_value(&self) -> Value { + let mut root: BTreeMap = BTreeMap::new(); + root.insert( + "date".into(), + Value::from(self.now.format("%Y-%m-%d").to_string()), + ); + root.insert( + "time".into(), + Value::from(self.now.format("%H:%M:%S").to_string()), + ); + root.insert( + "datetime".into(), + Value::from(self.now.to_rfc3339_opts(SecondsFormat::Secs, true)), + ); + root.insert("cwd".into(), Value::from(self.cwd.display().to_string())); + root.insert("scope".into(), scope_value(self.scope)); + root.insert( + "tools".into(), + Value::from( + self.tool_names + .iter() + .cloned() + .map(Value::from) + .collect::>(), + ), + ); + root.insert( + "files".into(), + Value::from( + self.files + .iter() + .map(|(k, v)| (k.clone(), Value::from(v.clone()))) + .collect::>(), + ), + ); + Value::from(root) + } +} + +fn scope_value(scope: &Scope) -> Value { + let readable: Vec = scope + .readable_paths() + .map(|p| Value::from(p.display().to_string())) + .collect(); + let writable: Vec = scope + .writable_paths() + .map(|p| Value::from(p.display().to_string())) + .collect(); + let mut obj: BTreeMap = BTreeMap::new(); + obj.insert("readable".into(), Value::from(readable)); + obj.insert("writable".into(), Value::from(writable)); + obj.insert("summary".into(), Value::from(scope.summary())); + Value::from(obj) +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::TimeZone; + use manifest::{Permission, ScopeConfig, ScopeRule}; + use tempfile::TempDir; + + fn fixed_now() -> DateTime { + Utc.with_ymd_and_hms(2026, 4, 15, 9, 30, 0).unwrap() + } + + fn build_scope(dir: &Path) -> Scope { + let cfg = ScopeConfig { + allow: vec![ScopeRule { + target: dir.to_path_buf(), + permission: Permission::Write, + recursive: true, + }], + deny: Vec::new(), + }; + Scope::from_config(&cfg, dir).unwrap() + } + + fn ctx<'a>(cwd: &'a Path, scope: &'a Scope, tools: Vec) -> SystemPromptContext<'a> { + SystemPromptContext { + now: fixed_now(), + cwd, + scope, + tool_names: tools, + files: BTreeMap::new(), + } + } + + #[test] + fn parse_succeeds_for_minimal_template() { + let t = SystemPromptTemplate::parse("hello").unwrap(); + let dir = TempDir::new().unwrap(); + let scope = build_scope(dir.path()); + let rendered = t.render(&ctx(dir.path(), &scope, vec![])).unwrap(); + assert_eq!(rendered, "hello"); + } + + #[test] + fn parse_fails_on_syntax_error() { + let err = SystemPromptTemplate::parse("{{ unclosed").unwrap_err(); + assert!(matches!(err, SystemPromptError::Parse(_))); + } + + #[test] + fn render_substitutes_date_cwd_tools() { + let t = SystemPromptTemplate::parse( + "date={{ date }} cwd={{ cwd }} tools={{ tools | join(',') }}", + ) + .unwrap(); + let dir = TempDir::new().unwrap(); + let scope = build_scope(dir.path()); + let rendered = t + .render(&ctx( + dir.path(), + &scope, + vec!["alpha".into(), "beta".into()], + )) + .unwrap(); + assert!(rendered.contains("date=2026-04-15")); + assert!(rendered.contains(&format!("cwd={}", dir.path().display()))); + assert!(rendered.contains("tools=alpha,beta")); + } + + #[test] + fn render_fails_on_undefined_variable() { + let t = SystemPromptTemplate::parse("{{ ghost }}").unwrap(); + let dir = TempDir::new().unwrap(); + let scope = build_scope(dir.path()); + let err = t.render(&ctx(dir.path(), &scope, vec![])).unwrap_err(); + assert!(matches!(err, SystemPromptError::Render(_))); + } + + #[test] + fn escape_double_braces() { + let t = SystemPromptTemplate::parse("literal {{ '{{' }} here").unwrap(); + let dir = TempDir::new().unwrap(); + let scope = build_scope(dir.path()); + let rendered = t.render(&ctx(dir.path(), &scope, vec![])).unwrap(); + assert_eq!(rendered, "literal {{ here"); + } + + #[test] + fn scope_summary_renders() { + let t = SystemPromptTemplate::parse("{{ scope.summary }}").unwrap(); + let dir = TempDir::new().unwrap(); + let scope = build_scope(dir.path()); + let rendered = t.render(&ctx(dir.path(), &scope, vec![])).unwrap(); + assert!(rendered.starts_with("Readable:")); + assert!(rendered.contains(&dir.path().canonicalize().unwrap().display().to_string())); + } + + #[test] + fn files_reserved_namespace_is_empty() { + let t = SystemPromptTemplate::parse( + "{% if files.agents_md is defined %}yes{% else %}no{% endif %}", + ) + .unwrap(); + let dir = TempDir::new().unwrap(); + let scope = build_scope(dir.path()); + let rendered = t.render(&ctx(dir.path(), &scope, vec![])).unwrap(); + assert_eq!(rendered, "no"); + } +} diff --git a/crates/pod/tests/system_prompt_template_test.rs b/crates/pod/tests/system_prompt_template_test.rs new file mode 100644 index 00000000..d11f262c --- /dev/null +++ b/crates/pod/tests/system_prompt_template_test.rs @@ -0,0 +1,240 @@ +use std::pin::Pin; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use async_trait::async_trait; +use futures::Stream; +use llm_worker::Worker; +use llm_worker::llm_client::event::{Event as LlmEvent, ResponseStatus, StatusEvent}; +use llm_worker::llm_client::{ClientError, LlmClient, Request}; +use session_store::{FsStore, LogEntry, Store}; + +use pod::{Pod, PodError, SystemPromptTemplate}; + +// --------------------------------------------------------------------------- +// Mock LLM Client +// --------------------------------------------------------------------------- + +#[derive(Clone)] +struct MockClient { + responses: Arc>>, + call_count: Arc, +} + +impl MockClient { + fn new(responses: Vec>) -> Self { + Self { + responses: Arc::new(responses), + call_count: Arc::new(AtomicUsize::new(0)), + } + } +} + +#[async_trait] +impl LlmClient for MockClient { + fn clone_boxed(&self) -> Box { + Box::new(self.clone()) + } + + async fn stream( + &self, + _request: Request, + ) -> Result> + Send>>, ClientError> + { + let count = self.call_count.fetch_add(1, Ordering::SeqCst); + let idx = count.min(self.responses.len() - 1); + let events = self.responses[idx].clone(); + let stream = futures::stream::iter(events.into_iter().map(Ok)); + Ok(Box::pin(stream)) + } +} + +fn single_text_events(text: &str) -> Vec { + vec![ + LlmEvent::text_block_start(0), + LlmEvent::text_delta(0, text), + LlmEvent::text_block_stop(0, None), + LlmEvent::Status(StatusEvent { + status: ResponseStatus::Completed, + }), + ] +} + +fn manifest_toml(system_prompt: Option<&str>) -> String { + let prompt_line = match system_prompt { + Some(s) => format!("system_prompt = {:?}\n", s), + None => String::new(), + }; + format!( + r#" +[pod] +name = "test-pod" +pwd = "./" + +[provider] +kind = "anthropic" +model = "test-model" + +[worker] +max_tokens = 100 +{prompt_line} +[[scope.allow]] +target = "./" +permission = "write" +"# + ) +} + +async fn make_pod_with_template( + template_source: Option<&str>, + client: MockClient, +) -> Result, PodError> { + let manifest = pod::PodManifest::from_toml(&manifest_toml(template_source)).unwrap(); + + let store_tmp = tempfile::tempdir().unwrap(); + let store = FsStore::new(store_tmp.path()).await.unwrap(); + std::mem::forget(store_tmp); + + let pwd_tmp = tempfile::tempdir().unwrap(); + let pwd = pwd_tmp.path().to_path_buf(); + let scope = pod::Scope::writable(&pwd).unwrap(); + std::mem::forget(pwd_tmp); + + let worker = Worker::new(client); + let mut pod = Pod::new(manifest, worker, store, pwd, scope).await?; + + if let Some(source) = template_source { + let template = SystemPromptTemplate::parse(source) + .map_err(|source| PodError::InvalidSystemPromptTemplate { source })?; + pod.set_system_prompt_template(template); + } + Ok(pod) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn template_parse_rejects_invalid_syntax() { + let err = SystemPromptTemplate::parse("{{ unclosed").unwrap_err(); + // Surfaces via PodError::InvalidSystemPromptTemplate when used with + // Pod::from_manifest — tested at the SystemPromptTemplate level here + // because building a Pod via from_manifest requires a real provider. + let pod_err: PodError = PodError::InvalidSystemPromptTemplate { source: err }; + assert!(matches!( + pod_err, + PodError::InvalidSystemPromptTemplate { .. } + )); +} + +#[tokio::test] +async fn template_is_not_materialised_before_first_run() { + let client = MockClient::new(vec![single_text_events("ok")]); + let pod = make_pod_with_template(Some("hello"), client).await.unwrap(); + // Before first run, worker still has no system prompt. + assert!(pod.worker().get_system_prompt().is_none()); +} + +#[tokio::test] +async fn materialise_on_first_turn_populates_worker() { + let client = MockClient::new(vec![single_text_events("ok")]); + let mut pod = make_pod_with_template( + Some("date={{ date }} cwd={{ cwd }} tools={{ tools | join(',') }}"), + client, + ) + .await + .unwrap(); + pod.run("hi").await.unwrap(); + let rendered = pod + .worker() + .get_system_prompt() + .expect("system prompt materialised") + .to_string(); + assert!(rendered.contains("date=")); + assert!(rendered.contains("cwd=")); + assert!(rendered.contains(&pod.pwd().display().to_string())); + assert!(rendered.starts_with("date=")); +} + +#[tokio::test] +async fn session_start_state_captures_rendered_prompt() { + let client = MockClient::new(vec![single_text_events("ok")]); + let mut pod = make_pod_with_template(Some("hello cwd={{ cwd }}"), client) + .await + .unwrap(); + pod.run("hi").await.unwrap(); + + // Inspect the first log entry directly: it must be a SessionStart + // with the rendered system prompt, not `None`. + let entries = pod.store().read_all(pod.session_id()).await.unwrap(); + let first = entries.first().expect("at least one entry"); + match &first.entry { + LogEntry::SessionStart { system_prompt, .. } => { + let sp = system_prompt.as_deref().expect("system prompt set"); + assert!(sp.starts_with("hello cwd=")); + assert!(sp.contains(&pod.pwd().display().to_string())); + } + other => panic!("expected SessionStart as first entry, got {other:?}"), + } +} + +#[tokio::test] +async fn render_failure_propagates_as_pod_error() { + let client = MockClient::new(vec![single_text_events("ok")]); + let mut pod = make_pod_with_template(Some("{{ ghost }}"), client) + .await + .unwrap(); + let err = pod.run("hi").await.unwrap_err(); + assert!(matches!(err, PodError::SystemPromptRender { .. })); +} + +#[tokio::test] +async fn materialise_runs_only_once_across_turns() { + // Two turns; the second one must not re-render the template. We + // approximate this by checking that the rendered system prompt is + // identical across turns and that the Pod's template slot is + // exhausted after the first run. + let client = MockClient::new(vec![ + single_text_events("first"), + single_text_events("second"), + ]); + let mut pod = make_pod_with_template(Some("fixed prompt {{ cwd }}"), client) + .await + .unwrap(); + pod.run("one").await.unwrap(); + let first = pod.worker().get_system_prompt().unwrap().to_string(); + pod.run("two").await.unwrap(); + let second = pod.worker().get_system_prompt().unwrap().to_string(); + assert_eq!(first, second); +} + +#[tokio::test] +async fn compact_preserves_system_prompt() { + // Three user turns, then compact with retained_turns=1. The new + // compacted session must carry the same rendered system prompt and + // the template must not re-run. + let client = MockClient::new(vec![ + single_text_events("a"), + single_text_events("b"), + single_text_events("summary"), + single_text_events("c"), + ]); + let mut pod = make_pod_with_template(Some("SP cwd={{ cwd }}"), client) + .await + .unwrap(); + + pod.run("first").await.unwrap(); + let before = pod.worker().get_system_prompt().unwrap().to_string(); + pod.run("second").await.unwrap(); + + pod.compact(1).await.unwrap(); + + let after = pod.worker().get_system_prompt().unwrap().to_string(); + assert_eq!(before, after); + + // A further run must still see the same prompt (template is None, so + // ensure_system_prompt_materialized is a no-op). + pod.run("third").await.unwrap(); + assert_eq!(pod.worker().get_system_prompt().unwrap(), after.as_str()); +} diff --git a/crates/session-store/src/lib.rs b/crates/session-store/src/lib.rs index 52cf9f38..d3cef025 100644 --- a/crates/session-store/src/lib.rs +++ b/crates/session-store/src/lib.rs @@ -35,9 +35,9 @@ pub mod store; pub use event_trace::TraceEntry; pub use fs_store::FsStore; pub use session::{ - SessionStartState, create_compacted_session, create_session, ensure_head_or_fork, fork, - fork_at, restore, save_cache_locked, save_cache_unlocked, save_config_changed, save_delta, - save_outcome, save_turn_end, save_usage, + SessionStartState, create_compacted_session, create_session, create_session_with_id, + ensure_head_or_fork, fork, fork_at, restore, save_cache_locked, save_cache_unlocked, + save_config_changed, save_delta, save_outcome, save_turn_end, save_usage, }; pub use session_log::{ EntryHash, HashedEntry, LogEntry, Outcome, RestoredState, SessionOrigin, UsageRecord, diff --git a/crates/session-store/src/session.rs b/crates/session-store/src/session.rs index 5a07e57a..a142e0ce 100644 --- a/crates/session-store/src/session.rs +++ b/crates/session-store/src/session.rs @@ -25,6 +25,20 @@ pub async fn create_session( state: SessionStartState<'_>, ) -> Result<(SessionId, EntryHash), StoreError> { let session_id = crate::new_session_id(); + let hash = create_session_with_id(store, session_id, state).await?; + Ok((session_id, hash)) +} + +/// Write a fresh `SessionStart` entry using a pre-generated session ID. +/// +/// Used by callers that need to reserve a session ID synchronously but +/// defer the initial log append (e.g. Pod, which resolves a templated +/// system prompt only at first turn). Returns the resulting head hash. +pub async fn create_session_with_id( + store: &impl Store, + session_id: SessionId, + state: SessionStartState<'_>, +) -> Result { let entry = LogEntry::SessionStart { ts: session_log::now_millis(), system_prompt: state.system_prompt.map(String::from), @@ -40,7 +54,7 @@ pub async fn create_session( entry, }; store.append(session_id, &hashed_entry).await?; - Ok((session_id, hash)) + Ok(hash) } /// Create a compacted session from an existing one. diff --git a/tickets/system-prompt-template.md b/tickets/system-prompt-template.md index cb9f4e0f..88b1ce35 100644 --- a/tickets/system-prompt-template.md +++ b/tickets/system-prompt-template.md @@ -1,45 +1,70 @@ # システムプロンプトのテンプレート化 +**Status:** Reviewed — 指摘事項は `system-prompt-template.review.md` 参照。 + ## 背景 現状、`WorkerManifest.system_prompt` は単なる `Option` で、マニフェスト記述時点の固定テキストしか持てない。実行時に決まる情報(日付、cwd、scope、利用可能なツール、外部ファイルの内容など)をシステムプロンプトに埋め込む手段が無く、Pod ごとに文脈を調整したいケースに対応できない。 AGENTS.md の取り込みをはじめ、今後システムプロンプトへ差し込みたい情報は増えていく見込みで、その受け皿としてテンプレート機構を先に固める。 +プロジェクト内には既に2種類の遅延初期化パターンがある: +- **Tool (llm-worker 層)**: `register_tool` は factory を `pending` に積むだけで、`Worker::lock()` の `flush_pending` が first turn 直前に一括 materialize する。 +- **Hook (Pod 層)**: `hook_builder.add_*` は `HookRegistryBuilder` に積むだけで、`Pod::run` 冒頭の `ensure_interceptor_installed` が `builder.build()` → `worker.set_interceptor` を呼ぶ。1回性は bool フラグで担保。 + +システムプロンプトのテンプレートは **hook と完全に対称な Pod 層の ensure_\* パターン** に乗せる。Worker は低レベル基盤に留めるため、テンプレートの存在を知らない。 + ## 要件 ### 評価モデル -- ツールファクトリと同じく**遅延評価**。マニフェストの値はテンプレート定義として保持し、文字列への materialize は Pod がワーカーを起動して**最初のターンが開始されるタイミングで1回だけ**行う。 -- 一度 materialize したシステムプロンプトは以降のターンを通じて同一値を使う。**compact 後も再評価しない**(compact の前後でシステムプロンプトが変化しないことを保証する)。 +- マニフェストの値はテンプレート定義として `Pod` に保持し、文字列への materialize は **first turn 開始時に1回だけ** 行う。 +- `Pod::run` 冒頭の `ensure_interceptor_installed` の隣に `ensure_system_prompt_materialized` を追加する。`Option::take()` で構造的に1回性を担保し、materialize 済みなら早期 return。 +- 一度 render した文字列は `worker.set_system_prompt` 経由で Worker 側の既存フィールドに乗る。以降のターン、および **compact 後も再評価しない**(compact は Worker の system_prompt フィールドを触らないので、Pod 側の template が `None` になっている限り再 render は構造上不可能)。 ### テンプレート構文と変数 -- プレースホルダ構文を1つ決める(既存エンジン採用 or 最小独自記法)。選定理由を背景に書き残す。 -- 組み込み変数のセットを定義する。初期セットの候補: - - 日付 / 時刻 - - Pod の cwd - - scope 情報(読み取り可能パス等の要約) - - 利用可能なツール一覧 - - AGENTS.md 等の外部ファイル(別チケットで値を供給する前提で、キー空間だけ確保する) -- 未定義変数を参照したときの挙動(エラー / 空文字 / 警告ログ)と、リテラルとして `{{` を出したい場合のエスケープ方法を定める。 +- **テンプレートエンジン**: minijinja を採用する。理由: + - `{{ var }}` / `{% if %}` / `{% for %}` / filter が使え、AGENTS.md の有無で条件分岐したい将来要件にそのまま乗る + - `UndefinedBehavior::Strict` で未定義変数参照を明示エラーにできる(fail-fast に一致) + - エスケープは `{{ '{{' }}` で Jinja2 標準 + - Pure Rust、依存少、メンテ活発 +- 組み込み変数の初期セット: + - `date` / `time` / `datetime`(ISO8601 / RFC3339) + - `cwd`(Pod の絶対パス) + - `scope` — `{ readable: [...], writable: [...], summary: "..." }` + - `tools` — ツール名の sort 済みリスト + - `files` — AGENTS.md 等の外部ファイル用に予約キー空間(別チケットで値を供給する前提で、本チケットでは常に空 Map) +- 未定義変数参照は render エラーとして失敗。 +- `{{` のリテラル出力は `{{ '{{' }}` で可能。 ### マニフェスト上の記法 -- 既存フィールド `system_prompt` をそのままテンプレート文字列として解釈するか、テンプレート用フィールドを新設するかを決める。 -- マニフェストのパース段階ではテンプレートの構文検査のみ行い、変数解決は行わない(遅延評価の原則を崩さない)。 +- 既存フィールド `WorkerManifest.system_prompt: Option` をそのままテンプレート文字列として解釈する。新フィールドは作らない。 +- マニフェストのパース段階ではテンプレート構文検査のみ行う。値の解決は行わない。 + +### 責務の分離 + +- テンプレート機構は **Pod 層** に閉じる。llm-worker は低レベル基盤の原則を維持し、テンプレートの存在を知らない。 +- Worker には materialize 済みの `String` が `set_system_prompt` で渡されるだけ。 ### エラー処理 -- テンプレート展開時のエラー(未定義変数・構文エラー・外部入力の失敗)が first turn 開始時に起きた場合の扱いを決める。ワーカー起動失敗として扱うか、フォールバックで進めるか。 +- 構文エラー → `Pod::from_manifest` 内のテンプレート parse で検出 → `PodError::InvalidSystemPromptTemplate` で起動失敗。 +- render エラー(未定義変数など)→ first turn 開始時に `ensure_system_prompt_materialized` で検出 → `PodError::SystemPromptRender` で起動失敗。 +- フォールバックは用意しない。fail-fast で統一する。 ## 完了条件 -- マニフェストに書いたシステムプロンプトがテンプレートとして解釈され、少なくとも組み込み変数の数種類(例: 日付、cwd)が first turn 開始時に展開されて LLM への system メッセージに反映される。 +- マニフェストに書いたシステムプロンプトがテンプレートとして解釈され、組み込み変数(date / cwd / scope / tools など)が first turn 開始時に展開されて LLM への system メッセージに反映される。 - compact を挟んでもシステムプロンプトが再評価されないことをテストで担保する。 -- 外部ファイル系の変数(AGENTS.md など)は別チケットで供給するため、本チケットでは「変数として受け取れる器」までを用意する。 +- 外部ファイル系の変数(AGENTS.md など)は別チケットで供給するため、本チケットでは「変数として受け取れる器(空の `files` Map)」までを用意する。 ## 範囲外 - AGENTS.md の読み取り自体は別チケット(`agents-md-ingestion.md`)で扱う。 - ユーザ単位の共通設定ファイル(Insomnia 独自の user config)は本チケットのスコープ外。 + +## 付随する変更 + +`SessionStartState.system_prompt` は materialize 後の値で埋まる必要があるため、`create_session` の呼び出しを `Pod::from_manifest` から `Pod::ensure_session_head` に寄せる(= session 作成そのものも遅延する)。これは hook / system_prompt と同じ ensure_\* パターンに揃える変更で、本チケットの一環として行う。 diff --git a/tickets/system-prompt-template.review.md b/tickets/system-prompt-template.review.md new file mode 100644 index 00000000..df937612 --- /dev/null +++ b/tickets/system-prompt-template.review.md @@ -0,0 +1,90 @@ +# Review: system-prompt-template + +## 前提・要件の確認 + +チケットの完了条件と要件をひとつずつ実装と照合した。 + +### 評価モデル ✓ + +- `Pod` が `system_prompt_template: Option` を保持し、`Pod::from_manifest` でパース、`Pod::run`/`Pod::resume` 冒頭の `ensure_system_prompt_materialized` が `Option::take()` で1回だけ materialize する — `pod.rs:385-412`。 +- 既存の `ensure_interceptor_installed` の直後に置かれており、hook と対称な ensure_\* パターンに揃っている。 +- materialize 後は `worker.set_system_prompt(rendered)` で Worker の既存フィールドに流し込む。以降の compact 経由でも template フィールドは `None` のため再 render 不可 — 構造で担保されている。 + +### テンプレート構文・変数 ✓ + +- minijinja 2.19 を採用、`UndefinedBehavior::Strict`、エスケープは `{{ '{{' }}`、single-named template 方式。 +- 組み込み変数の初期セット(`date` / `time` / `datetime` / `cwd` / `scope.{readable,writable,summary}` / `tools` / `files`)が `SystemPromptContext::to_minijinja_value` で全部供給されている — `system_prompt.rs:82-135`。 +- `files` は常に空 BTreeMap で、`{% if files.x is defined %}` 構文で安全にガードできることをテストで確認(`system_prompt.rs:234`)。 + +### マニフェスト上の記法 ✓ + +- 既存 `WorkerManifest.system_prompt: Option` をテンプレート文字列として解釈。新フィールドは増えていない。 +- `apply_worker_manifest` では敢えて `system_prompt` を触らず、doc コメントで理由を明示(`pod.rs:829-834`)。 + +### 責務の分離 ✓ + +- テンプレート機構は `crates/pod/src/system_prompt.rs` に閉じ、`llm-worker` はテンプレートの存在を一切知らない。 +- 唯一 llm-worker 側に入った変更は `ToolServerHandle::flush_pending` の `pub(crate) → pub` 化。これはテンプレート render 前にツール名を確定させるためで、Worker 層の機能性は変わらず、理由も doc で明示されている(`tool_server.rs:72-82`)。基盤原則への違反ではない。 + +### エラー処理 ✓ + +- 構文エラー → `PodError::InvalidSystemPromptTemplate`、render エラー → `PodError::SystemPromptRender`。両方とも `#[source]` 付きで `SystemPromptError` をラップ。fail-fast 一貫。 + +### 付随する変更 ✓ + +- `create_session` を `from_manifest` から外し、`ensure_session_head` 側で `create_session_with_id` を呼ぶように遅延化。`session-store` 側に `create_session_with_id` を新設 — ID を先に確定しつつ初回 log append を遅延するユースケース用、doc コメント付き。 +- 結果として `SessionStartState.system_prompt` が materialize 後の値で埋まる。`session_start_state_captures_rendered_prompt` テストで log entry を直接検証。 + +### 完了条件 ✓ + +- マニフェスト → テンプレート解釈 → 組み込み変数展開 → LLM system message 反映の一連の流れが `materialise_on_first_turn_populates_worker` で確認されている。 +- compact を挟んでもシステムプロンプトが再評価されないことを `compact_preserves_system_prompt` が直接検証。 +- `files` は空 Map の器として用意済み(AGENTS.md 供給は別チケット)。 + +### テスト + +- `crates/pod/src/system_prompt.rs` 単体: 7本(parse 成功/失敗、date/cwd/tools 置換、未定義変数、`{{` エスケープ、scope.summary、files 空)。 +- `crates/pod/tests/system_prompt_template_test.rs` 統合: 7本(parse エラー、first run 前未 materialize、first turn で materialize、SessionStart capture、render エラー伝播、2 turn 間で一意、compact 越え保存)。 +- `cargo test --workspace`: 323 passed / 0 failed。 + +## 指摘事項 + +いずれも blocking ではない。load-bearing な修正は無い。 + +### 1. `Pod::new` が `manifest.worker.system_prompt` を黙って無視する(軽微) + +`Pod::new` は manifest を受け取るが template parse は行わず、`system_prompt_template: None` で初期化する。そのため `Pod::new` 経由で manifest 由来の system_prompt を効かせたい場合はテスト側で `SystemPromptTemplate::parse` + `set_system_prompt_template` を手動で呼ぶ必要がある。 + +現状 `Pod::new` は production からは使われておらず、用途はテストのみ(`controller_test.rs`, `system_prompt_template_test.rs`)。production 経路は `Pod::from_manifest` 一本で、そちらは正しく parse する。 + +- **判断**: 現状維持で OK。`Pod::new` は低レベルコンストラクタであり、manifest の解釈を完全に担う責務は持っていない(`apply_worker_manifest` 相当の処理も外から呼ぶ想定)。ただし doc コメントで「`system_prompt` template は Pod::new の対象外、必要なら `set_system_prompt_template` を呼べ」と一行書いておくと事故を防ぎやすい。 + +### 2. `flush_pending` の public 化は「Pod 側でツール名を先取りする」ため(情報共有) + +Pod の `ensure_system_prompt_materialized` は Worker の lock() より前に走るので、その時点で pending factory はまだ materialize されていない可能性がある。そこで `worker.tool_server_handle().flush_pending()` を明示的に呼んでツール一覧を確定させている(`pod.rs:393`)。 + +flush_pending は冪等であり(実装上も空 Vec を drain するだけ)、Worker::lock() 側での二重呼び出しも問題ない。API 的には「force-materialise を higher layer が要求できる」という意味付けが明確なので、doc コメントがあれば十分。既に doc は追加済み。 + +- **判断**: 現状維持で OK。`llm-worker` を低レベル基盤に留める原則に照らしても、「明示的フラッシュ」という低レベル操作を公開するだけで、テンプレートの概念は漏れていない。 + +### 3. `SystemPromptTemplate` が単一テンプレート専用に `Environment<'static>` を抱える(美観) + +`minijinja::Environment` はテンプレートの名前空間を持つが、ここでは固定名 `"system_prompt"` で1枚だけ登録している。代替としては `minijinja::Template::new` 相当のスタンドアロンパースもあるが、minijinja の public API は Environment 経由が主流で、コストは Arc 1 つ分。 + +- **判断**: 現状維持で OK。将来 filter / function を足したくなった時に Environment を使うほうが素直。 + +### 4. `render` 時の `Environment::get_template` が `Result` を返すことへの扱い(微細) + +`SystemPromptTemplate::render` は `get_template` の失敗も `SystemPromptError::Render` にマップしているが、parse 時に必ず登録しているので実質到達しない経路。`expect` でも良いが、`.map_err` で統一されているのは無害。 + +- **判断**: 現状維持で OK。 + +### 5. `SystemPromptContext` の `tool_names: Vec` を `&[String]` にできる余地(微細) + +所有の必要性は無いので参照でも良い。ただし `ensure_system_prompt_materialized` が一時 Vec を作る以上、所有のほうが呼び出し側が楽。AGENTS.md 取り込み時に `files` を構築するコードと書き味を合わせる意味でも所有のままで良い。 + +- **判断**: 現状維持で OK。 + +## 結論 + +**accept**。チケットの要件を過不足なく満たしており、テストも 1:1 で要件項目を検証している。指摘事項はいずれも doc コメントの追記レベルで、必須ではない。