Skip to content

fix: stringify multiple tool responses is unnecessary #2

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

Merged
merged 1 commit into from
May 1, 2025

Conversation

riywo
Copy link
Contributor

@riywo riywo commented May 1, 2025

This commit actually fixes the issue of multiple tool responses properly.

The previous commit tried to stringify multiple responses but it was because the error was caused by the truncate library. We actually don't need to stringify them and this commit is fixing it.

This commit actually fixes the issue of multiple tool responses properly.

The previous commit tried to stringify multiple responses but it was because the error was caused by the truncate library. We actually don't need to stringify them and this commit is fixing it.
@Copilot Copilot AI review requested due to automatic review settings May 1, 2025 04:29
Copy link

@Copilot 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 fixes issues with how multiple tool responses are logged by removing unnecessary stringification and refactoring the logging flow.

  • Removed the import and usage of MessageContent and refactored log functions in MCP tools logging.
  • Updated the tool.invoke patch to use the new logging functions (logMcpStart and logMcpEnd).

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/ui/mcp-tools.ts Removed redundant stringification logic and refactored logging start/end functions.
src/mcp/tools.ts Updated tool.invoke patch to use the new logging functions for better logging flow.

@riywo riywo merged commit d0c2754 into main May 1, 2025
@riywo riywo deleted the fix/multiple-tool-responses branch May 1, 2025 04:32
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.

1 participant