-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Remove To-Do tool instructions from system prompt #4454
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| use std::future::Future; | ||
| use std::pin::Pin; | ||
|
|
||
| use mcp_core::handler::{PromptError, ResourceError}; | ||
| use mcp_core::protocol::ServerCapabilities; | ||
| use mcp_server::router::CapabilitiesBuilder; | ||
| use mcp_server::Router; | ||
| use rmcp::model::{Content, ErrorCode, ErrorData, JsonRpcMessage, Prompt, Resource, Tool}; | ||
| use serde_json::Value; | ||
| use tokio::sync::mpsc; | ||
|
|
||
| // Import the TODO tools | ||
| use goose::agents::todo_tools::{todo_read_tool, todo_write_tool}; | ||
|
|
||
| /// A lightweight router that provides TODO task management capabilities. | ||
| /// | ||
| /// This extension acts as a metadata provider - it exposes the TODO tools | ||
| /// and provides instructions, but the actual execution remains in the agent | ||
| /// for session storage access. | ||
| #[derive(Clone)] | ||
| pub struct TodoRouter { | ||
| tools: Vec<Tool>, | ||
| instructions: String, | ||
| } | ||
|
|
||
| impl Default for TodoRouter { | ||
| fn default() -> Self { | ||
| Self::new() | ||
| } | ||
| } | ||
|
|
||
| impl TodoRouter { | ||
| pub fn new() -> Self { | ||
| let instructions = r#"The todo extension provides persistent task management throughout your session. | ||
| These tools help you track multi-step work, maintain context between interactions, and ensure systematic task completion. | ||
| ## Task Management Guidelines | ||
| **Required Usage:** | ||
| - Use `todo__read` and `todo__write` for any task with 2+ steps, multiple files/components, or uncertain scope | ||
| - Skipping these tools when needed is considered an error | ||
| **Workflow:** | ||
| 1. Start: Always `todo__read` first, then `todo__write` a brief checklist using Markdown checkboxes | ||
| 2. During: After each major action, reread via `todo__read`, then update via `todo__write` - mark completed items, add new discoveries, note blockers | ||
| 3. Finish: Ensure every item is checked, or clearly list what remains | ||
| **Critical:** `todo__write` replaces the entire list. Always read before writing - not doing so is an error. | ||
| **Best Practices:** | ||
| - Keep items short, specific, and action-oriented | ||
| - Use nested checkboxes for subtasks | ||
| - Include context about blockers or dependencies | ||
| Example format: | ||
| ```markdown | ||
| - [x] Analyze request fully | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if we are moving the prompt here we need to make it clear similar to the old system.md one how to do it, including if we want it to use subagents at times (otherwise I found it will not) - https://github.com/block/goose/pull/4421/files#diff-507a51de1f94529d2674a027f5bf6e5eeed83ccfe9cbbe6cea674b75aef65ee8 like that
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I totally agree. I've been doing benching with this instruction here and it works well https://github.com/block/goose/pull/4311/files#diff-507a51de1f94529d2674a027f5bf6e5eeed83ccfe9cbbe6cea674b75aef65ee8R76 I hope we can merge in that subagent PR soon-ish |
||
| - [ ] Create implementation plan | ||
| - [x] General guidelines | ||
| - [ ] Sample work | ||
| - [ ] Begin on implementation plan | ||
| ```"#; | ||
|
|
||
| Self { | ||
| tools: vec![todo_read_tool(), todo_write_tool()], | ||
| instructions: instructions.to_string(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Router for TodoRouter { | ||
| fn name(&self) -> String { | ||
| "todo".to_string() | ||
| } | ||
|
|
||
| fn instructions(&self) -> String { | ||
| self.instructions.clone() | ||
| } | ||
|
|
||
| fn capabilities(&self) -> ServerCapabilities { | ||
| CapabilitiesBuilder::new() | ||
| .with_tools(false) | ||
| .with_prompts(false) | ||
| .build() | ||
| } | ||
|
|
||
| fn list_tools(&self) -> Vec<Tool> { | ||
| self.tools.clone() | ||
| } | ||
|
|
||
| fn call_tool( | ||
| &self, | ||
| _tool_name: &str, | ||
| _arguments: Value, | ||
| _notifier: mpsc::Sender<JsonRpcMessage>, | ||
| ) -> Pin<Box<dyn Future<Output = Result<Vec<Content>, ErrorData>> + Send + 'static>> { | ||
| // The agent handles TODO tool execution directly for session access. | ||
| // This router only provides metadata and tool definitions. | ||
| Box::pin(async move { | ||
| Err(ErrorData::new( | ||
| ErrorCode::METHOD_NOT_FOUND, | ||
| "TODO tools are executed directly by the agent".to_string(), | ||
| None, | ||
| )) | ||
| }) | ||
| } | ||
|
|
||
| fn list_resources(&self) -> Vec<Resource> { | ||
| Vec::new() | ||
| } | ||
|
|
||
| fn read_resource( | ||
| &self, | ||
| _uri: &str, | ||
| ) -> Pin<Box<dyn Future<Output = Result<String, ResourceError>> + Send + 'static>> { | ||
| Box::pin(async move { Ok(String::new()) }) | ||
| } | ||
|
|
||
| fn list_prompts(&self) -> Vec<Prompt> { | ||
| Vec::new() | ||
| } | ||
|
|
||
| fn get_prompt( | ||
| &self, | ||
| _prompt_name: &str, | ||
| ) -> Pin<Box<dyn Future<Output = Result<String, PromptError>> + Send + 'static>> { | ||
| Box::pin(async move { | ||
| Err(PromptError::NotFound( | ||
| "TODO extension has no prompts".to_string(), | ||
| )) | ||
| }) | ||
| } | ||
| } | ||
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.
yeah this doesn't seem ideal, but at least it is in right direction