test(streamable-http): add missing integration tests for pagination, bad request, and logging by jskjw157 · Pull Request #613 · modelcontextprotocol/kotlin-sdk

jskjw157 · 2026-03-22T07:32:24Z

This PR adds the remaining non-security integration tests for the Streamable HTTP transport as outlined in #183 and left over from #486.

Changes included:

Pagination: Added cursor-based pagination tests for Prompts, Resources, and Tools to Abstract*IntegrationTest.
Bad Request: Added tests to verify that invalid cursor parameters correctly throw an McpException with INTERNAL_ERROR.
Logging: Added LoggingIntegrationTestStreamableHttp to specifically test setLevel and logging message notifications over Streamable HTTP.

Note: Security (Allow/Deny) tests are intentionally excluded as discussed in #486, pending further server auth model implementation.

Copilot

Pull request overview

Adds missing Streamable HTTP integration coverage to close remaining non-security scenarios for listing pagination, invalid cursor handling, and logging message delivery/filtering.

Changes:

Add cursor-based pagination integration tests for prompts/resources/tools list endpoints.
Add invalid cursor (“bad request”) tests asserting server exceptions propagate as McpException with INTERNAL_ERROR.
Add Streamable HTTP–specific logging integration tests for setLoggingLevel and log message notifications.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt	New Streamable HTTP logging integration tests (notifications + level filtering).
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt	Adds tools list pagination + invalid cursor integration coverage.
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt	Adds resources list pagination + invalid cursor integration coverage.
integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt	Adds prompts list pagination + invalid cursor integration coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-22T07:35:19Z

.../kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt

+                val all = server.tools.values.map { it.tool }
+                val cursor = request.cursor?.toIntOrNull() ?: 0
+                val pageSize = 2
+                val page = all.drop(cursor).take(pageSize)
+                val next = if (cursor + page.size < all.size) (cursor + page.size).toString() else null
+                ListToolsResult(tools = page, nextCursor = next)
+            }
+        }
+
+        val first = client.listTools()
+        assertTrue(first.tools.isNotEmpty())
+        val next = first.nextCursor
+        assertNotNull(next)
+
+        val second = client.listTools(ListToolsRequest(PaginatedRequestParams(cursor = next)))
+        assertTrue(second.tools.isNotEmpty())
+
+        val combinedNames = (first.tools + second.tools).map { it.name }
+        assertTrue(combinedNames.any { it.startsWith(prefix) })
+    }


The pagination handler builds all from server.tools.values, which comes from an immutable persistent map (no guaranteed iteration order). Because the test only fetches two pages, the newly added paginated-tool-* entries may not appear in those pages, making this test potentially flaky. Consider sorting all by tool name and/or filtering to the prefix tools (or paging until nextCursor == null) so the assertions are deterministic.

Copilot · 2026-03-22T07:35:20Z

.../kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt

+        val exception = org.junit.jupiter.api.assertThrows<io.modelcontextprotocol.kotlin.sdk.types.McpException> {
+            runBlocking {
+                client.listTools(ListToolsRequest(PaginatedRequestParams(cursor = "bad")))
+            }


This test is already running inside runBlocking(Dispatchers.IO); wrapping the suspending client.listTools(...) call in an additional runBlocking { ... } inside assertThrows is redundant and can make failures/hangs harder to diagnose. Prefer using a suspending assertion (e.g., assertFailsWith) inside the existing runBlocking, or drop the outer runBlocking and keep a single runBlocking inside assertThrows.

Copilot · 2026-03-22T07:35:20Z

...lin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt

+        val first = client.listResources()
+        assertTrue(first.resources.isNotEmpty())
+        val next = first.nextCursor
+        assertNotNull(next)
+
+        val second = client.listResources(ListResourcesRequest(PaginatedRequestParams(cursor = next)))
+        assertTrue(second.resources.isNotEmpty())
+
+        val combinedUris = (first.resources + second.resources).map { it.uri }


The pagination handler uses server.resources.values (backed by a persistent map) as the source list. Iteration order isn’t guaranteed, and since the test only fetches two pages, the paginated-resource-* entries might not land in those pages, which can make the test flaky. Sort/filter the resources list (e.g., by uri/name) and/or page until nextCursor == null before asserting.

Suggested change

val first = client.listResources()

assertTrue(first.resources.isNotEmpty())

val next = first.nextCursor

assertNotNull(next)

val second = client.listResources(ListResourcesRequest(PaginatedRequestParams(cursor = next)))

assertTrue(second.resources.isNotEmpty())

val combinedUris = (first.resources + second.resources).map { it.uri }

val combinedUris = mutableListOf<String>()

var cursor: String? = null

do {

val request = if (cursor == null) {

ListResourcesRequest()

} else {

ListResourcesRequest(PaginatedRequestParams(cursor = cursor))

}

val response = client.listResources(request)

combinedUris += response.resources.map { it.uri }

cursor = response.nextCursor

} while (cursor != null)

assertTrue(combinedUris.isNotEmpty())

Copilot · 2026-03-22T07:35:20Z

...lin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt

+        val exception = org.junit.jupiter.api.assertThrows<McpException> {
+            runBlocking {
+                client.listResources(ListResourcesRequest(PaginatedRequestParams(cursor = "bad")))
+            }


This test runs in runBlocking(Dispatchers.IO) already; the additional nested runBlocking { client.listResources(...) } inside assertThrows is unnecessary. Consider using a suspending assertion within the current runBlocking scope, or remove the outer runBlocking and keep a single runBlocking around the suspending call.

Copilot · 2026-03-22T07:35:20Z

...otlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt

+        val nextCursor = first.nextCursor
+        assertNotNull(nextCursor)
+
+        val second = client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = nextCursor)))
+        assertTrue(second.prompts.isNotEmpty())
+
+        val combined = first.prompts + second.prompts
+        assertTrue(combined.any { it.name.startsWith(pagePrefix) })


all is built from server.prompts.values (persistent map iteration order is not guaranteed). Because the test only fetches two pages, the added paginated-prompt-* prompts may not be present in first + second, making this test potentially flaky. Sort/filter the prompts list (e.g., by name) and/or iterate through pages until nextCursor == null before asserting.

Suggested change

val nextCursor = first.nextCursor

assertNotNull(nextCursor)

val second = client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = nextCursor)))

assertTrue(second.prompts.isNotEmpty())

val combined = first.prompts + second.prompts

assertTrue(combined.any { it.name.startsWith(pagePrefix) })

val allPrompts = mutableListOf<io.modelcontextprotocol.kotlin.sdk.types.Prompt>()

allPrompts.addAll(first.prompts)

var cursor = first.nextCursor

while (cursor != null) {

val page = client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = cursor)))

allPrompts.addAll(page.prompts)

cursor = page.nextCursor

}

assertTrue(allPrompts.any { it.name.startsWith(pagePrefix) })

Copilot · 2026-03-22T07:35:21Z

...otlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt

+        val exception = org.junit.jupiter.api.assertThrows<McpException> {
+            runBlocking {
+                client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = "not-a-number")))
+            }


This test already uses runBlocking(Dispatchers.IO); the nested runBlocking { client.listPrompts(...) } inside assertThrows is redundant. Prefer a single runBlocking (either outer or inner) and a suspending assertion so the test structure is simpler and avoids blocking an extra thread.

Copilot · 2026-03-22T07:35:21Z

...rotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt

+        }
+
+        server.addTool(name = "test-logging-level", description = "test") { request ->
+            LoggingLevel.values().forEach { level ->


LoggingLevel.values() allocates a new array each call; elsewhere in the codebase tests use LoggingLevel.entries, which is allocation-free and the preferred Kotlin API. Consider switching to entries for consistency and to avoid unnecessary allocations.

Suggested change

LoggingLevel.values().forEach { level ->

LoggingLevel.entries.forEach { level ->

codecov-commenter · 2026-03-26T10:40:54Z

⚠️ Unsupported file format

Upload processing failed due to unsupported file format. Please review the parser error message:

Error parsing JUnit XML in /home/runner/work/kotlin-sdk/kotlin-sdk/integration-test/build/reports/detekt/testJvm.xml at 4:177

Caused by: RuntimeError: Error accessing saved testrun

For more help, visit our troubleshooting guide.

devcrocod

left a few comments

...rotocol/kotlin/sdk/integration/kotlin/streamablehttp/LoggingIntegrationTestStreamableHttp.kt

...lin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt

.../kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt

...otlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractPromptIntegrationTest.kt

…able HTTP Add integration tests for cursor-based pagination and invalid cursor handling across Prompts, Resources, and Tools endpoints. - Pagination tests iterate all pages until nextCursor is null - Invalid cursor tests use assertFailsWith to verify McpException - Remove duplicate LoggingIntegrationTestStreamableHttp (covered by ClientConnectionLoggingTest)

Copilot AI review requested due to automatic review settings March 22, 2026 07:32

Copilot started reviewing on behalf of jskjw157 March 22, 2026 07:32 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

jskjw157 force-pushed the feature/streamable-http-missing-tests branch from 6735788 to af97f37 Compare March 22, 2026 13:42

kpavlov force-pushed the feature/streamable-http-missing-tests branch 2 times, most recently from 84d33e7 to d839b42 Compare March 26, 2026 10:27

kpavlov force-pushed the feature/streamable-http-missing-tests branch 2 times, most recently from fa786e1 to 45a3348 Compare March 26, 2026 11:05

kpavlov requested a review from devcrocod March 26, 2026 11:14

kpavlov added the tests label Mar 26, 2026

kpavlov previously approved these changes Mar 26, 2026

View reviewed changes

devcrocod reviewed Mar 26, 2026

View reviewed changes

kpavlov force-pushed the feature/streamable-http-missing-tests branch from 45a3348 to 24a1bcc Compare March 26, 2026 13:01

jskjw157 dismissed kpavlov’s stale review via dfa245f March 28, 2026 09:24

jskjw157 force-pushed the feature/streamable-http-missing-tests branch from 24a1bcc to dfa245f Compare March 28, 2026 09:24

jskjw157 requested a review from devcrocod March 28, 2026 09:30

-        val first = client.listResources()
-        assertTrue(first.resources.isNotEmpty())
-        val next = first.nextCursor
-        assertNotNull(next)
-        val second = client.listResources(ListResourcesRequest(PaginatedRequestParams(cursor = next)))
-        assertTrue(second.resources.isNotEmpty())
-        val combinedUris = (first.resources + second.resources).map { it.uri }
+        val combinedUris = mutableListOf<String>()
+        var cursor: String? = null
+        do {
+            val request = if (cursor == null) {
+                ListResourcesRequest()
+            } else {
+                ListResourcesRequest(PaginatedRequestParams(cursor = cursor))
+            }
+            val response = client.listResources(request)
+            combinedUris += response.resources.map { it.uri }
+            cursor = response.nextCursor
+        } while (cursor != null)
+        assertTrue(combinedUris.isNotEmpty())

-        val nextCursor = first.nextCursor
-        assertNotNull(nextCursor)
-        val second = client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = nextCursor)))
-        assertTrue(second.prompts.isNotEmpty())
-        val combined = first.prompts + second.prompts
-        assertTrue(combined.any { it.name.startsWith(pagePrefix) })
+        val allPrompts = mutableListOf<io.modelcontextprotocol.kotlin.sdk.types.Prompt>()
+        allPrompts.addAll(first.prompts)
+        var cursor = first.nextCursor
+        while (cursor != null) {
+            val page = client.listPrompts(ListPromptsRequest(PaginatedRequestParams(cursor = cursor)))
+            allPrompts.addAll(page.prompts)
+            cursor = page.nextCursor
+        }
+        assertTrue(allPrompts.any { it.name.startsWith(pagePrefix) })

	LoggingLevel.values().forEach { level ->
	LoggingLevel.entries.forEach { level ->

Conversation

jskjw157 commented Mar 22, 2026

Changes included:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Unsupported file format

Uh oh!

devcrocod left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Mar 26, 2026 •

edited

Loading