diff --git a/prompts/en/memory_persistence.md.j2 b/prompts/en/memory_persistence.md.j2 index 2b2932313..933760994 100644 --- a/prompts/en/memory_persistence.md.j2 +++ b/prompts/en/memory_persistence.md.j2 @@ -69,3 +69,27 @@ This is an automatic process triggered periodically during conversation. You are 8. Verify stale memory before relying on it. If a recalled memory conflicts with the newer conversation context, treat the older item as stale, avoid propagating it as truth, and capture the latest truth via `updates` or `contradicts`. +{% if skill_reflection %} +## Skill Reflection + +This session involved substantial work, so this pass also decides whether it produced a reusable procedure. Memory answers "who is the user and what is going on"; skills answer "how do we do this class of task here." A correction like "stop posting walls of text in Discord" is not a fact about the user — it's a standing procedure change, and it belongs in the skill governing that task class. + +First decide whether anything is worth keeping. Ending with no skill writes is the common case and completely acceptable — but treat a session where the user corrected the agent's procedure as a strong write signal. + +If something is worth keeping, follow this order strictly: + +1. **Patch the skill that governed this task**, if one was used this session. Call `skills_list` to see what exists, `read_skill` before any modification. +2. **Patch a related existing skill** whose task class covers the lesson. +3. **Add a `references/` file** to an existing skill via `write_file` when the material is supporting detail, not procedure. +4. **Only then create a new skill** — named for the task class (`discord-formatting`), never the incident. Keep the description within its budget; it's loaded into every system prompt. + +Never persist: + +- Environment-dependent failures (a missing binary, a network hiccup, a full disk). +- Negative capability claims ("tool X doesn't work") — these harden into refusals that outlive the actual problem. +- Transient errors that resolved on retry. +- One-off task narratives — a skill is a procedure that will be right next month, not a log of what happened today. +- Unresolved failures dressed up as procedure. If the session didn't find a working approach, there is nothing to teach. + +Skill writes happen through `skill_manage` and are subject to rails: you can only modify workspace skills you have read this session, and installed or pinned skills are off-limits. If a rail refuses a write, do not fight it — note the lesson as a memory instead. +{% endif %} diff --git a/src/agent/channel.rs b/src/agent/channel.rs index 301b146c2..aab63de13 100644 --- a/src/agent/channel.rs +++ b/src/agent/channel.rs @@ -715,6 +715,12 @@ pub struct Channel { message_count: usize, /// When the last memory persistence branch was triggered. last_persistence_at: std::time::Instant, + /// Set when a turn or worker crossed the reflection work threshold. + /// Consumed by the next persistence branch, which then also reflects + /// on skills. Atomic because turn processing marks it through `&self`. + reflection_signal: std::sync::atomic::AtomicBool, + /// When the last skill-reflection pass was spawned, for cooldown. + last_reflection_at: Option, /// Branch IDs for silent memory persistence branches (results not injected into history). memory_persistence_branches: HashSet, /// Optional Discord reply target captured when each branch was started. @@ -907,6 +913,8 @@ impl Channel { message_count: 0, last_persistence_at: std::time::Instant::now(), memory_persistence_branches: HashSet::new(), + reflection_signal: std::sync::atomic::AtomicBool::new(false), + last_reflection_at: None, branch_reply_targets: HashMap::new(), coalesce_buffer: Vec::new(), coalesce_deadline: None, @@ -2923,18 +2931,29 @@ impl Channel { .await; } - let applied_history = { + let (applied_history, turn_tool_calls) = { let mut guard = self.state.history.write().await; - apply_history_after_turn( + let applied = apply_history_after_turn( &result, &mut guard, history, history_len_before, &self.id, is_retrigger, - ) + ); + let appended_from = history_len_before.min(guard.len()); + let tool_calls = + crate::agent::channel_history::count_tool_call_messages(&guard[appended_from..]); + (applied, tool_calls) }; + { + let reflection = self.deps.runtime_config.skills_config.load().reflection; + if reflection.enabled && turn_tool_calls >= reflection.min_tool_iterations { + self.mark_reflection_signal("turn_tool_calls"); + } + } + let remove_result = match self.resolved_settings.delegation { DelegationMode::Direct => { crate::tools::remove_direct_mode_tools(&self.tool_server, allow_direct_reply).await @@ -3400,6 +3419,15 @@ impl Channel { run_logger.log_worker_completed(*worker_id, result, *success); + // A worker finishing real work successfully is a reflection + // signal: the session likely produced a reusable lesson. + if *success { + let reflection = self.deps.runtime_config.skills_config.load().reflection; + if reflection.enabled { + self.mark_reflection_signal("worker_completed"); + } + } + self.state.active_workers.write().await.remove(worker_id); self.state.worker_inputs.write().await.remove(worker_id); self.state.worker_injections.write().await.remove(worker_id); @@ -3700,18 +3728,55 @@ impl Channel { status.render_full(¤t_time_line, &system_info) } + /// Note that this conversation just did substantial work. The next + /// persistence pass will also reflect on skills, cooldown permitting. + /// + /// Cron conversations never reflect: scheduled runs repeat the same + /// procedure on a timer and would grind out noise skills. + fn mark_reflection_signal(&self, source: &'static str) { + if self.id.starts_with("cron") { + return; + } + let was_set = self + .reflection_signal + .swap(true, std::sync::atomic::Ordering::Relaxed); + if !was_set { + tracing::debug!(channel_id = %self.id, source, "skill reflection signal set"); + } + } + + /// Whether the next persistence pass should reflect on skills. + fn reflection_due(&self) -> bool { + if !self + .reflection_signal + .load(std::sync::atomic::Ordering::Relaxed) + { + return false; + } + let config = self.deps.runtime_config.skills_config.load().reflection; + if !config.enabled { + return false; + } + match self.last_reflection_at { + Some(at) => at.elapsed().as_secs() >= config.cooldown_secs, + None => true, + } + } + /// Check if a memory persistence branch should be spawned. /// - /// Three triggers (any one fires): - /// 1. **Message count** — threshold reached (default 20, configurable) - /// 2. **Time-based** — elapsed since last persistence, if conversation is active - /// 3. **Event density** — working memory events from this channel since last persistence + /// Three memory triggers (any one fires): message count, time since last + /// persistence, and working-memory event density. A pending skill + /// reflection signal is a fourth trigger and can spawn the pass on its + /// own; the branch it spawns also reflects on skills. async fn check_memory_persistence(&mut self) { let config = **self.deps.runtime_config.memory_persistence.load(); - if !config.enabled - || config.message_interval == 0 - || !self.resolved_settings.memory.persistence_enabled() - { + let persistence_enabled = config.enabled + && config.message_interval != 0 + && self.resolved_settings.memory.persistence_enabled(); + let reflection_due = self.reflection_due(); + + if !persistence_enabled && !reflection_due { return; } @@ -3719,14 +3784,16 @@ impl Channel { let elapsed = self.last_persistence_at.elapsed(); // Trigger 1: Message count threshold. - let message_trigger = self.message_count >= wm_config.persistence_message_threshold; + let message_trigger = + persistence_enabled && self.message_count >= wm_config.persistence_message_threshold; // Trigger 2: Time-based — only if conversation is active (message_count > 0). - let time_trigger = self.message_count > 0 + let time_trigger = persistence_enabled + && self.message_count > 0 && elapsed.as_secs() >= wm_config.persistence_time_threshold_secs; // Trigger 3: Event density — working memory events from this channel. - let density_trigger = if !message_trigger && !time_trigger { + let density_trigger = if persistence_enabled && !message_trigger && !time_trigger { // Only check DB if the cheap triggers didn't fire. let since = chrono::Utc::now() - chrono::Duration::seconds(elapsed.as_secs() as i64); match self @@ -3745,7 +3812,7 @@ impl Channel { false }; - if !message_trigger && !time_trigger && !density_trigger { + if !message_trigger && !time_trigger && !density_trigger && !reflection_due { return; } @@ -3753,21 +3820,29 @@ impl Channel { "message_count" } else if time_trigger { "time" - } else { + } else if density_trigger { "event_density" + } else { + "reflection" }; // Reset counters before spawning so subsequent messages don't pile up. self.message_count = 0; self.last_persistence_at = std::time::Instant::now(); + if reflection_due { + self.last_reflection_at = Some(std::time::Instant::now()); + self.reflection_signal + .store(false, std::sync::atomic::Ordering::Relaxed); + } - match spawn_memory_persistence_branch(&self.state, &self.deps).await { + match spawn_memory_persistence_branch(&self.state, &self.deps, reflection_due).await { Ok(branch_id) => { self.memory_persistence_branches.insert(branch_id); tracing::info!( channel_id = %self.id, branch_id = %branch_id, trigger, + skill_reflection = reflection_due, "memory persistence branch spawned" ); } diff --git a/src/agent/channel_dispatch.rs b/src/agent/channel_dispatch.rs index 9270837b2..d6a3191eb 100644 --- a/src/agent/channel_dispatch.rs +++ b/src/agent/channel_dispatch.rs @@ -212,9 +212,14 @@ pub async fn spawn_branch_from_state( /// Uses the same branching infrastructure as regular branches but with a /// dedicated prompt focused on memory recall + save. The result is not injected /// into channel history — the channel handles these branch IDs specially. +/// +/// When `skill_reflection` is set, the same pass also reflects on skills: +/// the prompt gains the reflection section and the branch gets skill tools +/// under agent-origin rails. pub(crate) async fn spawn_memory_persistence_branch( state: &ChannelState, deps: &AgentDeps, + skill_reflection: bool, ) -> std::result::Result { let contract_state = Arc::new(MemoryPersistenceContractState::default()); @@ -223,7 +228,7 @@ pub(crate) async fn spawn_memory_persistence_branch( let model_name = routing.resolve(ProcessType::Branch, None).to_string(); let tool_use_enforcement = deps.runtime_config.tool_use_enforcement.load(); let system_prompt = prompt_engine - .render_static("memory_persistence") + .render_memory_persistence_prompt(skill_reflection) .and_then(|prompt| { prompt_engine.maybe_append_tool_use_enforcement( prompt, @@ -241,13 +246,18 @@ pub(crate) async fn spawn_memory_persistence_branch( "memory persistence", &prompt, &system_prompt, - "persisting memories...", + if skill_reflection { + "persisting memories and reflecting on skills..." + } else { + "persisting memories..." + }, "memory_persistence_branch", BranchSpawnOptions { profile: BranchToolProfile::MemoryPersistence { contract_state, working_memory: Some(state.deps.working_memory.clone()), channel_id: Some(state.channel_id.to_string()), + skill_reflection, }, }, ) diff --git a/src/agent/channel_history.rs b/src/agent/channel_history.rs index c57833041..6eb74cbd3 100644 --- a/src/agent/channel_history.rs +++ b/src/agent/channel_history.rs @@ -181,6 +181,25 @@ pub(crate) struct AppliedHistory { pub reply_text: Option, } +/// Count assistant messages carrying tool calls in a history slice. +/// +/// Used as the per-turn work measure for skill reflection: each counted +/// message is one tool iteration of the agentic loop. +pub(crate) fn count_tool_call_messages(messages: &[rig::message::Message]) -> usize { + messages + .iter() + .filter(|message| { + if let rig::message::Message::Assistant { content, .. } = message { + content + .iter() + .any(|c| matches!(c, rig::message::AssistantContent::ToolCall(_))) + } else { + false + } + }) + .count() +} + pub(crate) fn pop_retrigger_bridge_message(history: &mut Vec) -> bool { if history.last().is_some_and(is_retrigger_bridge_message) { history.pop(); @@ -496,7 +515,7 @@ pub(crate) fn event_is_for_channel(event: &ProcessEvent, channel_id: &ChannelId) #[cfg(test)] mod tests { - use super::{apply_history_after_turn, event_is_for_channel}; + use super::{apply_history_after_turn, count_tool_call_messages, event_is_for_channel}; use crate::{ChannelId, ProcessEvent, ProcessId}; use rig::completion::{CompletionError, PromptError}; use rig::message::Message; @@ -800,6 +819,30 @@ mod tests { ); } + #[test] + fn count_tool_call_messages_counts_only_tool_call_assistants() { + let tool_call_msg = Message::Assistant { + id: None, + content: rig::OneOrMany::one(rig::message::AssistantContent::tool_call( + "call_1", + "shell", + serde_json::json!({"command": "ls"}), + )), + }; + + let messages = vec![ + user_msg("do the thing"), + tool_call_msg.clone(), + user_msg("(tool result)"), + tool_call_msg, + assistant_msg("done"), + ]; + + assert_eq!(count_tool_call_messages(&messages), 2); + assert_eq!(count_tool_call_messages(&[]), 0); + assert_eq!(count_tool_call_messages(&make_history(&["hi", "hey"])), 0); + } + /// Rollback on empty history is a no-op and must not panic. #[test] fn rollback_on_empty_history_is_noop() { diff --git a/src/agent/ingestion.rs b/src/agent/ingestion.rs index 1d132985b..81e820997 100644 --- a/src/agent/ingestion.rs +++ b/src/agent/ingestion.rs @@ -505,6 +505,7 @@ async fn process_chunk( contract_state: contract_state.clone(), working_memory: Some(deps.working_memory.clone()), channel_id: None, + skill_reflection: false, }, deps.api_state.clone(), deps.wiki_store.clone(), diff --git a/src/api/agents.rs b/src/api/agents.rs index f70b6a0e4..e825ab6c0 100644 --- a/src/api/agents.rs +++ b/src/api/agents.rs @@ -759,6 +759,7 @@ pub async fn create_agent_internal( }; let raw_config = crate::config::AgentConfig { + skills: None, id: agent_id.clone(), default: false, display_name: request.display_name.clone().filter(|s| !s.is_empty()), diff --git a/src/config/load.rs b/src/config/load.rs index b4c4e5df5..cd5c79dc6 100644 --- a/src/config/load.rs +++ b/src/config/load.rs @@ -16,13 +16,29 @@ use super::{ DiscordInstanceConfig, EmailConfig, EmailInstanceConfig, GroupDef, HumanDef, IngestionConfig, LinkDef, LlmConfig, MattermostConfig, MattermostInstanceConfig, McpServerConfig, McpTransport, MemoryJanitorConfig, MemoryPersistenceConfig, MessagingConfig, MetricsConfig, OpenCodeConfig, - ParticipantContextConfig, ProjectsConfig, ProviderConfig, SignalConfig, SignalInstanceConfig, - SlackCommandConfig, SlackConfig, SlackInstanceConfig, TelegramConfig, TelegramInstanceConfig, - TelemetryConfig, TwitchConfig, TwitchInstanceConfig, WarmupConfig, WebhookConfig, - normalize_adapter, validate_named_messaging_adapters, + ParticipantContextConfig, ProjectsConfig, ProviderConfig, ReflectionConfig, SignalConfig, + SignalInstanceConfig, SkillsConfig, SlackCommandConfig, SlackConfig, SlackInstanceConfig, + TelegramConfig, TelegramInstanceConfig, TelemetryConfig, TwitchConfig, TwitchInstanceConfig, + WarmupConfig, WebhookConfig, normalize_adapter, validate_named_messaging_adapters, }; use crate::error::{ConfigError, Result}; +/// Merge a `[skills]` TOML section over a base config. +fn resolve_skills_config(toml: TomlSkillsConfig, base: SkillsConfig) -> SkillsConfig { + SkillsConfig { + reflection: toml + .reflection + .map(|r| ReflectionConfig { + enabled: r.enabled.unwrap_or(base.reflection.enabled), + min_tool_iterations: r + .min_tool_iterations + .unwrap_or(base.reflection.min_tool_iterations), + cooldown_secs: r.cooldown_secs.unwrap_or(base.reflection.cooldown_secs), + }) + .unwrap_or(base.reflection), + } +} + use anyhow::Context as _; use std::collections::HashMap; @@ -942,6 +958,7 @@ impl Config { ingestion: None, cortex: None, warmup: None, + skills: None, browser: None, channel: None, mcp: None, @@ -1584,6 +1601,11 @@ impl Config { .unwrap_or(base_defaults.memory_persistence.message_interval), }) .unwrap_or(base_defaults.memory_persistence), + skills: toml + .defaults + .skills + .map(|s| resolve_skills_config(s, base_defaults.skills)) + .unwrap_or(base_defaults.skills), coalesce: toml .defaults .coalesce @@ -1853,6 +1875,7 @@ impl Config { .message_interval .unwrap_or(defaults.memory_persistence.message_interval), }), + skills: a.skills.map(|s| resolve_skills_config(s, defaults.skills)), coalesce: a.coalesce.map(|c| CoalesceConfig { enabled: c.enabled.unwrap_or(defaults.coalesce.enabled), debounce_ms: c.debounce_ms.unwrap_or(defaults.coalesce.debounce_ms), @@ -1984,6 +2007,7 @@ impl Config { ingestion: None, cortex: None, warmup: None, + skills: None, browser: None, channel: None, mcp: None, @@ -2683,3 +2707,33 @@ fn load_human_md(human_dir: &std::path::Path) -> Option { _ => None, } } + +#[cfg(test)] +mod skills_config_tests { + use super::*; + + #[test] + fn resolve_skills_config_merges_partial_toml_over_base() { + let base = SkillsConfig::default(); + + let toml: TomlSkillsConfig = + toml::from_str("[reflection]\nmin_tool_iterations = 4").unwrap(); + let merged = resolve_skills_config(toml, base); + assert_eq!(merged.reflection.min_tool_iterations, 4); + assert_eq!(merged.reflection.enabled, base.reflection.enabled); + assert_eq!( + merged.reflection.cooldown_secs, + base.reflection.cooldown_secs + ); + + let toml: TomlSkillsConfig = toml::from_str("").unwrap(); + let merged = resolve_skills_config(toml, base); + assert_eq!(merged.reflection.enabled, base.reflection.enabled); + + let toml: TomlSkillsConfig = + toml::from_str("[reflection]\nenabled = false\ncooldown_secs = 60").unwrap(); + let merged = resolve_skills_config(toml, base); + assert!(!merged.reflection.enabled); + assert_eq!(merged.reflection.cooldown_secs, 60); + } +} diff --git a/src/config/runtime.rs b/src/config/runtime.rs index d9313f6e4..1fc41c3f1 100644 --- a/src/config/runtime.rs +++ b/src/config/runtime.rs @@ -78,6 +78,8 @@ pub struct RuntimeConfig { pub settings: ArcSwap>>, /// Skill provenance and usage tracking, set after agent initialization. pub skill_usage: ArcSwap>>, + /// Skill lifecycle configuration (reflection triggers, cooldowns). + pub skills_config: ArcSwap, /// Prompt snapshot store for debugging prompt construction. pub prompt_snapshots: ArcSwap>>, /// Secrets store for encrypted credential storage. @@ -158,6 +160,7 @@ impl RuntimeConfig { cron_scheduler: ArcSwap::from_pointee(None), settings: ArcSwap::from_pointee(None), skill_usage: ArcSwap::from_pointee(None), + skills_config: ArcSwap::from_pointee(agent_config.skills), prompt_snapshots: ArcSwap::from_pointee(None), secrets: ArcSwap::from_pointee(None), sandbox: Arc::new(ArcSwap::from_pointee(agent_config.sandbox.clone())), @@ -288,6 +291,7 @@ impl RuntimeConfig { self.user_timezone.store(Arc::new(resolved.user_timezone)); self.cortex.store(Arc::new(resolved.cortex)); self.warmup.store(Arc::new(resolved.warmup)); + self.skills_config.store(Arc::new(resolved.skills)); self.participant_context .store(Arc::new(config.defaults.participant_context)); // Preserve project_paths from the current sandbox config when diff --git a/src/config/toml_schema.rs b/src/config/toml_schema.rs index b336ad354..946cb1512 100644 --- a/src/config/toml_schema.rs +++ b/src/config/toml_schema.rs @@ -299,6 +299,7 @@ pub(super) struct TomlDefaultsConfig { pub(super) ingestion: Option, pub(super) cortex: Option, pub(super) warmup: Option, + pub(super) skills: Option, pub(super) participant_context: Option, pub(super) browser: Option, pub(super) channel: Option, @@ -345,6 +346,18 @@ pub(super) struct TomlMemoryPersistenceConfig { pub(super) message_interval: Option, } +#[derive(Deserialize)] +pub(super) struct TomlSkillsConfig { + pub(super) reflection: Option, +} + +#[derive(Deserialize)] +pub(super) struct TomlReflectionConfig { + pub(super) enabled: Option, + pub(super) min_tool_iterations: Option, + pub(super) cooldown_secs: Option, +} + #[derive(Deserialize)] pub(super) struct TomlCoalesceConfig { pub(super) enabled: Option, @@ -491,6 +504,7 @@ pub(super) struct TomlAgentConfig { pub(super) ingestion: Option, pub(super) cortex: Option, pub(super) warmup: Option, + pub(super) skills: Option, pub(super) browser: Option, pub(super) channel: Option, pub(super) mcp: Option>, diff --git a/src/config/types.rs b/src/config/types.rs index 21547ca2f..2ea4af8f3 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -643,6 +643,7 @@ pub struct DefaultsConfig { pub ingestion: IngestionConfig, pub cortex: CortexConfig, pub warmup: WarmupConfig, + pub skills: SkillsConfig, pub participant_context: ParticipantContextConfig, pub browser: BrowserConfig, pub channel: ChannelConfig, @@ -784,6 +785,40 @@ impl Default for MemoryPersistenceConfig { } } +/// Skill lifecycle configuration. +#[derive(Debug, Clone, Copy, Default)] +pub struct SkillsConfig { + /// Reflection: the outcome-to-skill pump riding the memory persistence + /// branch. + pub reflection: ReflectionConfig, +} + +/// When the persistence branch also reflects on skills. +/// +/// Reflection is counter-based, not idle-based: the signal that something +/// was learned is that real work happened. When a turn crosses +/// `min_tool_iterations` (or a worker attached to the conversation succeeds), +/// the next persistence branch runs with skill tools under agent-origin +/// rails. Cron-originated conversations never reflect. +#[derive(Debug, Clone, Copy)] +pub struct ReflectionConfig { + pub enabled: bool, + /// Tool iterations in a single turn that mark it worth reflecting on. + pub min_tool_iterations: usize, + /// Minimum seconds between reflection passes per conversation. + pub cooldown_secs: u64, +} + +impl Default for ReflectionConfig { + fn default() -> Self { + Self { + enabled: true, + min_tool_iterations: 10, + cooldown_secs: 3600, + } + } +} + /// Working memory system configuration. /// /// Controls the temporal event log, intra-day synthesis, channel activity map, @@ -1400,6 +1435,7 @@ pub struct AgentConfig { pub ingestion: Option, pub cortex: Option, pub warmup: Option, + pub skills: Option, pub browser: Option, pub channel: Option, pub mcp: Option>, @@ -1463,6 +1499,7 @@ pub struct ResolvedAgentConfig { pub ingestion: IngestionConfig, pub cortex: CortexConfig, pub warmup: WarmupConfig, + pub skills: SkillsConfig, pub browser: BrowserConfig, pub channel: ChannelConfig, pub mcp: Vec, @@ -1495,6 +1532,7 @@ impl Default for DefaultsConfig { ingestion: IngestionConfig::default(), cortex: CortexConfig::default(), warmup: WarmupConfig::default(), + skills: SkillsConfig::default(), participant_context: ParticipantContextConfig::default(), browser: BrowserConfig::default(), channel: ChannelConfig::default(), @@ -1562,6 +1600,7 @@ impl AgentConfig { ingestion: self.ingestion.unwrap_or(defaults.ingestion), cortex: self.cortex.unwrap_or(defaults.cortex), warmup: self.warmup.unwrap_or(defaults.warmup), + skills: self.skills.unwrap_or(defaults.skills), browser: self .browser .clone() diff --git a/src/prompts/engine.rs b/src/prompts/engine.rs index d178c5408..b04af58c6 100644 --- a/src/prompts/engine.rs +++ b/src/prompts/engine.rs @@ -300,6 +300,20 @@ impl PromptEngine { ) } + /// Render the memory persistence branch system prompt. + /// + /// `skill_reflection` adds the reflection section: the pass also decides + /// whether the session produced a reusable procedure worth persisting as + /// a skill. + pub fn render_memory_persistence_prompt(&self, skill_reflection: bool) -> Result { + self.render( + "memory_persistence", + context! { + skill_reflection => skill_reflection, + }, + ) + } + /// Render the skills listing for a branch system prompt. /// /// Branches read skills directly via `read_skill` or pass names to @@ -870,6 +884,24 @@ mod tests { assert!(!prompt.contains("## Knowledge Context")); } + #[test] + fn memory_persistence_prompt_gates_reflection_section() { + let engine = PromptEngine::new("en").expect("prompt engine should build"); + + let plain = engine + .render_memory_persistence_prompt(false) + .expect("persistence prompt should render"); + assert!(plain.contains("memory persistence process")); + assert!(!plain.contains("## Skill Reflection")); + + let reflecting = engine + .render_memory_persistence_prompt(true) + .expect("reflection prompt should render"); + assert!(reflecting.contains("## Skill Reflection")); + assert!(reflecting.contains("never the incident")); + assert!(reflecting.contains("Never persist")); + } + #[test] fn knowledge_synthesis_prompt_preserves_participant_roles() { let engine = PromptEngine::new("en").expect("prompt engine should build"); diff --git a/src/tools.rs b/src/tools.rs index aa82b3cc0..28e531d9c 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -284,6 +284,9 @@ pub enum BranchToolProfile { contract_state: Arc, working_memory: Option>, channel_id: Option, + /// When set, this persistence pass also reflects on skills: the + /// branch gets skill tools under agent-origin rails. + skill_reflection: bool, }, } @@ -906,22 +909,33 @@ pub fn create_branch_tool_server( sandbox, )); - // Skill tools go to conversation branches only. They carry User origin - // because the user is present and directing; memory-persistence and - // ingestion branches are focused pipelines and get no mutation surface. - // The reflection branch (autonomous) constructs its own server with - // WriteOrigin::Agent and the read-before-write rail active. - if matches!(profile, BranchToolProfile::Default) { + // Skill tools by profile. Conversation branches carry User origin — the + // user is present and directing. A persistence pass with reflection on + // carries Agent origin: workspace-only writes, no installed or pinned + // targets, read-before-write, delete archives. Persistence passes + // without reflection (and ingestion) get no skill surface at all. + let skill_origin = match &profile { + BranchToolProfile::Default => Some(crate::skills::WriteOrigin::User), + BranchToolProfile::MemoryPersistence { + skill_reflection: true, + .. + } => Some(crate::skills::WriteOrigin::Agent), + BranchToolProfile::MemoryPersistence { .. } => None, + }; + if let Some(origin) = skill_origin { let skill_read_tracker = new_skill_read_tracker(); + let mut skill_manage = SkillManageTool::new(runtime_config.clone(), origin) + .with_read_tracker(skill_read_tracker.clone()); + if let BranchToolProfile::MemoryPersistence { + channel_id: Some(channel_id), + .. + } = &profile + { + skill_manage = skill_manage.with_conversation_id(channel_id.clone()); + } server = server - .tool( - ReadSkillTool::new(runtime_config.clone()) - .with_read_tracker(skill_read_tracker.clone()), - ) - .tool( - SkillManageTool::new(runtime_config.clone(), crate::skills::WriteOrigin::User) - .with_read_tracker(skill_read_tracker), - ) + .tool(ReadSkillTool::new(runtime_config.clone()).with_read_tracker(skill_read_tracker)) + .tool(skill_manage) .tool(SkillsListTool::new(runtime_config.clone())); } @@ -944,6 +958,7 @@ pub fn create_branch_tool_server( contract_state, working_memory, channel_id, + .. } = profile { let mut tool = MemoryPersistenceCompleteTool::new(contract_state);