Skip to content

Conversation

@alexhancock
Copy link
Collaborator

@alexhancock alexhancock commented Jul 23, 2025

Move from internal representation of Tool and ToolAnnotations to the rmcp versions

@alexhancock alexhancock marked this pull request as draft July 23, 2025 20:51
@alexhancock alexhancock force-pushed the alexhancock/rmcp-tools-annotations branch 6 times, most recently from 3d30f75 to 2a6f06e Compare July 24, 2025 16:35
@alexhancock alexhancock requested a review from jamadeo July 24, 2025 16:35
@alexhancock alexhancock marked this pull request as ready for review July 24, 2025 16:36
@alexhancock alexhancock force-pushed the alexhancock/rmcp-tools-annotations branch from 2a6f06e to 07ce1a4 Compare July 24, 2025 16:47
@alexhancock alexhancock mentioned this pull request Jul 24, 2025
4 tasks
Copy link
Collaborator

@jamadeo jamadeo left a comment

Choose a reason for hiding this comment

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

Nice. Left a couple of comments where some of the type conversions seemed a bit odd, mainly with the Arcs.

Also I think the rmcp ToolAnnotations type has a builder that might make the construction a bit nicer, but if so let's leave it for a follow up goose task.

.map(|t| rmcp::model::Tool {
name: t.name.into(),
description: Some(std::borrow::Cow::Owned(t.description)),
input_schema: std::sync::Arc::new(t.input_schema.as_object().unwrap().clone()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this need to be an Arc?

for tool in tools {
let frontend_tool = FrontendTool {
name: tool.name.clone(),
name: tool.name.clone().parse().unwrap(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the .parse() doing?

idempotent_hint: true,
open_world_hint: false,
}),
Arc::new(
Copy link
Collaborator

Choose a reason for hiding this comment

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

is Arc necessary?

@alexhancock alexhancock force-pushed the alexhancock/rmcp-tools-annotations branch from e2d06a4 to 4a57970 Compare July 24, 2025 17:43
@alexhancock
Copy link
Collaborator Author

alexhancock commented Jul 24, 2025

@jamadeo Feedback addressed in 4a57970 for viz

@alexhancock alexhancock requested a review from jamadeo July 24, 2025 17:43
@alexhancock alexhancock force-pushed the alexhancock/rmcp-tools-annotations branch from 4a57970 to 6befed3 Compare July 24, 2025 18:33
@jamadeo jamadeo merged commit b4fc6f7 into main Jul 24, 2025
8 checks passed
@jamadeo jamadeo deleted the alexhancock/rmcp-tools-annotations branch July 24, 2025 18:58
lifeizhou-ap added a commit that referenced this pull request Jul 25, 2025
* main:
  docs: Quick spotlight fix (#3633)
  alexhancock/rmcp-tools-annotations (#3617)
  fix: clean up subagent (#3565)
  Adds the `WaitingForUserInput` state (#3620)
  docs: update extensions library (#3612)
  Fixing grants path (#3632)
  docs: June 2024 Community All-Stars Spotlight (#3631)
  grant program (#3630)
  Lifei/sub recipe desktop temp (#3576)
michaelneale added a commit that referenced this pull request Jul 28, 2025
* main: (25 commits)
  fix: add maintainer, homepage and categories to DEB/RPM package config (#3096)
  blog: agent to agent convo (#3677)
  Possible to disable random thinking messages (#3304)
  Two VS code tutorials (#3603)
  small blog fixes (#3549)
  docs: fix installation command for YouTube Transcript MCP in servers.json (#3595)
  Docs for using Docker Model Runner as a local LLM provider.  (#3509)
  Docs: VS Code Extension move to tutorials (#3601)
  Fix working directory when session has no messages (#3513)
  goose docs MCP server (#3665)
  Remove confusing status output when testing sharing url connection and it shows 404 (#3659)
  chore: use typed notifications from rmcp (#3653)
  feat: convert GetPromptResult from mcp_core to rmcp version (#3650)
  feat: Replace usage of mcp_core Tools/ToolAnnotations in openapi schema (#3649)
  fix: ensure execution task result is shown (#3629)
  docs: Quick spotlight fix (#3633)
  alexhancock/rmcp-tools-annotations (#3617)
  fix: clean up subagent (#3565)
  Adds the `WaitingForUserInput` state (#3620)
  docs: update extensions library (#3612)
  ...
michaelneale added a commit that referenced this pull request Jul 28, 2025
* main: (27 commits)
  Add inline python extension (#3107)
  fix: add maintainer, homepage and categories to DEB/RPM package config (#3096)
  blog: agent to agent convo (#3677)
  Possible to disable random thinking messages (#3304)
  Two VS code tutorials (#3603)
  small blog fixes (#3549)
  docs: fix installation command for YouTube Transcript MCP in servers.json (#3595)
  Docs for using Docker Model Runner as a local LLM provider.  (#3509)
  Docs: VS Code Extension move to tutorials (#3601)
  Fix working directory when session has no messages (#3513)
  goose docs MCP server (#3665)
  Remove confusing status output when testing sharing url connection and it shows 404 (#3659)
  chore: use typed notifications from rmcp (#3653)
  feat: convert GetPromptResult from mcp_core to rmcp version (#3650)
  feat: Replace usage of mcp_core Tools/ToolAnnotations in openapi schema (#3649)
  fix: ensure execution task result is shown (#3629)
  docs: Quick spotlight fix (#3633)
  alexhancock/rmcp-tools-annotations (#3617)
  fix: clean up subagent (#3565)
  Adds the `WaitingForUserInput` state (#3620)
  ...
michaelneale added a commit that referenced this pull request Jul 28, 2025
* main: (69 commits)
  Add inline python extension (#3107)
  fix: add maintainer, homepage and categories to DEB/RPM package config (#3096)
  blog: agent to agent convo (#3677)
  Possible to disable random thinking messages (#3304)
  Two VS code tutorials (#3603)
  small blog fixes (#3549)
  docs: fix installation command for YouTube Transcript MCP in servers.json (#3595)
  Docs for using Docker Model Runner as a local LLM provider.  (#3509)
  Docs: VS Code Extension move to tutorials (#3601)
  Fix working directory when session has no messages (#3513)
  goose docs MCP server (#3665)
  Remove confusing status output when testing sharing url connection and it shows 404 (#3659)
  chore: use typed notifications from rmcp (#3653)
  feat: convert GetPromptResult from mcp_core to rmcp version (#3650)
  feat: Replace usage of mcp_core Tools/ToolAnnotations in openapi schema (#3649)
  fix: ensure execution task result is shown (#3629)
  docs: Quick spotlight fix (#3633)
  alexhancock/rmcp-tools-annotations (#3617)
  fix: clean up subagent (#3565)
  Adds the `WaitingForUserInput` state (#3620)
  ...
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