Skip to content

Conversation

@kpavlov
Copy link
Contributor

@kpavlov kpavlov commented Nov 7, 2025

Refactor and enhance prompt handling logic and tests, update Kotest version and modularize AbstractTransport

  • Added more post-conditions/scenarios to ServerPromptsTest
  • Extracted AbstractTransport to a dedicated file for modularity.
  • Improved Protocol connection state validation: When disconnected, the Protocol will throw IllegalStateException instead of Error.
  • Minor Kotlin fixes and suppressions to improve code quality.

Motivation and Context

Improve test scenarios in ServerPromptsTest

How Has This Been Tested?

Integration tests updated

Breaking Changes

When disconnected, the Protocol will throw IllegalStateException instead of Error.

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
  • Test update
  • Refactoring

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

@kpavlov kpavlov added tests enhancement New feature or request dependencies Pull requests that update a dependency file labels Nov 7, 2025
@kpavlov kpavlov force-pushed the kpavlov/refactor branch 2 times, most recently from 9ce7ee7 to 7d315c0 Compare November 10, 2025 16:06
@kpavlov kpavlov marked this pull request as ready for review November 10, 2025 16:07
@@ -80,21 +157,55 @@ class ServerPromptsTest : AbstractServerFeaturesTest() {
assertFalse(promptListChangedNotificationReceived, "No notification should be sent when prompt doesn't exist")
}

@Test
fun `removePrompt should throw when prompts capability is not supported`() = runTest {
@Nested
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need inner class? Maybe it would be better to separate it?

Copy link
Contributor Author

@kpavlov kpavlov Nov 11, 2025

Choose a reason for hiding this comment

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

It's a good idea. Let's do it in follow-up PR

@kpavlov kpavlov force-pushed the kpavlov/refactor branch 2 times, most recently from 7751d02 to eecbbee Compare November 11, 2025 16:43
@kpavlov kpavlov mentioned this pull request Nov 11, 2025
10 tasks
…d modularize `AbstractTransport`

- Extracted `AbstractTransport` to a dedicated file for modularity.
- Improved prompt validation with `require` statement and updated related tests.
- Bump jvmTarget to 11
- Fixed typos and improved test cases for better clarity.
- Deprecate `_meta` in favor of `meta` with fallback implementation; update tests to handle `McpException`.
Copy link
Contributor

@devcrocod devcrocod left a comment

Choose a reason for hiding this comment

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

👍

@kpavlov kpavlov merged commit 5636faf into main Nov 12, 2025
4 checks passed
@kpavlov kpavlov deleted the kpavlov/refactor branch November 12, 2025 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement New feature or request tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants