8000 MCP fixes by pubmodmatt · Pull Request #2596 · apollographql/rover · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

MCP fixes #2596

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
May 21, 2025
Merged

MCP fixes #2596

merged 6 commits into from
May 21, 2025

Conversation

pubmodmatt
Copy link
Contributor
@pubmodmatt pubmodmatt commented May 16, 2025 •
edited
Loading

Fixes several issues with rover dev --mcp:

  • All output from the MCP server was always logged as ERROR or UNKNOWN (which used the same styling as ERROR). This made it look as if the MCP server had failed, when in fact this was just normal output. The logging from the MCP server is now just passed through to the rover dev output. In the future, we'll need to add the ability to log in JSON like the Apollo Router has, so we can parse the error level and message as is done for router.
  • The code to shut down the Router and MCP Server processes did not really do anything. It just stopped the spawned task that waited for the child process, but did nothing to actually stop the child process itself. Ctrl+C only worked because the OS sends the SIGINT to the forked child processes as well as the parent. But in cases where one or the other of the child processes died, the other process was not being killed. This has been fixed so either child process exiting will cause the other to be killed.
  • Improve path handling. The MCP code was previously using the lossy display() method on PathBuf. It now uses Utf8PathBuf like the rest of the rover code.
  • Added an --mcp-sse-address option to set the bind address for the MCP server.

@pubmodmatt pubmodmatt added this to the vNext milestone May 16, 2025
@pubmodmatt pubmodmatt self-assigned this May 16, 2025
@pubmodmatt pubmodmatt requested a review from a team as a code owner May 16, 2025 21:53
@pubmodmatt pubmodmatt added the fix 🩹 fixes a bug label May 16, 2025
@svc-apollo-docs
Copy link
Collaborator
svc-apollo-docs commented May 16, 2025 •
edited
Loading

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 2 changed, 0 removed 
* (developer-tools)/apollo-mcp-server/(latest)/command-reference.mdx
* (developer-tools)/apollo-mcp-server/(latest)/guides/index.mdx

Build ID: bf24ddba8ad577228fbba0ce

URL: https://www.apollographql.com/docs/deploy-preview/bf24ddba8ad577228fbba0ce

Jephuff
Jephuff approved these changes May 20, 2025
View reviewed changes
Copy link
Contributor
@Jephuff Jephuff left a comment

Choose a reason for hiding this comment

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

🎉

@Jephuff Jephuff mentioned this pull request May 21, 2025
Merged
@pubmodmatt pubmodmatt enabled auto-merge (squash) May 21, 2025 22:00
@pubmodmatt pubmodmatt disabled auto-merge May 21, 2025 22:01
@pubmodmatt pubmodmatt enabled auto-merge (squash) May 21, 2025 22:01
@pubmodmatt pubmodmatt merged commit 585887c into main May 21, 2025
32 checks passed
@pubmodmatt pubmodmatt deleted the pubmodmatt/mcp_logging branch May 21, 2025 22:04
@pubmodmatt pubmodmatt mentioned this pull request May 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jephuff Jephuff Jephuff approved these changes

Assignees

@pubmodmatt pubmodmatt

Labels
fix 🩹 fixes a bug
Projects
None yet
Milestone
v0.31.1
Development

Successfully merging this pull request may close these issues.
3 participants
@pubmodmatt @svc-apollo-docs @Jephuff
0