Skip to content

Conversation

skarpovdev
Copy link
Contributor

@skarpovdev skarpovdev commented Aug 28, 2025

Motivation and Context

Now should ignore first output messages test fails because the valid JSON message is on the same line as non-JSON "Stdio server started".
I added a recovery which attempts to find JSON by searching for the first {. A case when we fail to desirialize a string remains the same and logs the error message as before. It is generallt not a likely case since it is expected that each message is on a separate line, but it doesn't seem to cost us anything to try and recover an otherwise lost valid message from the server.

The changes in kotlin-sdk-test/build.gradle.kts are necessary because without them the JVM tests that use JUnit5 are not run at the moment.

The changes in .github/workflows/validate-pr.yml will allow us to view test results as artifacts to see which tests were skipped and generally simplify investigating tests results.

How Has This Been Tested?

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@skarpovdev skarpovdev marked this pull request as ready for review August 28, 2025 16:50
@skarpovdev skarpovdev changed the title Add recovery during message deserialization and update workflow to up… Add recovery during deserialization and upload test artifacts Aug 28, 2025
@tiginamaria tiginamaria self-requested a review August 29, 2025 09:54
Copy link
Contributor

@tiginamaria tiginamaria left a comment

Choose a reason for hiding this comment

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

Thank you! I've left just one clarification question there to know the exact reason of the parsing error. Other than that, all is good!

@tiginamaria tiginamaria merged commit a23856e into modelcontextprotocol:main Aug 29, 2025
3 checks passed
@skarpovdev skarpovdev deleted the skarpov/fix-readbuffer branch August 29, 2025 10:36
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.

2 participants