dynamic subdirectory hint loading#5759
dynamic subdirectory hint loading#5759johnmatthewtennant wants to merge 18 commits intoblock:mainfrom
Conversation
517a356 to
0e765ba
Compare
- Use existing turns_taken counter instead of separate turn_counter - Rename 'context' to 'hints' throughout (more consistent terminology) - Rename load_agents_from_directory to load_hints_from_directory - Add comment explaining relationship between hints and extras - Change to load hints hierarchically from working_dir to directory (not just single directory) Per review comments on PR block#5759 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
crates/goose/src/hints/load_hints.rs
Outdated
| use crate::hints::import_files::read_referenced_files; | ||
|
|
||
| /// Build a gitignore matcher for the given directory | ||
| pub fn build_gitignore(working_dir: &Path) -> Gitignore { |
There was a problem hiding this comment.
I don't think we want to take gitignore into account, there was a recent change to address that as it was incorrect
|
thanks @johnmatthewtennant a good start. Can you explain how this knows the event when it changes into a dir (ie as part of say some shell tool call?) - as I don't think at the platform level goose necessarily knows exactly when it changes dirs righ? (correct me if wrong). one simpler way (just a thought - not a deep thought): this could be local to the developer MCP as well, where its tools know what dir they are operating in, and can be aware of hints files and ensure they are loaded/unloaded there (or perhaps signalled back to the load hints platform tool?). So it is a little subtle, but maybe the solution isn't too hard (ie we could make the tools aware of any adjacent file, and ensure it is loaded always?) Just a thought - happy to explore more, but this is an important initiative, and I think really really needed for multi module and certainly monorepos, so please - do continue and reach out (discord if you can!) |
crates/goose/src/agents/agent.rs
Outdated
| pub const MANUAL_COMPACT_TRIGGER: &str = "Please compact this conversation"; | ||
|
|
||
| /// Maximum number of turns a directory hint can be idle before pruning | ||
| /// Can be overridden with GOOSE_DYNAMIC_SUBDIRECTORY_HINT_PRUNING_TURNS environment variable |
There was a problem hiding this comment.
I personally think we wouldn't make it configurable. Let's just decide on good default behavior. Three sounds fine to me.
crates/goose/src/agents/agent.rs
Outdated
| .iter() | ||
| .filter_map(|req| { | ||
| if let Ok(call) = &req.tool_call { | ||
| if call.name == "developer__text_editor" { |
There was a problem hiding this comment.
Part of me wonders if maybe the right way to do this is to expose an interface to let MCP servers conditionally propose additional hint dirs instead of hardcoding to look at tool call names.
Maybe MCP notifications from server to client?
If we had such a thing, developer could do its thing moving around working on files and constantly suggest to the agent to add and remove dirs (or maybe more generalized in suggesting to add content to the system prompt), but other servers could too.
@johnmatthewtennant @michaelneale Thoughts?
There was a problem hiding this comment.
I would be curious what claude code, codex do, gemini-cli - latter 2 are open source so if we peek at them, that is probably a reasonable pattern to follow. I think what is important is that it works how people expect, and I can appreciate that you would have hints at different levels (any non trivial app ends up using monorepo patterns) so I think is important we do something here - especially if people expect it to just work (ie we would have things in ui/desktop for goose which are differnet from AGENTS.md in the root? that isn't that surprising).
I think this approach has legs...
There was a problem hiding this comment.
having thought a bit more, I think it does make sense to load the local hints as best it can, it is what I would want to do. Especially when I look at large mono repos - would be the most scalable way to let large teams tweak things. If you end up in their module - their hints should apply for sure. I think same could apply to skills and other things which can be loaded.
|
@johnmatthewtennant still interested in this - I think the idea is sound, needs some updating to main though |
1778d89 to
4a06421
Compare
|
@michaelneale Sorry for the delay! I'm working on updating from main now |
0a811b5 to
273a82c
Compare
Implements dynamic loading of hint files from subdirectories when accessing files in new directories during agent execution. This allows hints to be loaded progressively as the agent navigates through different parts of a monorepo, rather than loading all hints upfront. Key features: - Load hints from directories dynamically when text_editor tool is used - Track loaded directories to avoid redundant loading - Prune idle directory contexts after configurable number of turns - Maintain session state for loaded hints across tool calls
…ctory hint loading Per Michael's comment on load_hints.rs:11, the new dynamic subdirectory hint loading feature should not use gitignore. This was based on the recent change in block#5688 that removed gitignore from the developer MCP. Changes: - Remove build_gitignore() function (NEW code) - Remove gitignore parameter from load_hints_from_directory() (NEW code) - Use empty gitignore in dynamic subdirectory loading to avoid filtering - Keep gitignore in existing load_hint_files() function (EXISTING code) - Keep gitignore in read_referenced_files() function (EXISTING code) - Inline gitignore building in prompt_manager.rs for existing startup hint loading This ensures that only the NEW dynamic subdirectory loading feature ignores gitignore, while the EXISTING startup hint loading continues to respect it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Marks test_hint_loading_security_guards and test_hint_loading_filesystem_updates as serial to prevent SQLite database lock errors when running in parallel in CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Apply AGENTS.md code quality rules: - Move 11 unit tests from inline modules to tests/ folder - Improve constant/variable naming for clarity - Downgrade routine info! logs to debug! level - Remove redundant comments that restate code All tests pass. No functional changes. Note: goose-self-test.yaml not updated as the feature already has comprehensive integration tests in dynamic_subdirectory_hints_test.rs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
273a82c to
1e7ad87
Compare
Restructure the build() method to maintain the original control flow, reducing unnecessary changes. Keeps the same two-step process: build the extras vec first, then sanitize - just extracting .content from the new SystemPromptExtra struct. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Remove comments that restate obvious code per AGENTS.md guidelines: - Remove "This happens on every agent iteration" comment - Remove "Check if directory exists" and "Check if file exists" comments - Remove redundant doc comments on helper methods Revert unnecessary refactoring of existing code: - Restore original loop-based implementation in get_local_directories - Restore original test code ordering These changes reduce the diff by ~24 lines while maintaining the same functionality and focusing the PR on new features rather than refactoring existing working code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Remove comments that restate function/field names per AGENTS.md: - Remove obvious field and method doc comments - Use unwrap_or_default() instead of unwrap_or_else for idiomaticity Reduces diff by 7 lines while keeping all useful documentation (security notes, return value explanations, behavioral context). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Remove 6 comments that restate what the code obviously does: - "Verify state/hints/loaded" comments before assertions - "Cleanup" comments before delete_session calls Keeps all useful comments that explain test structure, turn context, and expected behaviors per AGENTS.md guidelines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Remove 4 comments that restate what the code obviously does: - extension_data_test.rs: Remove "Verify context details", "Load directories at different turns", "Access auth at turn 8" - load_hints_test.rs: Remove "Create agents.md with import in subdir" Keeps all useful comments that explain test purpose, expected behaviors, and complex logic per AGENTS.md guidelines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This function is only used within the hints module, so there's no need to export it publicly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This method was only exposed for testing. The test now directly uses LoadedAgentsState.prune_stale() which is the actual logic being tested, reducing the public API surface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The test_hint_loading_and_pruning_integration test was missing the #[serial] attribute, causing SQLite database locking errors when running in parallel with other tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ae867b4 to
3b269fb
Compare
|
I've discovered that this feature is currently incompatible with the The dynamic hint loading triggers on const TOOLS_THAT_TRIGGER_HINT_LOADING: &[&str] = &["developer__text_editor"];However, when code_execution is enabled (now the default), all tool calls are wrapped. Instead of seeing: The agent sees: The file path is buried in JavaScript code, so the trigger never fires. I need to rethink how we hook into the tool call and capture the path |
|
gotcha - yeah underneath it is still calling the same dev extension, so there may be a way to trap it deeper. Also, I woudn't necessarily expect that we wont' have other tools as peers to code_execution - ie developer one, so it may still be worthwhile having that in there in the meantime? |
|
closing as this is a little old - please do re-open when ready |
Summary
Goose reads the hints in the working directory but does not read hints in subdirectories. This is problematic for working in monorepos.
E.g. If you run Goose from the repo-root folder, then Goose reads
repo-root/agents.mdon startup.If you direct goose to make changes to the frontend in
repo-root/frontend, Goose will start reading and editing files without readingrepo-root/frontend/agents.md.This PR adds logic to dynamically load hints in subdirectories when Goose reads files in those subdirectories.
It also maintains a turn counter and prunes the hints from the context when Goose hasn't accessed files in the subdirectory in the last N turns.
I'm new to the codebase and leaned heavily on AI to produce the draft PR so I'd like some help assessing if this is the right approach.
Type of Change
AI Assistance
Testing
Related Issues
Relates to #5840