Skip to content

Conversation

@alexhancock
Copy link
Collaborator

Replace JsonRpcMessage/Request/Response/Error/Notification from the internal mcp crates with the equivalent https://github.com/modelcontextprotocol/rust-sdk types

@alexhancock alexhancock requested a review from jamadeo July 21, 2025 20:38
@alexhancock alexhancock marked this pull request as draft July 21, 2025 20:38
@alexhancock alexhancock marked this pull request as ready for review July 21, 2025 20:48
@alexhancock alexhancock force-pushed the alexhancock/rmcp-json-rpc branch from 4ffb6c5 to 1b0a927 Compare July 21, 2025 20:50
"stream": "stdout",
"output": line.to_string(),
}
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

in rmcp I see

/// A JSON object type alias for convenient handling of JSON data.
///
/// You can use [`crate::object!`] or [`crate::model::object`] to create a json object quickly.
/// This is commonly used for storing arbitrary JSON data in MCP messages.
pub type JsonObject<F = Value> = serde_json::Map<String, F>;

can we use that object! macro here?

service: Mutex<tower::timeout::Timeout<McpService<T>>>,
next_id: AtomicU64,
next_id: RequestId,
next_id_counter: AtomicU64, // Added for atomic ID generation
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need both of these? can we construct the RequestId from the Atomic when we need it?

source: Box::<Error>::new(e.into()),
})?;
let request_json = serde_json::to_value(&request)?;
let generic_request: JsonRpcMessage = serde_json::from_value(request_json)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need a to_value/from_value here?

@alexhancock
Copy link
Collaborator Author

Will rebase to address import conflicts + address PR feedback in the morning

@jamadeo jamadeo mentioned this pull request Jul 22, 2025
4 tasks
@alexhancock alexhancock force-pushed the alexhancock/rmcp-json-rpc branch 7 times, most recently from 536789b to e09f444 Compare July 22, 2025 16:18
@alexhancock alexhancock force-pushed the alexhancock/rmcp-json-rpc branch from e09f444 to 03d2663 Compare July 22, 2025 16:36
@alexhancock alexhancock requested a review from jamadeo July 22, 2025 16:37
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

id: rmcp::model::RequestId::Number(0),
result: serde_json::Map::new(),
});
Ok(dummy_response)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was going to suggest we make the message optional now that Nil isn't a valid JsonRpcMessage, but this code will all go away soon anyway, so maybe not that important...

@alexhancock alexhancock merged commit 3e5c9b8 into main Jul 22, 2025
8 checks passed
@alexhancock alexhancock deleted the alexhancock/rmcp-json-rpc branch July 22, 2025 17:45
katzdave added a commit that referenced this pull request Jul 22, 2025
* 'main' of github.com:block/goose: (23 commits)
  fix: add fallback id to messages if none provided (#3584)
  feat: migrate ErrorData from internal mcp crates to rmcp version (#3586)
  fix: adjust subrecipe description to allow running tests (#3585)
  Scenario tests (#3430)
  feat: migrate JsonRpcMessage/Request/Response/Error/Notification from internal mcp crates to rmcp versions (#3564)
  Restore recipe parameters functionality (#3530)
  feat: Enhanced loading states with thinking icons and flying bird animation (#3478)
  Agent loop defensive (#3554)
  chore: remove needless clone() in goose/providers (#2528)
  Add recipe install warning (#3537)
  Replace mcp_core::resource::* with rmcp types (#3563)
  Add YouTube video embed to using-goosehints.md (#3560)
  fix: ensure retry-config and success-criteria are populated in openapi spec (#3575)
  fix: use sequential when sub recipe task is 1. (#3573)
  fix: track message id to keep like with like (#3572)
  Replace mcp_core::prompt with rmcp::model types (#3561)
  feat (ui): close recipe modals with esc key (#3568)
  feat: recipes can retry with success criteria (#3474)
  Env var to set Ollama request timeout (#3516)
  Updating docs to match new UI (#3552)
  ...
michaelneale added a commit that referenced this pull request Jul 23, 2025
* main:
  docs: desktop recipe format (#3594)
  Fix model display name not being updated immediately after leaving settings (#3587)
  Added option to summarize the chat when an error is triggered (#3598)
  Remove mcp_macros and unused types (#3581)
  fix: add fallback id to messages if none provided (#3584)
  feat: migrate ErrorData from internal mcp crates to rmcp version (#3586)
  fix: adjust subrecipe description to allow running tests (#3585)
  Scenario tests (#3430)
  feat: migrate JsonRpcMessage/Request/Response/Error/Notification from internal mcp crates to rmcp versions (#3564)
  Restore recipe parameters functionality (#3530)
  feat: Enhanced loading states with thinking icons and flying bird animation (#3478)
  Agent loop defensive (#3554)
  chore: remove needless clone() in goose/providers (#2528)
  Add recipe install warning (#3537)
  Replace mcp_core::resource::* with rmcp types (#3563)
  Add YouTube video embed to using-goosehints.md (#3560)
  fix: ensure retry-config and success-criteria are populated in openapi spec (#3575)
taylorkmho added a commit that referenced this pull request Jul 23, 2025
* main:
  feat: subagent turn count, simple agent loop (#3597)
  feat: subagent independent extension manager (#3596)
  Improve session history loading resiliency (#3588)
  Added logging and changed default route case to not redirect to home when there is an invalid route (#3610)
  fix: chat sidebar layout overlapping content occasionally (#3590)
  fix: loading shared sessions (#3607)
  docs: use installer component for tutorial, add updating extensions section (#3608)
  fix: show token alert popover during agent responses and agent failure cases (#3536)
  reuse the cancellation token in the agent level (#3599)
  Docs: Move MongoDB tutorial to MCP section (#3602)
  docs: desktop recipe format (#3594)
  Fix model display name not being updated immediately after leaving settings (#3587)
  Added option to summarize the chat when an error is triggered (#3598)
  Remove mcp_macros and unused types (#3581)
  fix: add fallback id to messages if none provided (#3584)
  feat: migrate ErrorData from internal mcp crates to rmcp version (#3586)
  fix: adjust subrecipe description to allow running tests (#3585)
  Scenario tests (#3430)
  feat: migrate JsonRpcMessage/Request/Response/Error/Notification from internal mcp crates to rmcp versions (#3564)
  Restore recipe parameters functionality (#3530)
lifeizhou-ap added a commit that referenced this pull request Jul 24, 2025
* main: (28 commits)
  fix: multi-tool calls in streamed openai-compatible responses (#3609)
  feat: subagent turn count, simple agent loop (#3597)
  feat: subagent independent extension manager (#3596)
  Improve session history loading resiliency (#3588)
  Added logging and changed default route case to not redirect to home when there is an invalid route (#3610)
  fix: chat sidebar layout overlapping content occasionally (#3590)
  fix: loading shared sessions (#3607)
  docs: use installer component for tutorial, add updating extensions section (#3608)
  fix: show token alert popover during agent responses and agent failure cases (#3536)
  reuse the cancellation token in the agent level (#3599)
  Docs: Move MongoDB tutorial to MCP section (#3602)
  docs: desktop recipe format (#3594)
  Fix model display name not being updated immediately after leaving settings (#3587)
  Added option to summarize the chat when an error is triggered (#3598)
  Remove mcp_macros and unused types (#3581)
  fix: add fallback id to messages if none provided (#3584)
  feat: migrate ErrorData from internal mcp crates to rmcp version (#3586)
  fix: adjust subrecipe description to allow running tests (#3585)
  Scenario tests (#3430)
  feat: migrate JsonRpcMessage/Request/Response/Error/Notification from internal mcp crates to rmcp versions (#3564)
  ...
atarantino pushed a commit to atarantino/goose that referenced this pull request Aug 5, 2025
… internal mcp crates to rmcp versions (block#3564)

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