Skip to content

Conversation

@jamadeo
Copy link
Collaborator

@jamadeo jamadeo commented Jul 18, 2025

Basically, delete crates/mcp-core/src/content.rs and add in rmcp types in its place. Kicked off by me and mostly done by Goose+Claude.

@jamadeo jamadeo marked this pull request as draft July 18, 2025 03:49
@jamadeo jamadeo changed the base branch from main to alexhancock/rmcp-role July 18, 2025 03:50
@jamadeo jamadeo force-pushed the jackamadeo/goose-rmcp-content branch from 339bfc8 to 634bdea Compare July 18, 2025 04:02
@jamadeo jamadeo requested a review from Copilot July 18, 2025 13:09
Copy link
Contributor

Copilot AI left a 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 replaces the mcp_core::content module with types from the rmcp::model crate, consolidating content type definitions and removing duplicate implementations. This is part of a broader migration strategy to unify content types across the codebase.

  • Removes the entire mcp_core/src/content.rs file containing Content, TextContent, ImageContent, Annotations, and EmbeddedResource types
  • Updates all imports throughout the codebase to use rmcp::model equivalents instead
  • Adapts code to work with the new type structure, particularly the "raw" wrapper pattern used in rmcp types

Reviewed Changes

Copilot reviewed 61 out of 62 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
crates/mcp-core/src/content.rs Complete removal of content type definitions
crates/mcp-core/src/lib.rs Removes content module exports
crates/mcp-core/src/protocol.rs Updates Content import to use rmcp
crates/mcp-core/src/prompt.rs Updates content type usage and structure adaptation
crates/mcp-server/src/main.rs Updates Content import source
crates/mcp-server/src/router.rs Updates Content import source
crates/mcp-server/Cargo.toml Adds rmcp workspace dependency
crates/goose/src/message.rs Major updates to content type handling and conversions
Multiple goose provider format files Updates to handle new content structure with raw wrapper
Multiple test files Updates test code to use new content APIs

}
}
}).collect();
Ok(converted_messages)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should go away once we do the PromptMessage migration. But we needed to do a bit of it here since we want to work with the rmcp content types.

@alexhancock alexhancock force-pushed the alexhancock/rmcp-role branch 3 times, most recently from 5aef1aa to 355ac32 Compare July 18, 2025 15:34
Base automatically changed from alexhancock/rmcp-role to main July 18, 2025 16:30
@jamadeo jamadeo force-pushed the jackamadeo/goose-rmcp-content branch from f608459 to 9841c9e Compare July 18, 2025 19:34
data: string;
mimeType: string;
type: string;
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this looks like a bit less useful than what we had before

Copy link
Collaborator

Choose a reason for hiding this comment

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

are there any frontend imports of this that may need to be updated? we don't have a ton of testing in the frontend unfortunately

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah the TS checker passing does give some confidence that this change is OK, but I'll pay some extra attention to this to be sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah, turns out we are not using this type at all, because we have our own extended MessageContent type that we use instead

data: string;
mimeType: string;
type: string;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

are there any frontend imports of this that may need to be updated? we don't have a ton of testing in the frontend unfortunately

@jamadeo jamadeo marked this pull request as ready for review July 18, 2025 20:29
@jamadeo jamadeo merged commit d529146 into main Jul 18, 2025
8 checks passed
@jamadeo jamadeo deleted the jackamadeo/goose-rmcp-content branch July 18, 2025 21:23
jsibbison-square added a commit that referenced this pull request Jul 20, 2025
…ntral-deeplinks

* origin/main: (22 commits)
  feat: deprecate jetbrains extension in favor of public one (#2589)
  feat: Add LiteLLM provider with automatic prompt caching support (#3380)
  docs: update desktop instructions for managing sessions (#3522)
  docs: update desktop instructions for session recipes (#3521)
  Replace mcp_core::content types with rmcp::model types (#3500)
  docs: update desktop instructions for tool perms (#3518)
  docs: update desktop instructions for tool router (#3519)
  Alexhancock/reapply 3491 (#3515)
  docs: update mcp install instructions for desktop (#3504)
  Docs: Access settings in new UI (#3514)
  feat: switch from mcp_core::Role to rmcp::model::Role (#3488)
  Revert "fix the output not being visible issue (#3491)" (#3511)
  fix: Load and Use recipes in new window (#3501)
  fix: working dir was not being set correctly  (#3477)
  Fix launching session in new window (#3497)
  Fix tool call allow still showing initial state in chat after navigating back (#3498)
  feat: add rmcp as a workspace dep (#3483)
  feat: consolidate subagent execution for dynamic tasks (#3444)
  fix token alert indicator/popovers hiding and showing (#3492)
  Fix llm errors not propagating to the ui and auto summarize not starting (#3490)
  ...
cbruyndoncx pushed a commit to cbruyndoncx/goose that referenced this pull request Jul 20, 2025
michaelneale added a commit that referenced this pull request Jul 21, 2025
* main:
  Extension Library Improvements (#3541)
  fix(ui): enable selection of zero-config providers in desktop GUI (#3378)
  refactor: Renames recipe route to recipes to be consistent (#3540)
  Blog: Orchestrating 6 Subagents to Build a Collaborative API Playground (#3528)
  Catch json errors a little better (#3437)
  Rust debug (#3510)
  refactor: Centralise deeplink encode and decode into server (#3489)
  feat: deprecate jetbrains extension in favor of public one (#2589)
  feat: Add LiteLLM provider with automatic prompt caching support (#3380)
  docs: update desktop instructions for managing sessions (#3522)
  docs: update desktop instructions for session recipes (#3521)
  Replace mcp_core::content types with rmcp::model types (#3500)
  docs: update desktop instructions for tool perms (#3518)
  docs: update desktop instructions for tool router (#3519)
  Alexhancock/reapply 3491 (#3515)
  docs: update mcp install instructions for desktop (#3504)
  Docs: Access settings in new UI (#3514)
  feat: switch from mcp_core::Role to rmcp::model::Role (#3488)
  Revert "fix the output not being visible issue (#3491)" (#3511)
  fix: Load and Use recipes in new window (#3501)
@jamadeo jamadeo mentioned this pull request Jul 22, 2025
4 tasks
atarantino pushed a commit to atarantino/goose that referenced this pull request Aug 5, 2025
Signed-off-by: Adam Tarantino <tarantino.adam@hey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants