Skip to content

fix(kv-cache): inject subdirectory hints as tail messages #9767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mcelrath wants to merge 1 commit into from
Closed

fix(kv-cache): inject subdirectory hints as tail messages #9767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions crates/goose/src/agents/agent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -245,6 +246,7 @@ pub struct Agent {
pub(super) frontend_tools: Mutex<HashMap<String, FrontendTool>>,
pub(super) frontend_instructions: Mutex<Option<String>>,
pub(super) prompt_manager: Mutex<PromptManager>,
pub(super) subdirectory_hint_tracker: Mutex<SubdirectoryHintTracker>,
pub tool_confirmation_router: ToolConfirmationRouter,
pub(super) tool_result_tx: mpsc::Sender<(String, ToolResult<CallToolResult>)>,
pub(super) tool_result_rx: ToolResultReceiver,
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -2461,14 +2464,14 @@ impl Agent {
}

{
let has_new_hints = self
.prompt_manager
let hint_text = self

@DOsinga DOsinga Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the right instinct -- these discovered hints should be part of the message that they discover, but I see some issues here:

  • this gets saved to the session manager, but not the current conversation, so it won't impact the current conversation until we reload the session
  • it gets added as user visible, to the user now sees a potentially very large AGENTS.md
  • it doesn't check whether we already have this AGENTS.md in the conversation. so the agent will add this multiple times, one for each toolcall

how did you test this? the stats on the PR suggests this all saves tokens, but if we hit this path multiple times, I am not sure it wouldl

.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));
}
}

Expand Down
49 changes: 5 additions & 44 deletions crates/goose/src/agents/prompt_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
#[cfg(test)]
use chrono::DateTime;
use chrono::Utc;
use indexmap::IndexMap;
use serde::Serialize;
use serde_json::Value;
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,
Expand All @@ -22,8 +19,6 @@ const MAX_TOOLS: usize = 50;
pub struct PromptManager {
system_prompt_override: Option<String>,
system_prompt_extras: IndexMap<String, String>,
current_date_timestamp: String,
subdirectory_hint_tracker: SubdirectoryHintTracker,
}

impl Default for PromptManager {
Expand All @@ -35,7 +30,6 @@ impl Default for PromptManager {
#[derive(Serialize)]
struct SystemPromptContext {
extensions: Vec<ExtensionInfo>,
current_date_time: String,
#[serde(skip_serializing_if = "Option::is_none")]
extension_tool_limits: Option<(usize, usize)>,
goose_mode: GooseMode,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<Utc>) -> 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(),
}
}

Expand All @@ -231,24 +210,6 @@ impl PromptManager {
self.system_prompt_extras.shift_remove(key);
}

pub fn record_tool_arguments(
&mut self,
arguments: &Option<serde_json::Map<String, serde_json::Value>>,
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);
Expand Down Expand Up @@ -397,7 +358,7 @@ mod tests {

#[test]
fn test_basic() {
let manager = PromptManager::with_timestamp(DateTime::<Utc>::from_timestamp(0, 0).unwrap());
let manager = PromptManager::new();

let system_prompt = manager.builder().build();

Expand All @@ -406,7 +367,7 @@ mod tests {

#[test]
fn test_one_extension() {
let manager = PromptManager::with_timestamp(DateTime::<Utc>::from_timestamp(0, 0).unwrap());
let manager = PromptManager::new();

let system_prompt = manager
.builder()
Expand All @@ -422,7 +383,7 @@ mod tests {

#[test]
fn test_typical_setup() {
let manager = PromptManager::with_timestamp(DateTime::<Utc>::from_timestamp(0, 0).unwrap());
let manager = PromptManager::new();

let system_prompt = manager
.builder()
Expand Down Expand Up @@ -487,7 +448,7 @@ mod tests {

extensions.sort_by(|a, b| a.name.cmp(&b.name));

let manager = PromptManager::with_timestamp(DateTime::<Utc>::from_timestamp(0, 0).unwrap());
let manager = PromptManager::new();
let system_prompt = manager
.builder()
.with_extensions(extensions.into_iter())
Expand Down
18 changes: 18 additions & 0 deletions crates/goose/src/hints/load_hints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {

@DOsinga DOsinga Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drops the subdir_hints:<dir> key and pushes the content as an anonymous user message, so there's no way to tell later which directories are already represented in the conversation. For the resume case we need the injected message to stay identifiable per directory — please preserve the key (e.g. as a marker line/prefix in the message, or a message-metadata field) so loaded_dirs can be reconstructed from history.

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::<Vec<_>>()
.join("\n\n"),
)
}
}

fn resolve_to_parent_dir(token: &str, working_dir: &Path) -> Option<PathBuf> {

@DOsinga DOsinga Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking nit: the dedup HashSet<PathBuf> compares paths lexically and this never canonicalizes, so sub/a.txt, ./sub/b.txt, and sub/../sub/c.txt all resolve to distinct keys for the same directory and each triggers a fresh injection. Since load_hints_from_directory already gates on is_dir(), canonicalizing the resolved dir before comparison (it exists on disk) would collapse these and also handle symlinks.

Expand Down
Loading
Loading