diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 0cdbe9f489b9..3a2c9e7b9042 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -38,6 +38,7 @@ use crate::conversation::message::{ SystemNotificationType, ToolRequest, }; use crate::conversation::{debug_conversation_fix, fix_conversation, Conversation}; +use crate::hints::SubdirectoryHintTracker; use crate::mcp_utils::ToolResult; use crate::permission::permission_inspector::PermissionInspector; use crate::permission::permission_judge::PermissionCheckResult; @@ -245,6 +246,7 @@ pub struct Agent { pub(super) frontend_tools: Mutex>, pub(super) frontend_instructions: Mutex>, pub(super) prompt_manager: Mutex, + pub(super) subdirectory_hint_tracker: Mutex, pub tool_confirmation_router: ToolConfirmationRouter, pub(super) tool_result_tx: mpsc::Sender<(String, ToolResult)>, pub(super) tool_result_rx: ToolResultReceiver, @@ -371,6 +373,7 @@ impl Agent { frontend_tools: Mutex::new(HashMap::new()), frontend_instructions: Mutex::new(None), prompt_manager: Mutex::new(PromptManager::new()), + subdirectory_hint_tracker: Mutex::new(SubdirectoryHintTracker::new()), tool_confirmation_router: ToolConfirmationRouter::new(), tool_result_tx: tool_tx, tool_result_rx: Arc::new(Mutex::new(tool_rx)), @@ -1042,7 +1045,7 @@ impl Agent { }); tracing::Span::current().record("input", tracing::field::display(&input_summary)); - self.prompt_manager + self.subdirectory_hint_tracker .lock() .await .record_tool_arguments(&tool_call.arguments, &session.working_dir); @@ -2461,14 +2464,14 @@ impl Agent { } { - let has_new_hints = self - .prompt_manager + let hint_text = self + .subdirectory_hint_tracker .lock() .await - .load_subdirectory_hints(&working_dir); - if has_new_hints && !tools_updated { - (tools, toolshim_tools, system_prompt, _) = - self.prepare_tools_and_prompt(&session_config.id, &session.working_dir).await?; + .collect_new_hints(&working_dir); + if let Some(hints) = hint_text { + messages_to_add + .push(Message::user().with_text(hints).with_visibility(false, true)); } } diff --git a/crates/goose/src/agents/prompt_manager.rs b/crates/goose/src/agents/prompt_manager.rs index 810d7e6739d6..6aeee24adcee 100644 --- a/crates/goose/src/agents/prompt_manager.rs +++ b/crates/goose/src/agents/prompt_manager.rs @@ -1,6 +1,3 @@ -#[cfg(test)] -use chrono::DateTime; -use chrono::Utc; use indexmap::IndexMap; use serde::Serialize; use serde_json::Value; @@ -8,7 +5,7 @@ use std::collections::HashMap; use crate::agents::{extension::ExtensionInfo, moim}; use crate::hints::load_hints::build_gitignore; -use crate::hints::{get_context_filenames, load_hint_files, SubdirectoryHintTracker}; +use crate::hints::{get_context_filenames, load_hint_files}; use crate::{ config::{Config, GooseMode}, prompt_template, @@ -22,8 +19,6 @@ const MAX_TOOLS: usize = 50; pub struct PromptManager { system_prompt_override: Option, system_prompt_extras: IndexMap, - current_date_timestamp: String, - subdirectory_hint_tracker: SubdirectoryHintTracker, } impl Default for PromptManager { @@ -35,7 +30,6 @@ impl Default for PromptManager { #[derive(Serialize)] struct SystemPromptContext { extensions: Vec, - current_date_time: String, #[serde(skip_serializing_if = "Option::is_none")] extension_tool_limits: Option<(usize, usize)>, goose_mode: GooseMode, @@ -146,7 +140,6 @@ impl<'a> SystemPromptBuilder<'a, PromptManager> { let context = SystemPromptContext { extensions: sanitized_extensions_info, - current_date_time: self.manager.current_date_timestamp.clone(), extension_tool_limits, goose_mode, is_autonomous: goose_mode == GooseMode::Auto, @@ -204,20 +197,6 @@ impl PromptManager { PromptManager { system_prompt_override: None, system_prompt_extras: IndexMap::new(), - // Use the fixed current date time so that prompt cache can be used. - // Filtering to an hour to balance user time accuracy and multi session prompt cache hits. - current_date_timestamp: Utc::now().format("%Y-%m-%d %H:00").to_string(), - subdirectory_hint_tracker: SubdirectoryHintTracker::new(), - } - } - - #[cfg(test)] - pub fn with_timestamp(dt: DateTime) -> Self { - PromptManager { - system_prompt_override: None, - system_prompt_extras: IndexMap::new(), - current_date_timestamp: dt.format("%Y-%m-%d %H:%M:%S").to_string(), - subdirectory_hint_tracker: SubdirectoryHintTracker::new(), } } @@ -231,24 +210,6 @@ impl PromptManager { self.system_prompt_extras.shift_remove(key); } - pub fn record_tool_arguments( - &mut self, - arguments: &Option>, - working_dir: &Path, - ) { - self.subdirectory_hint_tracker - .record_tool_arguments(arguments, working_dir); - } - - pub fn load_subdirectory_hints(&mut self, working_dir: &Path) -> bool { - let new_hints = self.subdirectory_hint_tracker.load_new_hints(working_dir); - let has_new = !new_hints.is_empty(); - for (key, content) in new_hints { - self.system_prompt_extras.insert(key, content); - } - has_new - } - /// Override the system prompt with custom text pub fn set_system_prompt_override(&mut self, template: String) { self.system_prompt_override = Some(template); @@ -397,7 +358,7 @@ mod tests { #[test] fn test_basic() { - let manager = PromptManager::with_timestamp(DateTime::::from_timestamp(0, 0).unwrap()); + let manager = PromptManager::new(); let system_prompt = manager.builder().build(); @@ -406,7 +367,7 @@ mod tests { #[test] fn test_one_extension() { - let manager = PromptManager::with_timestamp(DateTime::::from_timestamp(0, 0).unwrap()); + let manager = PromptManager::new(); let system_prompt = manager .builder() @@ -422,7 +383,7 @@ mod tests { #[test] fn test_typical_setup() { - let manager = PromptManager::with_timestamp(DateTime::::from_timestamp(0, 0).unwrap()); + let manager = PromptManager::new(); let system_prompt = manager .builder() @@ -487,7 +448,7 @@ mod tests { extensions.sort_by(|a, b| a.name.cmp(&b.name)); - let manager = PromptManager::with_timestamp(DateTime::::from_timestamp(0, 0).unwrap()); + let manager = PromptManager::new(); let system_prompt = manager .builder() .with_extensions(extensions.into_iter()) diff --git a/crates/goose/src/hints/load_hints.rs b/crates/goose/src/hints/load_hints.rs index 6cf553cbe331..931daebba8ae 100644 --- a/crates/goose/src/hints/load_hints.rs +++ b/crates/goose/src/hints/load_hints.rs @@ -93,6 +93,24 @@ impl SubdirectoryHintTracker { } results } + + /// Returns hint text for directories newly touched since the last call, + /// joined into a single block, or None if nothing new was discovered. + /// Intended to be injected as an agent-visible tail message so the system + /// prompt stays stable. + pub fn collect_new_hints(&mut self, working_dir: &Path) -> Option { + let new_hints = self.load_new_hints(working_dir); + if new_hints.is_empty() { + return None; + } + Some( + new_hints + .into_iter() + .map(|(_, content)| content) + .collect::>() + .join("\n\n"), + ) + } } fn resolve_to_parent_dir(token: &str, working_dir: &Path) -> Option { diff --git a/crates/goose/tests/subdirectory_hints.rs b/crates/goose/tests/subdirectory_hints.rs new file mode 100644 index 000000000000..78495d249fca --- /dev/null +++ b/crates/goose/tests/subdirectory_hints.rs @@ -0,0 +1,215 @@ +use anyhow::Result; +use async_trait::async_trait; +use futures::StreamExt; +use goose::agents::{Agent, AgentConfig, AgentEvent, GoosePlatform, SessionConfig}; +use goose::config::permission::PermissionManager; +use goose::config::GooseMode; +use goose::conversation::message::{Message, MessageContent}; +use goose::providers::base::{ + stream_from_single_message, MessageStream, Provider, ProviderDef, ProviderMetadata, +}; +use goose::session::session_manager::SessionType; +use goose::session::SessionManager; +use goose_providers::conversation::token_usage::{ProviderUsage, Usage}; +use goose_providers::errors::ProviderError; +use goose_providers::model::ModelConfig; +use rmcp::model::{CallToolRequestParams, Tool}; +use rmcp::object; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; +use tempfile::TempDir; + +const SENTINEL: &str = "SENTINEL_SUBDIR_HINT_CONTENT"; + +/// Drives two tool calls that both touch `sub/`, then finishes with text. The +/// tool name is irrelevant: `record_tool_arguments` runs at the top of +/// `dispatch_tool_call`, before tool resolution, so an unresolved tool still +/// records the directory (and returns an error result quickly). Records how +/// many of its incoming requests already carried the injected hint. +struct ToolCallingProvider { + call_count: AtomicUsize, + requests_with_hint: AtomicUsize, +} + +impl ToolCallingProvider { + fn new() -> Self { + Self { + call_count: AtomicUsize::new(0), + requests_with_hint: AtomicUsize::new(0), + } + } +} + +#[async_trait] +impl Provider for ToolCallingProvider { + async fn stream( + &self, + _model_config: &ModelConfig, + _session_id: &str, + _system_prompt: &str, + messages: &[Message], + _tools: &[Tool], + ) -> Result { + let request_has_hint = messages.iter().any(|m| { + m.content + .iter() + .any(|c| matches!(c, MessageContent::Text(t) if t.text.contains(SENTINEL))) + }); + if request_has_hint { + self.requests_with_hint.fetch_add(1, Ordering::SeqCst); + } + + let call = self.call_count.fetch_add(1, Ordering::SeqCst); + let usage = ProviderUsage::new( + "mock-model".to_string(), + Usage::new(Some(10), Some(5), Some(15)), + ); + + // Calls 0 and 1 each touch `sub/` via a tool; call 2 ends with text. + let message = if call < 2 { + let path = if call == 0 { "sub/a.txt" } else { "sub/b.txt" }; + Message::assistant().with_tool_request( + format!("call_{call}"), + Ok(CallToolRequestParams::new("inspect").with_arguments(object!({ "path": path }))), + ) + } else { + Message::assistant().with_text("All done.") + }; + + Ok(stream_from_single_message(message, usage)) + } + + fn get_name(&self) -> &str { + "mock-tool-calling" + } +} + +impl goose::providers::base::ProviderDescriptor for ToolCallingProvider { + fn metadata() -> ProviderMetadata { + ProviderMetadata { + name: "mock".to_string(), + display_name: "Mock Tool Calling Provider".to_string(), + description: "Mock provider for subdirectory hint testing".to_string(), + default_model: "mock-model".to_string(), + known_models: vec![], + model_doc_link: "".to_string(), + config_keys: vec![], + setup_steps: vec![], + model_selection_hint: None, + fast_model: None, + } + } +} + +impl ProviderDef for ToolCallingProvider { + type Provider = Self; + + fn from_env( + _extensions: Vec, + _tls_config: Option, + ) -> futures::future::BoxFuture<'static, anyhow::Result> { + Box::pin(async { Ok(Self::new()) }) + } +} + +/// When tool calls touch a subdirectory containing `.goosehints`, the agent +/// injects those hints as an agent-only message in the live conversation +/// (reaching the in-flight turn, not just the session store) exactly once even +/// across repeated calls to the same directory. +#[tokio::test] +async fn subdirectory_hints_injected_once_agent_only() -> Result<()> { + let workdir = TempDir::new()?; + let sub = workdir.path().join("sub"); + std::fs::create_dir_all(&sub)?; + std::fs::write(sub.join(".goosehints"), SENTINEL)?; + + let data_dir = TempDir::new()?; + let session_manager = Arc::new(SessionManager::new(data_dir.path().to_path_buf())); + let config = AgentConfig::new( + session_manager.clone(), + PermissionManager::instance(), + None, + GooseMode::Auto, + true, // disable session naming so it doesn't consume a provider call + GoosePlatform::GooseCli, + ); + let agent = Agent::with_config(config); + + let session = session_manager + .create_session( + workdir.path().to_path_buf(), + "subdir-hints-test".to_string(), + SessionType::Hidden, + GooseMode::Auto, + ) + .await?; + + let provider = Arc::new(ToolCallingProvider::new()); + agent + .update_provider( + provider.clone(), + ModelConfig::new("mock-model"), + &session.id, + ) + .await?; + + let session_config = SessionConfig { + id: session.id.clone(), + schedule_id: None, + max_turns: Some(5), + retry_config: None, + }; + + let reply_stream = agent + .reply( + Message::user().with_text("Look at the files under sub/"), + session_config, + None, + ) + .await?; + tokio::pin!(reply_stream); + while let Some(event) = reply_stream.next().await { + match event { + Ok(AgentEvent::Message(_)) | Ok(_) => {} + Err(e) => return Err(e), + } + } + + let conversation = session_manager + .get_session(&session.id, true) + .await? + .conversation + .expect("session has a conversation"); + + let hint_messages: Vec<&Message> = conversation + .messages() + .iter() + .filter(|m| { + m.content + .iter() + .any(|c| matches!(c, MessageContent::Text(t) if t.text.contains(SENTINEL))) + }) + .collect(); + + assert_eq!( + hint_messages.len(), + 1, + "subdirectory hint should be injected exactly once across both tool calls to sub/, got {}", + hint_messages.len() + ); + + let hint = hint_messages[0]; + assert!(hint.is_agent_visible(), "hint must be visible to the agent"); + assert!( + !hint.is_user_visible(), + "hint must not be dumped into the user-visible transcript" + ); + + assert!( + provider.requests_with_hint.load(Ordering::SeqCst) >= 1, + "the injected hint must reach the live conversation (a later provider call must see it), \ + not just be written to the session store" + ); + + Ok(()) +}