-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: do not take into account gitignore in developer mcp #5688
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
fix: do not take into account gitignore in developer mcp #5688
Conversation
d9337ac to
b9a55ed
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
This PR decouples .gitignore from .gooseignore in the developer MCP server by removing the fallback behavior where .gitignore would be used when .gooseignore doesn't exist. After this change, only .gooseignore files will be respected for file access restrictions, allowing operations on files that would otherwise be blocked by .gitignore.
Key changes:
- Removed
.gitignorefallback logic frombuild_ignore_patternsfunction - Removed test validating the now-removed gitignore fallback behavior
- Updated comment to reflect that only
.gooseignoreis considered
Reviewed Changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/goose-mcp/src/developer/rmcp_developer.rs |
Removed gitignore fallback in build_ignore_patterns, deleted one test for removed behavior, kept default patterns when no ignore file exists |
ui/desktop/openapi.json |
Added missing trailing newline |
Comments suppressed due to low confidence (2)
crates/goose-mcp/src/developer/rmcp_developer.rs:2005
- This test validates gitignore fallback behavior that was removed in
build_ignore_patterns. The test will now fail because.gitignoreis no longer respected. Remove this test or update it to verify that gitignore is NOT respected.
server.text_editor(write_params).await.unwrap();
// Test viewing specific range
let view_params = Parameters(TextEditorParams {
path: file_path_str.to_string(),
command: "view".to_string(),
view_range: Some(vec![3, 6]),
file_text: None,
old_str: None,
new_str: None,
insert_line: None,
diff: None,
});
let view_result = server.text_editor(view_params).await.unwrap();
let text = view_result
.content
.iter()
.find(|c| {
c.audience()
.is_some_and(|roles| roles.contains(&Role::User))
})
.unwrap()
.as_text()
.unwrap();
// Should contain lines 3-6 with line numbers
assert!(text.text.contains("3: Line 3"));
assert!(text.text.contains("4: Line 4"));
assert!(text.text.contains("5: Line 5"));
assert!(text.text.contains("6: Line 6"));
assert!(text.text.contains("(lines 3-6)"));
// Should not contain other lines
crates/goose-mcp/src/developer/rmcp_developer.rs:2071
- This test validates gitignore fallback behavior that was removed in
build_ignore_patterns. The test will now fail because.gitignoreis no longer respected. Remove this test or update it to verify that gitignore is NOT respected.
// Test viewing from line 3 to end using -1
let view_params = Parameters(TextEditorParams {
path: file_path_str.to_string(),
command: "view".to_string(),
view_range: Some(vec![3, -1]),
file_text: None,
old_str: None,
new_str: None,
insert_line: None,
diff: None,
});
let view_result = server.text_editor(view_params).await.unwrap();
let text = view_result
.content
.iter()
.find(|c| {
c.audience()
.is_some_and(|roles| roles.contains(&Role::User))
})
.unwrap()
.as_text()
.unwrap();
// Should contain lines 3-5
assert!(text.text.contains("3: Line 3"));
assert!(text.text.contains("4: Line 4"));
assert!(text.text.contains("5: Line 5"));
assert!(text.text.contains("(lines 3-end)"));
// Should not contain lines 1-2
assert!(!text.text.contains("1: Line 1"));
assert!(!text.text.contains("2: Line 2"));
}
#[tokio::test]
#[serial]
michaelneale
left a comment
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 think this makes sense, as does seem wrong. Main concern was goose rat-holing into node_modules and .venv and other derived directories wasting context, but I think it is smarter now, so not really needed.
* origin/main: (29 commits) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) feat: add Supabase MCP server to registry (#5629) Unlist VS Code extension tutorials from MCP and experimental sections (#5677) fix: make image processing work in github copilot provider (#5687) fix: do not take into account gitignore in developer mcp (#5688) docs: session storage migration (#5682) New maintainers (#5685) chore: Update governance (#5660) chore(release): release version 1.14.0 (minor) (#5676) fix : action icons overlap session title in chat history (#5684) Document recent goose PRs (#5683) docs: add GOOSE_PATH_ROOT environment variable documentation (#5678) feat: SessionManager integration for acp sessions (#5657) teach copilot our CI (#5672) bump openapi version directly (#5674) governance: update MAINTAINERS.md to reflect new maintainers (#5675) chore: upgrade rmcp to 0.8.5 (#5673) Update release instructions (#5662) Swapped out to_string_lossy with display for user facing text (#5666) ...
* 'main' of github.com:block/goose: Fix: Always show autocompact threshold ui (#5701) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) feat: add Supabase MCP server to registry (#5629) Unlist VS Code extension tutorials from MCP and experimental sections (#5677) fix: make image processing work in github copilot provider (#5687) fix: do not take into account gitignore in developer mcp (#5688) docs: session storage migration (#5682) New maintainers (#5685) chore: Update governance (#5660) chore(release): release version 1.14.0 (minor) (#5676)
* main: (27 commits) hackathon banner (#5710) Fix documentation-only change detection for push events (#5712) Added transaction commits to multi sql functions in session_manager (#5693) fix: improve and simplify tool call chain rendering (#5704) Fix: Always show autocompact threshold ui (#5701) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) feat: add Supabase MCP server to registry (#5629) Unlist VS Code extension tutorials from MCP and experimental sections (#5677) fix: make image processing work in github copilot provider (#5687) fix: do not take into account gitignore in developer mcp (#5688) docs: session storage migration (#5682) New maintainers (#5685) chore: Update governance (#5660) chore(release): release version 1.14.0 (minor) (#5676) fix : action icons overlap session title in chat history (#5684) Document recent goose PRs (#5683) docs: add GOOSE_PATH_ROOT environment variable documentation (#5678) feat: SessionManager integration for acp sessions (#5657) teach copilot our CI (#5672) ...
…eanup * 'main' of github.com:block/goose: (46 commits) Fix context progress bar not resetting after /clear command (#5652) docs: removing double announcements (#5714) docs: mcp sampling support (#5708) hackathon banner (#5710) Fix documentation-only change detection for push events (#5712) Added transaction commits to multi sql functions in session_manager (#5693) fix: improve and simplify tool call chain rendering (#5704) Fix: Always show autocompact threshold ui (#5701) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) feat: add Supabase MCP server to registry (#5629) Unlist VS Code extension tutorials from MCP and experimental sections (#5677) fix: make image processing work in github copilot provider (#5687) fix: do not take into account gitignore in developer mcp (#5688) docs: session storage migration (#5682) New maintainers (#5685) chore: Update governance (#5660) chore(release): release version 1.14.0 (minor) (#5676) fix : action icons overlap session title in chat history (#5684) Document recent goose PRs (#5683) ...
* origin/main: (51 commits) Compaction resiliency improvements (#5618) docs: ask goose button (#5730) Update prompt injection detection metrics (due to errors exporting to datadog) (#5692) Spence/icon2 tokyo drift (#5728) docs: logs rotation and misc updates (#5727) docs: automatic ollama model detection (#5725) Fix context progress bar not resetting after /clear command (#5652) docs: removing double announcements (#5714) docs: mcp sampling support (#5708) hackathon banner (#5710) Fix documentation-only change detection for push events (#5712) Added transaction commits to multi sql functions in session_manager (#5693) fix: improve and simplify tool call chain rendering (#5704) Fix: Always show autocompact threshold ui (#5701) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) feat: add Supabase MCP server to registry (#5629) Unlist VS Code extension tutorials from MCP and experimental sections (#5677) fix: make image processing work in github copilot provider (#5687) fix: do not take into account gitignore in developer mcp (#5688) ...
Signed-off-by: Blair Allan <[email protected]>
…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) <[email protected]>
…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) <[email protected]>
Do not couple gitignore to gooseignore when considering commands operating on file paths in the developer MCP
Fixes #2911
Example of a command working which would not work right now after this fix: