From d4dacd62aea404a9a0ac1b5195627b834aca5234 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Tue, 19 May 2026 19:06:42 +0530 Subject: [PATCH] feat(learning): explicit user-preference tool + always-on pinned-facet prompt injection (#2150) Co-authored-by: Gray Cyrus --- .../agent/harness/session/builder.rs | 30 + src/openhuman/agent/harness/session/turn.rs | 81 ++- .../agent/harness/session/turn_tests.rs | 172 +++++ src/openhuman/agent/harness/session/types.rs | 5 + src/openhuman/config/schema/learning.rs | 14 + src/openhuman/config/schema/load.rs | 8 + src/openhuman/tools/impl/agent/mod.rs | 2 + .../tools/impl/agent/remember_preference.rs | 609 ++++++++++++++++++ src/openhuman/tools/ops.rs | 9 + 9 files changed, 929 insertions(+), 1 deletion(-) create mode 100644 src/openhuman/tools/impl/agent/remember_preference.rs diff --git a/src/openhuman/agent/harness/session/builder.rs b/src/openhuman/agent/harness/session/builder.rs index 55da8a666..ac4afcd97 100644 --- a/src/openhuman/agent/harness/session/builder.rs +++ b/src/openhuman/agent/harness/session/builder.rs @@ -79,6 +79,7 @@ impl AgentBuilder { auto_save: None, post_turn_hooks: Vec::new(), learning_enabled: false, + explicit_preferences_enabled: true, event_session_id: None, event_channel: None, agent_definition_name: None, @@ -206,6 +207,16 @@ impl AgentBuilder { self } + /// Enables or disables explicit-preference injection. + /// + /// When `true` (the default), preferences stored via `remember_preference` + /// are fetched from the `user_profile` namespace and injected into the + /// system prompt on every turn, independent of `learning_enabled`. + pub fn explicit_preferences_enabled(mut self, enabled: bool) -> Self { + self.explicit_preferences_enabled = enabled; + self + } + /// Sets the event-bus `session_id` and `channel` used to tag /// `DomainEvent`s emitted by this agent. /// @@ -494,6 +505,7 @@ impl AgentBuilder { last_tree_prefetch_at: None, post_turn_hooks: self.post_turn_hooks, learning_enabled: self.learning_enabled, + explicit_preferences_enabled: self.explicit_preferences_enabled, event_session_id: self .event_session_id .unwrap_or_else(|| "standalone".to_string()), @@ -1002,6 +1014,23 @@ impl Agent { ); } + // Explicit-preferences injection — independent of the full learning + // subsystem. When `explicit_preferences_enabled` is true (the default) + // and the full learning subsystem is NOT already wiring UserProfileSection, + // we add it here so pinned preferences written by `remember_preference` + // reach every session prompt. The `fetch_learned_context` gate is + // widened by `explicit_preferences_enabled` on the Agent (see + // `session/turn.rs`) so the data is actually fetched and populated. + if config.learning.explicit_preferences_enabled && !config.learning.enabled { + prompt_builder = prompt_builder.add_section(Box::new( + crate::openhuman::learning::UserProfileSection::new(memory.clone()), + )); + log::info!( + "[learning] explicit-preference UserProfileSection registered \ + (learning.enabled=false, explicit_preferences_enabled=true)" + ); + } + // (#623) Memory context for threads spawned from a subconscious // reflection: append the resolved `source_chunks` snapshot from // the reflection row as a `ReflectionMemoryContextSection`. The @@ -1462,6 +1491,7 @@ impl Agent { .auto_save(config.memory.auto_save) .post_turn_hooks(post_turn_hooks) .learning_enabled(config.learning.enabled) + .explicit_preferences_enabled(config.learning.explicit_preferences_enabled) .agent_definition_name(agent_id.to_string()) .omit_profile(effective_omit_profile) .omit_memory_md(effective_omit_memory_md); diff --git a/src/openhuman/agent/harness/session/turn.rs b/src/openhuman/agent/harness/session/turn.rs index cdf85ee81..282d2ad2b 100644 --- a/src/openhuman/agent/harness/session/turn.rs +++ b/src/openhuman/agent/harness/session/turn.rs @@ -1296,11 +1296,90 @@ impl Agent { /// /// This is an async, non-blocking operation that populates the context /// for the system prompt. + /// + /// # Explicit-preferences narrow path + /// + /// When `learning_enabled` is `false` but `explicit_preferences_enabled` + /// is `true`, only the `user_profile` namespace (pinned preferences from + /// the `remember_preference` tool) is fetched and returned. All other + /// inference-derived data (observations, patterns, reflections, tree + /// summaries) remains empty — the inference stack is not touched. pub(super) async fn fetch_learned_context(&self) -> LearnedContextData { - if !self.learning_enabled { + // Fast path: neither the full learning subsystem nor the explicit + // preferences path is active — skip all memory reads. + if !self.learning_enabled && !self.explicit_preferences_enabled { + tracing::debug!( + "[learning] fetch_learned_context: both learning_enabled and \ + explicit_preferences_enabled are false — returning empty context" + ); return LearnedContextData::default(); } + // Narrow explicit-preferences path: only fetch pinned user_profile + // entries; skip all inference-derived data. + if !self.learning_enabled && self.explicit_preferences_enabled { + tracing::debug!( + "[learning] fetch_learned_context: explicit_preferences_enabled=true, \ + learning_enabled=false — fetching only pinned user_profile entries" + ); + let profile_entries = self + .memory + .list( + Some("user_profile"), + // Core category is used by RememberPreferenceTool for pinned entries. + // We list without category filter so we pick up both Core entries + // (pinned) and any Custom("user_profile") entries from the older + // UserProfileHook code path, keeping this backward-compatible. + None, + None, + ) + .await + .unwrap_or_default(); + + // `.list()` already scopes to the `user_profile` namespace at the + // store layer (via the `Some("user_profile")` argument above). This + // `.filter()` is a defensive guard against any future store-layer + // change that might weaken that scoping — it is not load-bearing + // under the current implementation. + if profile_entries.len() > 50 { + tracing::warn!( + total = profile_entries.len(), + dropped = profile_entries.len() - 50, + "[learning] user_profile pinned preferences exceed prompt cap of 50; \ + {} entries will be dropped from this turn's context", + profile_entries.len() - 50, + ); + } + let user_profile: Vec = profile_entries + .iter() + .filter(|e| { + e.namespace + .as_deref() + .map_or(false, |ns| ns == "user_profile") + }) + .take(50) + .map(|e| sanitize_learned_entry(&e.content)) + .collect(); + + tracing::debug!( + "[learning] fetch_learned_context: fetched {} pinned user_profile entries", + user_profile.len() + ); + + return LearnedContextData { + observations: Vec::new(), + patterns: Vec::new(), + user_profile, + reflections: Vec::new(), + tree_root_summaries: Vec::new(), + }; + } + + // Full learning path: fetch all inference-derived data. + tracing::debug!( + "[learning] fetch_learned_context: learning_enabled=true — fetching full context" + ); + let obs_entries = self .memory .list( diff --git a/src/openhuman/agent/harness/session/turn_tests.rs b/src/openhuman/agent/harness/session/turn_tests.rs index e39f94639..087ddf3e3 100644 --- a/src/openhuman/agent/harness/session/turn_tests.rs +++ b/src/openhuman/agent/harness/session/turn_tests.rs @@ -544,3 +544,175 @@ async fn execute_tool_call_applies_inline_result_budget() { assert!(result.output.contains("truncated by tool_result_budget")); assert!(record.output_summary.starts_with("long: ok (")); } + +// ── Explicit-preferences narrow path ────────────────────────────────────────── +// +// These tests verify that `fetch_learned_context` correctly handles the three +// flag combinations: +// 1. both flags off → empty context +// 2. explicit_preferences_enabled=true, learning_enabled=false +// → only pinned user_profile entries returned, no inference data +// 3. learning_enabled=true → full path (existing tests cover this; we only +// verify that explicit entries are included as well) +// +// We use the real `UnifiedMemory` backend (sqlite) so the list/store round-trip +// is exercised end-to-end without mocking the memory layer. + +fn make_agent_with_memory( + memory: Arc, + workspace_dir: std::path::PathBuf, + learning_enabled: bool, + explicit_preferences_enabled: bool, +) -> Agent { + Agent::builder() + .provider(Box::new(DummyProvider)) + .tools(vec![]) + .memory(memory) + .tool_dispatcher(Box::new(XmlToolDispatcher)) + .workspace_dir(workspace_dir) + .event_context("pref-test-session", "pref-test-channel") + .learning_enabled(learning_enabled) + .explicit_preferences_enabled(explicit_preferences_enabled) + .build() + .unwrap() +} + +fn make_real_memory(workspace: &std::path::Path) -> Arc { + use crate::openhuman::embeddings::NoopEmbedding; + use crate::openhuman::memory::UnifiedMemory; + Arc::new(UnifiedMemory::new(workspace, Arc::new(NoopEmbedding), None).unwrap()) +} + +#[tokio::test] +async fn fetch_learned_context_returns_empty_when_both_flags_off() { + let tmp = tempfile::TempDir::new().unwrap(); + let mem = make_real_memory(tmp.path()); + + // Store a pinned preference so we can verify it is NOT returned. + mem.store( + "user_profile", + "pinned/tooling/package_manager", + "[pinned] (class=tooling) package_manager: pnpm", + crate::openhuman::memory::MemoryCategory::Core, + None, + ) + .await + .unwrap(); + + let agent = make_agent_with_memory( + mem, + tmp.path().to_path_buf(), + false, // learning_enabled + false, // explicit_preferences_enabled + ); + + let learned = agent.fetch_learned_context().await; + + assert!( + learned.user_profile.is_empty(), + "both flags off: user_profile must be empty, got {:?}", + learned.user_profile + ); + assert!(learned.observations.is_empty()); + assert!(learned.patterns.is_empty()); + assert!(learned.reflections.is_empty()); +} + +#[tokio::test] +async fn fetch_learned_context_returns_pinned_prefs_when_explicit_flag_on_learning_off() { + let tmp = tempfile::TempDir::new().unwrap(); + let mem = make_real_memory(tmp.path()); + + // Store two pinned preferences via the same key format RememberPreferenceTool uses. + mem.store( + "user_profile", + "pinned/tooling/package_manager", + "[pinned] (class=tooling) package_manager: pnpm", + crate::openhuman::memory::MemoryCategory::Core, + None, + ) + .await + .unwrap(); + mem.store( + "user_profile", + "pinned/style/verbosity", + "[pinned] (class=style) verbosity: terse", + crate::openhuman::memory::MemoryCategory::Core, + None, + ) + .await + .unwrap(); + + let agent = make_agent_with_memory( + mem, + tmp.path().to_path_buf(), + false, // learning_enabled — full inference stack OFF + true, // explicit_preferences_enabled — narrow path ON + ); + + let learned = agent.fetch_learned_context().await; + + assert_eq!( + learned.user_profile.len(), + 2, + "explicit flag on, learning off: expected 2 pinned preferences, got: {:?}", + learned.user_profile + ); + assert!( + learned + .user_profile + .iter() + .any(|s| s.contains("package_manager")), + "package_manager preference must appear in user_profile: {:?}", + learned.user_profile + ); + assert!( + learned.user_profile.iter().any(|s| s.contains("verbosity")), + "verbosity preference must appear in user_profile: {:?}", + learned.user_profile + ); + // Inference-derived data must remain empty — the stack was NOT engaged. + assert!( + learned.observations.is_empty(), + "observations must be empty when learning_enabled=false" + ); + assert!( + learned.patterns.is_empty(), + "patterns must be empty when learning_enabled=false" + ); + assert!( + learned.reflections.is_empty(), + "reflections must be empty when learning_enabled=false" + ); +} + +#[tokio::test] +async fn fetch_learned_context_explicit_flag_off_learning_off_returns_empty_even_with_stored_prefs() +{ + let tmp = tempfile::TempDir::new().unwrap(); + let mem = make_real_memory(tmp.path()); + + mem.store( + "user_profile", + "pinned/style/tone", + "[pinned] (class=style) tone: formal", + crate::openhuman::memory::MemoryCategory::Core, + None, + ) + .await + .unwrap(); + + let agent = make_agent_with_memory( + mem, + tmp.path().to_path_buf(), + false, // learning_enabled + false, // explicit_preferences_enabled — both off + ); + + let learned = agent.fetch_learned_context().await; + assert!( + learned.user_profile.is_empty(), + "both flags off: user_profile must be empty even when prefs exist, got: {:?}", + learned.user_profile + ); +} diff --git a/src/openhuman/agent/harness/session/types.rs b/src/openhuman/agent/harness/session/types.rs index 380a7b601..a85dc7ef9 100644 --- a/src/openhuman/agent/harness/session/types.rs +++ b/src/openhuman/agent/harness/session/types.rs @@ -67,6 +67,10 @@ pub struct Agent { pub(super) last_tree_prefetch_at: Option, pub(super) post_turn_hooks: Vec>, pub(super) learning_enabled: bool, + /// When `true`, pinned preferences stored via `remember_preference` are + /// fetched from the `user_profile` namespace and injected into the system + /// prompt on every turn, independent of `learning_enabled`. + pub(super) explicit_preferences_enabled: bool, pub(super) event_session_id: String, pub(super) event_channel: String, /// Human-readable agent definition name (e.g. `"main"`, @@ -206,6 +210,7 @@ pub struct AgentBuilder { pub(super) auto_save: Option, pub(super) post_turn_hooks: Vec>, pub(super) learning_enabled: bool, + pub(super) explicit_preferences_enabled: bool, pub(super) event_session_id: Option, pub(super) event_channel: Option, pub(super) agent_definition_name: Option, diff --git a/src/openhuman/config/schema/learning.rs b/src/openhuman/config/schema/learning.rs index 7fe614c0b..9f11236f7 100644 --- a/src/openhuman/config/schema/learning.rs +++ b/src/openhuman/config/schema/learning.rs @@ -128,6 +128,19 @@ pub struct LearningConfig { /// How often the periodic rebuild loop runs in seconds. Default: 1800 (30 minutes). #[serde(default = "default_rebuild_interval_secs")] pub rebuild_interval_secs: u64, + + /// Enable explicit user-preference injection into the system prompt. + /// + /// When `true` (the default), preferences saved via the `remember_preference` + /// tool are injected into every session prompt regardless of whether the full + /// inference-based learning subsystem (`enabled`) is on. This is the + /// narrow, always-on path for user-authoritative pinned preferences — + /// no reflection, no heuristics, no stability engine. + /// + /// Explicitly set to `false` (or `OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED=0`) + /// to suppress all preference injection even for explicitly pinned entries. + #[serde(default = "default_true")] + pub explicit_preferences_enabled: bool, } fn default_rebuild_interval_secs() -> u64 { @@ -163,6 +176,7 @@ impl Default for LearningConfig { episodic_capture_enabled: default_true(), stm_recall_enabled: default_true(), unified_compaction_enabled: default_true(), + explicit_preferences_enabled: default_true(), } } } diff --git a/src/openhuman/config/schema/load.rs b/src/openhuman/config/schema/load.rs index 1295f8ddc..3b7feb64f 100644 --- a/src/openhuman/config/schema/load.rs +++ b/src/openhuman/config/schema/load.rs @@ -1279,6 +1279,14 @@ impl Config { self.learning.tool_memory_capture_enabled = enabled; } } + if let Some(flag) = env.get("OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED") { + let normalized = flag.trim().to_ascii_lowercase(); + match normalized.as_str() { + "1" | "true" | "yes" | "on" => self.learning.explicit_preferences_enabled = true, + "0" | "false" | "no" | "off" => self.learning.explicit_preferences_enabled = false, + _ => {} + } + } if let Some(source) = env.get("OPENHUMAN_LEARNING_REFLECTION_SOURCE") { let normalized = source.trim().to_ascii_lowercase(); match normalized.as_str() { diff --git a/src/openhuman/tools/impl/agent/mod.rs b/src/openhuman/tools/impl/agent/mod.rs index 67c776c53..52d0b71f5 100644 --- a/src/openhuman/tools/impl/agent/mod.rs +++ b/src/openhuman/tools/impl/agent/mod.rs @@ -6,6 +6,7 @@ mod delegate; mod dispatch; pub(crate) mod onboarding_status; mod plan_exit; +pub mod remember_preference; mod skill_delegation; mod spawn_parallel_agents; mod spawn_subagent; @@ -20,6 +21,7 @@ pub use check_onboarding_status::CheckOnboardingStatusTool; pub use complete_onboarding::CompleteOnboardingTool; pub use delegate::DelegateTool; pub use plan_exit::{PlanExitTool, PLAN_EXIT_MARKER}; +pub use remember_preference::RememberPreferenceTool; pub use skill_delegation::SkillDelegationTool; pub use spawn_parallel_agents::SpawnParallelAgentsTool; pub use spawn_subagent::SpawnSubagentTool; diff --git a/src/openhuman/tools/impl/agent/remember_preference.rs b/src/openhuman/tools/impl/agent/remember_preference.rs new file mode 100644 index 000000000..dc00bbb13 --- /dev/null +++ b/src/openhuman/tools/impl/agent/remember_preference.rs @@ -0,0 +1,609 @@ +//! Tool: remember_preference — deterministically pin an explicit user preference. +//! +//! Unlike the inference-based `UserProfileHook`, this tool is called by the +//! model when the user **explicitly** states or requests that a preference be +//! saved ("I prefer pnpm", "always be terse", "remember my timezone is IST"). +//! The model supplies the structured `(class, key, value)` triple; the tool +//! writes it directly to the `user_profile` memory namespace as a pinned entry. +//! +//! # Storage contract +//! +//! Entries are keyed as `pinned/{class}/{key}` inside the `user_profile` +//! namespace, and the stored content is formatted as: +//! +//! ```text +//! [pinned] (class=tooling) package_manager: pnpm +//! ``` +//! +//! This format is intentionally stable so `fetch_learned_context` can surface +//! the entries unchanged in the `UserProfileSection` prompt block. The +//! `[pinned]` marker makes pinned entries visually distinct in the rendered +//! prompt. +//! +//! # Idempotency +//! +//! `Memory::store` performs an upsert (the underlying SQLite backend has a +//! `UNIQUE` constraint on `(namespace, key)` with `ON CONFLICT REPLACE`). +//! Re-saving the same `(class, key)` with a new value overwrites the previous +//! entry — no duplicates are created. +//! +//! # Bypassing the inference stack +//! +//! This tool does **not** touch the stability detector, the candidate +//! ring-buffer, `learning/extract/heuristics.rs`, or any other inference +//! component. The preference is authoritative from the moment the tool +//! returns `Ok`. + +use crate::openhuman::memory::{Memory, MemoryCategory}; +use crate::openhuman::security::policy::ToolOperation; +use crate::openhuman::security::SecurityPolicy; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; +use async_trait::async_trait; +use serde_json::json; +use std::sync::Arc; + +/// Valid facet classes that a pinned preference may belong to. +/// +/// The six classes provide a lightweight taxonomy that lets the system-prompt +/// renderer group or filter preferences in the future without requiring a +/// schema migration. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FacetClass { + /// Communication style (tone, verbosity, format). + Style, + /// Personal identity facts (name, role, pronouns). + Identity, + /// Toolchain choices (language, package manager, editor). + Tooling, + /// Hard vetoes — things the model must never do. + Veto, + /// Long-term goals and working objectives. + Goal, + /// Channel / communication-medium preferences. + Channel, +} + +impl FacetClass { + /// Parse a case-insensitive string from the model's `class` argument. + pub fn parse(s: &str) -> Option { + match s.trim().to_ascii_lowercase().as_str() { + "style" => Some(Self::Style), + "identity" => Some(Self::Identity), + "tooling" => Some(Self::Tooling), + "veto" => Some(Self::Veto), + "goal" => Some(Self::Goal), + "channel" => Some(Self::Channel), + _ => None, + } + } + + pub fn as_str(self) -> &'static str { + match self { + Self::Style => "style", + Self::Identity => "identity", + Self::Tooling => "tooling", + Self::Veto => "veto", + Self::Goal => "goal", + Self::Channel => "channel", + } + } +} + +/// The namespace used for all pinned preferences in the memory backend. +pub const PINNED_PREFERENCES_NAMESPACE: &str = "user_profile"; + +/// Builds the memory key for a pinned preference. +/// +/// Format: `pinned/{class}/{key}`, e.g. `pinned/tooling/package_manager`. +pub fn pinned_key(class: FacetClass, key: &str) -> String { + format!("pinned/{}/{}", class.as_str(), key) +} + +/// Builds the stored content string for a pinned preference. +/// +/// Format: `[pinned] (class=tooling) package_manager: pnpm` +pub fn pinned_content(class: FacetClass, key: &str, value: &str) -> String { + format!("[pinned] (class={}) {}: {}", class.as_str(), key, value) +} + +/// Agent tool that explicitly pins a user preference into the `user_profile` +/// memory namespace. +/// +/// The model calls this when the user states or requests a preference be +/// remembered. All arguments (`class`, `key`, `value`) are supplied by the +/// model — it maps the user's natural-language intent to the structured triple. +pub struct RememberPreferenceTool { + memory: Arc, + security: Arc, +} + +impl RememberPreferenceTool { + pub fn new(memory: Arc, security: Arc) -> Self { + Self { memory, security } + } +} + +#[async_trait] +impl Tool for RememberPreferenceTool { + fn name(&self) -> &str { + "remember_preference" + } + + fn description(&self) -> &str { + "Pin an explicit user preference so it persists across all future sessions. \ + Call this when the user states or asks to save a preference — e.g. \ + \"I prefer pnpm\", \"always be terse\", \"never email Sarah\", \ + \"remember my timezone is IST\". \ + Map the user's intent to a `class` (one of: style, identity, tooling, veto, goal, channel), \ + a snake_case `key` (e.g. package_manager, verbosity, timezone), \ + and a concise `value` string. \ + Re-saving the same class+key overwrites the previous value — no duplicates are created." + } + + fn parameters_schema(&self) -> serde_json::Value { + json!({ + "type": "object", + "required": ["class", "key", "value"], + "properties": { + "class": { + "type": "string", + "enum": ["style", "identity", "tooling", "veto", "goal", "channel"], + "description": "Facet class — style (tone/format), identity (name/role), \ + tooling (language/editor/package manager), \ + veto (hard do-not-do), goal (objective), \ + channel (communication medium preference)." + }, + "key": { + "type": "string", + "description": "Snake_case slug that uniquely names this preference within its class, \ + e.g. package_manager, verbosity, timezone, preferred_language. \ + Must contain only lowercase letters, digits, and underscores." + }, + "value": { + "type": "string", + "description": "The preference value, e.g. pnpm, terse, IST, Rust." + } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + tracing::debug!( + "[tool][remember_preference] invoked with args: class={:?} key={:?} value_present={} value_len={}", + args.get("class").and_then(|v| v.as_str()), + args.get("key").and_then(|v| v.as_str()), + args.get("value").is_some(), + args.get("value") + .and_then(|v| v.as_str()) + .map_or(0, |s| s.len()), + ); + + // Security gate — tool requires Write-level autonomy. + if let Err(error) = self + .security + .enforce_tool_operation(ToolOperation::Act, "remember_preference") + { + tracing::warn!("[tool][remember_preference] security gate rejected: {error}"); + return Ok(ToolResult::error(error)); + } + + // Parse class. + let class_str = match args.get("class").and_then(|v| v.as_str()) { + Some(s) => s, + None => { + return Ok(ToolResult::error( + "missing required argument: class".to_string(), + )); + } + }; + let class = match FacetClass::parse(class_str) { + Some(c) => c, + None => { + tracing::warn!( + "[tool][remember_preference] invalid class={:?}; valid values: \ + style, identity, tooling, veto, goal, channel", + class_str + ); + return Ok(ToolResult::error(format!( + "invalid class {:?}; must be one of: style, identity, tooling, veto, goal, channel", + class_str + ))); + } + }; + + // Parse key — must be a non-empty, snake_case slug. + let key = match args.get("key").and_then(|v| v.as_str()) { + Some(k) => k.trim(), + None => { + return Ok(ToolResult::error( + "missing required argument: key".to_string(), + )); + } + }; + if key.is_empty() { + return Ok(ToolResult::error("key cannot be empty".to_string())); + } + if !key + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_') + { + tracing::warn!( + "[tool][remember_preference] key {:?} contains invalid characters; \ + only lowercase letters, digits, and underscores are allowed (snake_case)", + key + ); + return Ok(ToolResult::error(format!( + "key {:?} contains invalid characters; use only lowercase letters, digits, and underscores (snake_case)", + key + ))); + } + + // Parse value — normalize to a single line so that embedded \r/\n cannot + // corrupt the line-oriented `[pinned] … key: value` storage format. + let value_raw = match args.get("value").and_then(|v| v.as_str()) { + Some(v) => v, + None => { + return Ok(ToolResult::error( + "missing required argument: value".to_string(), + )); + } + }; + // Collapse any embedded CR/LF to a single space, then trim surrounding + // whitespace so the stored and pinned representations are always one line. + let value_normalized: String = value_raw + .replace(['\r', '\n'], " ") + .split_whitespace() + .collect::>() + .join(" "); + let value = value_normalized.as_str(); + if value.is_empty() { + return Ok(ToolResult::error("value cannot be empty".to_string())); + } + + let mem_key = pinned_key(class, key); + let content = pinned_content(class, key, value); + + tracing::debug!( + "[tool][remember_preference] upserting pinned preference: namespace={} key={} class={} value_len={}", + PINNED_PREFERENCES_NAMESPACE, + mem_key, + class.as_str(), + value.len() + ); + + match self + .memory + .store( + PINNED_PREFERENCES_NAMESPACE, + &mem_key, + &content, + // Core category — pinned preferences are permanent user facts. + MemoryCategory::Core, + None, + ) + .await + { + Ok(()) => { + tracing::info!( + "[tool][remember_preference] pinned preference stored: \ + namespace={} key={} class={} value_len={}", + PINNED_PREFERENCES_NAMESPACE, + mem_key, + class.as_str(), + value.len() + ); + Ok(ToolResult::success(format!( + "Preference saved: [{class}] {key} = {value}", + class = class.as_str() + ))) + } + Err(e) => { + tracing::error!( + "[tool][remember_preference] failed to store preference \ + namespace={} key={}: {e:#}", + PINNED_PREFERENCES_NAMESPACE, + mem_key + ); + Ok(ToolResult::error(format!("Failed to save preference: {e}"))) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::embeddings::NoopEmbedding; + use crate::openhuman::memory::UnifiedMemory; + use crate::openhuman::security::{AutonomyLevel, SecurityPolicy}; + use serde_json::json; + use tempfile::TempDir; + + fn test_security() -> Arc { + Arc::new(SecurityPolicy::default()) + } + + fn test_mem() -> (TempDir, Arc) { + let tmp = TempDir::new().unwrap(); + let mem = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + (tmp, Arc::new(mem)) + } + + // ── FacetClass ───────────────────────────────────────────────────────── + + #[test] + fn facet_class_parse_case_insensitive() { + assert_eq!(FacetClass::parse("Style"), Some(FacetClass::Style)); + assert_eq!(FacetClass::parse("IDENTITY"), Some(FacetClass::Identity)); + assert_eq!(FacetClass::parse("tooling"), Some(FacetClass::Tooling)); + assert_eq!(FacetClass::parse("veto"), Some(FacetClass::Veto)); + assert_eq!(FacetClass::parse("goal"), Some(FacetClass::Goal)); + assert_eq!(FacetClass::parse("channel"), Some(FacetClass::Channel)); + assert_eq!(FacetClass::parse("unknown"), None); + assert_eq!(FacetClass::parse(""), None); + } + + #[test] + fn facet_class_as_str_round_trips() { + for class in [ + FacetClass::Style, + FacetClass::Identity, + FacetClass::Tooling, + FacetClass::Veto, + FacetClass::Goal, + FacetClass::Channel, + ] { + let parsed = FacetClass::parse(class.as_str()).expect("round-trip must succeed"); + assert_eq!(parsed, class); + } + } + + // ── Key / content helpers ─────────────────────────────────────────────── + + #[test] + fn pinned_key_format() { + assert_eq!( + pinned_key(FacetClass::Tooling, "package_manager"), + "pinned/tooling/package_manager" + ); + assert_eq!( + pinned_key(FacetClass::Style, "verbosity"), + "pinned/style/verbosity" + ); + } + + #[test] + fn pinned_content_format() { + assert_eq!( + pinned_content(FacetClass::Tooling, "package_manager", "pnpm"), + "[pinned] (class=tooling) package_manager: pnpm" + ); + } + + // ── Tool metadata ─────────────────────────────────────────────────────── + + #[test] + fn tool_name_and_permission() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem, test_security()); + assert_eq!(tool.name(), "remember_preference"); + assert_eq!(tool.permission_level(), PermissionLevel::Write); + } + + #[test] + fn schema_has_required_fields() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem, test_security()); + let schema = tool.parameters_schema(); + assert_eq!(schema["type"], "object"); + let required = schema["required"].as_array().unwrap(); + let names: Vec<&str> = required.iter().filter_map(|v| v.as_str()).collect(); + assert!(names.contains(&"class")); + assert!(names.contains(&"key")); + assert!(names.contains(&"value")); + } + + // ── Argument validation ───────────────────────────────────────────────── + + #[tokio::test] + async fn missing_class_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem, test_security()); + let result = tool + .execute(json!({"key": "timezone", "value": "IST"})) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("class")); + } + + #[tokio::test] + async fn invalid_class_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem, test_security()); + let result = tool + .execute(json!({"class": "bogus", "key": "timezone", "value": "IST"})) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("invalid class")); + } + + #[tokio::test] + async fn missing_key_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem, test_security()); + let result = tool + .execute(json!({"class": "style", "value": "terse"})) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("key")); + } + + #[tokio::test] + async fn empty_key_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem, test_security()); + let result = tool + .execute(json!({"class": "style", "key": " ", "value": "terse"})) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("key cannot be empty")); + } + + #[tokio::test] + async fn key_with_spaces_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem, test_security()); + let result = tool + .execute(json!({"class": "style", "key": "my pref", "value": "terse"})) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("invalid characters")); + } + + #[tokio::test] + async fn missing_value_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem, test_security()); + let result = tool + .execute(json!({"class": "tooling", "key": "pkg_mgr"})) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("value")); + } + + // ── Successful upsert ─────────────────────────────────────────────────── + + #[tokio::test] + async fn stores_preference_in_user_profile_namespace() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem.clone(), test_security()); + let result = tool + .execute(json!({"class": "tooling", "key": "package_manager", "value": "pnpm"})) + .await + .unwrap(); + assert!(!result.is_error, "unexpected error: {}", result.output()); + assert!(result.output().contains("package_manager")); + + let entry = mem + .get( + PINNED_PREFERENCES_NAMESPACE, + "pinned/tooling/package_manager", + ) + .await + .unwrap(); + assert!(entry.is_some(), "entry must have been stored"); + let entry = entry.unwrap(); + assert_eq!( + entry.content, + "[pinned] (class=tooling) package_manager: pnpm" + ); + assert_eq!(entry.category, MemoryCategory::Core); + } + + #[tokio::test] + async fn idempotent_overwrite_does_not_create_duplicate() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem.clone(), test_security()); + + // First write. + tool.execute(json!({"class": "style", "key": "verbosity", "value": "verbose"})) + .await + .unwrap(); + + // Overwrite with new value. + let result = tool + .execute(json!({"class": "style", "key": "verbosity", "value": "terse"})) + .await + .unwrap(); + assert!( + !result.is_error, + "overwrite must succeed: {}", + result.output() + ); + + // Verify the overwritten content via get() which reads the actual content column. + let entry = mem + .get(PINNED_PREFERENCES_NAMESPACE, "pinned/style/verbosity") + .await + .unwrap() + .expect("entry must exist after overwrite"); + assert_eq!( + entry.content, "[pinned] (class=style) verbosity: terse", + "overwritten content must reflect the latest value" + ); + + // Verify no duplicate entries exist via list(). + let all_entries = mem + .list(Some(PINNED_PREFERENCES_NAMESPACE), None, None) + .await + .unwrap(); + let verbosity_entries: Vec<_> = all_entries + .iter() + .filter(|e| e.key == "pinned/style/verbosity") + .collect(); + assert_eq!(verbosity_entries.len(), 1, "must not duplicate entries"); + } + + #[tokio::test] + async fn stores_all_six_classes() { + let (_tmp, mem) = test_mem(); + let tool = RememberPreferenceTool::new(mem.clone(), test_security()); + + for (class, key, value) in [ + ("style", "tone", "formal"), + ("identity", "name", "Alice"), + ("tooling", "editor", "neovim"), + ("veto", "no_emoji", "true"), + ("goal", "ship_feature", "memory refactor"), + ("channel", "preferred", "slack"), + ] { + let result = tool + .execute(json!({"class": class, "key": key, "value": value})) + .await + .unwrap(); + assert!( + !result.is_error, + "class={class} failed: {}", + result.output() + ); + } + + let entries = mem + .list(Some(PINNED_PREFERENCES_NAMESPACE), None, None) + .await + .unwrap(); + assert_eq!(entries.len(), 6); + } + + // ── Security gate ─────────────────────────────────────────────────────── + + #[tokio::test] + async fn blocked_in_readonly_mode() { + let (_tmp, mem) = test_mem(); + let readonly = Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + ..SecurityPolicy::default() + }); + let tool = RememberPreferenceTool::new(mem.clone(), readonly); + let result = tool + .execute(json!({"class": "style", "key": "tone", "value": "formal"})) + .await + .unwrap(); + assert!(result.is_error); + assert!(mem + .get(PINNED_PREFERENCES_NAMESPACE, "pinned/style/tone") + .await + .unwrap() + .is_none()); + } +} diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 9aafb2662..f5894d1cd 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -136,6 +136,15 @@ pub fn all_tools_with_runtime( Box::new(MemoryRecallTool::new(memory.clone())), Box::new(MemoryForgetTool::new(memory.clone(), security.clone())), Box::new(MemoryTreeTool), + // Explicit user-preference pinning — always registered so the model + // can save user-stated preferences regardless of whether the full + // inference-based learning subsystem is enabled. The preference + // injection into the system prompt is controlled independently by + // `config.learning.explicit_preferences_enabled`. + Box::new(RememberPreferenceTool::new( + memory.clone(), + security.clone(), + )), // WhatsApp data store — read-only agent surface (issue #1341). // The matching `whatsapp_data_ingest` write-path stays internal-only // (registered in `src/core/all.rs::build_internal_only_controllers`)