-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: move goosehints/AGENTS.md handling to goose, and out of developer extension #5575
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the hints/goosehints loading functionality by moving it from the goose-mcp crate to the main goose crate, making it available for use in the core agent system. The hints are now loaded dynamically during agent reply preparation rather than being included in MCP server initialization.
Key changes:
- Moved hints module from
goose-mcp/src/developer/goose_hintstogoose/src/hints - Updated to use
Paths::in_config_dir()for consistent path resolution instead ofshellexpand::tilde - Integrated hints loading into
Agent::prepare_tools_and_prompt()based on working directory - Updated hint message text to be more user-centric
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/goose/src/lib.rs | Adds public hints module to the goose crate |
| crates/goose/src/hints/mod.rs | New module definition exposing hints functionality |
| crates/goose/src/hints/load_hints.rs | Moved from goose-mcp, updated to use Paths API for config directory resolution |
| crates/goose/src/hints/import_files.rs | Moved from goose-mcp, handles file reference parsing and expansion in hint files |
| crates/goose/src/agents/reply_parts.rs | Updated to accept working_dir parameter and load hints during prompt preparation |
| crates/goose/src/agents/agent.rs | Threads working_dir through to prepare_tools_and_prompt() |
| crates/goose/Cargo.toml | Adds ignore crate dependency for gitignore pattern handling |
| crates/goose-mcp/src/developer/rmcp_developer.rs | Removes hints loading from MCP server initialization |
| crates/goose-mcp/src/developer/mod.rs | Removes goose_hints module reference |
| crates/goose-mcp/src/developer/goose_hints/mod.rs | File deleted (moved to goose crate) |
| Cargo.lock | Updates dependency lock file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fallback to empty gitignore if building fails | ||
| GitignoreBuilder::new(working_dir) | ||
| .build() | ||
| .expect("Failed to create fallback gitignore") |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback logic is flawed: if GitignoreBuilder::new(working_dir).build() fails on line 95-96, calling it again with the same parameters on lines 99-100 will likely fail for the same reason. The expect() will panic instead of providing a graceful fallback. Consider using a truly empty gitignore as fallback, such as GitignoreBuilder::new(\".\").build().unwrap_or_else(|_| panic!(\"...))`.
| // Fallback to empty gitignore if building fails | |
| GitignoreBuilder::new(working_dir) | |
| .build() | |
| .expect("Failed to create fallback gitignore") | |
| // Fallback to truly empty gitignore if building fails | |
| GitignoreBuilder::new(".") | |
| .build() | |
| .expect("Failed to create empty fallback gitignore") |
|
|
||
| let hints = load_hint_files( | ||
| working_dir, | ||
| &[".goosehints".to_string(), "AGENTS.md".to_string()], |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hint filenames are hardcoded here. Consider extracting these to a constant or making them configurable to maintain consistency with how they were previously configurable via CONTEXT_FILE_NAMES environment variable in the MCP version.
89482f3 to
e037c16
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e037c16 to
9d75883
Compare
crates/goose/src/hints/load_hints.rs
Outdated
| } | ||
|
|
||
| #[test] | ||
| #[serial] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serial doesn't protect you; it just means that this test will run serial with all the other serial tests. we're changing the current directory here, so that will make other tests flaky. do we need to change the current directory?
crates/goose/src/hints/load_hints.rs
Outdated
| assert!(hints.contains("# Project README")); | ||
| assert!(hints.contains("This is the project documentation")); | ||
| assert!(hints.contains("# Development Guide")); | ||
| assert!(hints.contains("Follow these steps")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to check 2 lines for each file (or even inject two lines)
crates/goose/src/hints/load_hints.rs
Outdated
| assert!(hints.contains("Project Information")); | ||
| assert!(hints.contains("Additional instructions here")); | ||
|
|
||
| // Should contain the referenced files' content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should comments that cover the asserts
crates/goose/src/hints/load_hints.rs
Outdated
|
|
||
| #[test] | ||
| #[serial] | ||
| fn test_global_goosehints() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this test please
crates/goose/src/hints/load_hints.rs
Outdated
| // choose_app_strategy().config_dir() | ||
| // Paths::config_dir() | ||
| // - macOS/Linux: ~/.config/goose/ | ||
| // - Windows: ~\AppData\Roaming\Block\goose\config\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete
crates/goose/src/hints/load_hints.rs
Outdated
| }); | ||
| let global_hints_path = Paths::in_config_dir(hints_filename); | ||
|
|
||
| if let Some(parent) = global_hints_path.parent() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we creating this directory? we're just reading from it?
crates/goose/src/agents/agent.rs
Outdated
| cancel_token: Option<CancellationToken>, | ||
| ) -> Result<BoxStream<'_, Result<AgentEvent>>> { | ||
| // Load hint files and add to system prompt | ||
| let config = Config::global(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should do this in reply_internal - that one is big enough as is. instead add this functionality to the prompt-manager itself and then add it to the call to the prompt manager:
let prompt_manager = self.prompt_manager.lock().await;
let system_prompt = prompt_manager
.builder(model_name)
.with_extensions(extensions_info.into_iter())
.with_frontend_instructions(self.frontend_instructions.lock().await.clone())
.with_extension_and_tool_counts(extension_count, tool_count)
.build();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 12 out of 14 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
crates/goose/src/hints/load_hints.rs:298
- The test content for README.md was simplified, removing 'Project overview content', but the corresponding assertion checking for this content was also removed (line 318 in the diff). This change reduces test coverage as it no longer verifies that multi-line file content is properly included when expanded. Consider restoring the original test content to maintain more comprehensive test coverage of the file expansion functionality.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dbefb64 to
a6ef558
Compare
* main: (31 commits) Standardize CLI argument flags and update documentation (#5516) Release 1.13.0 fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615) Token state not showing on load, or after message is finished. (#5606) Change the other location too (#5608) feat(ui): bring back quick launcher (#5144) Support platform tools through CLI (#5570) Avoid web double write (#5601) fix: gemini flash -> pro for mcp smoke tests (#5574) Manual compaction test and fix (#5568) fix: tidy up claude cli handling (#5594) Remove jetbrains (#5602) feat(githubcopilot): add support for newer Copilot AI Models (#5603) fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) ...
…eanup * 'main' of github.com:block/goose: Standardize CLI argument flags and update documentation (#5516) Release 1.13.0 fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615)
* main: (21 commits) differentiate debug/release in cache key (#5613) Unify subrecipe and subagent execution through shared recipe pipeline (#5082) Standardize CLI argument flags and update documentation (#5516) Release 1.13.0 fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615) Token state not showing on load, or after message is finished. (#5606) Change the other location too (#5608) feat(ui): bring back quick launcher (#5144) Support platform tools through CLI (#5570) Avoid web double write (#5601) fix: gemini flash -> pro for mcp smoke tests (#5574) Manual compaction test and fix (#5568) fix: tidy up claude cli handling (#5594) Remove jetbrains (#5602) feat(githubcopilot): add support for newer Copilot AI Models (#5603) fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) ...
…r extension (block#5575) Signed-off-by: fbalicchia <[email protected]>
* origin/main: (34 commits) Remove some logging (#5631) Use session IDs as task IDs for subagents instead of UUIDs (#5398) Fix the naming (#5628) fix: default tetrate model is broken, replace with haiku-4.5 (#5535) (#5587) Fetch less and use the right SHA (#5621) feat(ui): add custom macOS dock menu with New Window option (#5099) feat: remove hints from recipe prompts (#5622) docs: October 2025 Community All-Stars spotlight, Hacktoberfest edition (#5625) differentiate debug/release in cache key (#5613) Unify subrecipe and subagent execution through shared recipe pipeline (#5082) Standardize CLI argument flags and update documentation (#5516) Release 1.13.0 fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615) Token state not showing on load, or after message is finished. (#5606) Change the other location too (#5608) feat(ui): bring back quick launcher (#5144) Support platform tools through CLI (#5570) ...
* main: (33 commits) Fix Claude Code provider to default to Auto mode (#5638) (#5642) Scheduler cleanup (#5571) Better search paths and handling of CLI providers (#5554) docs: description required for "Add Extension" in cli - phase 2 (#5635) Remove some logging (#5631) Use session IDs as task IDs for subagents instead of UUIDs (#5398) Fix the naming (#5628) fix: default tetrate model is broken, replace with haiku-4.5 (#5535) (#5587) Fetch less and use the right SHA (#5621) feat(ui): add custom macOS dock menu with New Window option (#5099) feat: remove hints from recipe prompts (#5622) docs: October 2025 Community All-Stars spotlight, Hacktoberfest edition (#5625) differentiate debug/release in cache key (#5613) Unify subrecipe and subagent execution through shared recipe pipeline (#5082) Standardize CLI argument flags and update documentation (#5516) Release 1.13.0 fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615) ...
…r extension (block#5575) Signed-off-by: Blair Allan <[email protected]>
Move
.goosehintsandAGENTS.mdetc handling out of the developer extension and into goose itself. This shouldn't depend on an extension being active to work.This works at present, but I want to take a few revs on the the new code before merge.