-
Notifications
You must be signed in to change notification settings - Fork 162
Adds 'instructions' string to the Server and propagates to the InitializeResult #290
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
Changes from 11 commits
f5f24fa
574c435
0d8253e
c0249a5
b8a5945
6a95a59
4b7674f
1a714d7
74aeaff
3e682ec
083ff48
b05a196
99555d2
8600054
7a02d56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -394,4 +394,63 @@ class TypesTest { | |
assertEquals("id", request.argument.name) | ||
assertEquals("123", request.argument.value) | ||
} | ||
|
||
// InitializeResult Tests | ||
@Test | ||
fun `should create InitializeResult with default instructions`() { | ||
val serverInfo = Implementation(name = "test-server", version = "1.0.0") | ||
val result = InitializeResult( | ||
serverInfo = serverInfo, | ||
) | ||
|
||
assertEquals(LATEST_PROTOCOL_VERSION, result.protocolVersion) | ||
assertEquals(serverInfo, result.serverInfo) | ||
assertEquals(null, result.instructions) | ||
} | ||
|
||
@Test | ||
fun `should create InitializeResult with custom instructions`() { | ||
val serverInfo = Implementation(name = "test-server", version = "1.0.0") | ||
val instructions = "Use this server to perform calculations. Call the 'add' tool to add numbers." | ||
val result = InitializeResult( | ||
serverInfo = serverInfo, | ||
instructions = instructions, | ||
) | ||
|
||
assertEquals(LATEST_PROTOCOL_VERSION, result.protocolVersion) | ||
assertEquals(serverInfo, result.serverInfo) | ||
assertEquals(instructions, result.instructions) | ||
} | ||
|
||
@Test | ||
fun `should serialize and deserialize InitializeResult with instructions`() { | ||
val serverInfo = Implementation(name = "test-server", version = "1.0.0") | ||
val instructions = "This server provides file system access. Use the 'read' tool to read files." | ||
val result = InitializeResult( | ||
serverInfo = serverInfo, | ||
instructions = instructions, | ||
) | ||
|
||
val json = McpJson.encodeToString(result) | ||
val decoded = McpJson.decodeFromString<InitializeResult>(json) | ||
Comment on lines
+434
to
+435
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I think we should introduce some serialization helper for tests like this, like here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would probably be applicable to all the tests in this file, so I would prefer to leave it to a different PR if that's ok There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with both statements |
||
|
||
assertEquals(LATEST_PROTOCOL_VERSION, decoded.protocolVersion) | ||
assertEquals(serverInfo, decoded.serverInfo) | ||
assertEquals(instructions, decoded.instructions) | ||
} | ||
|
||
@Test | ||
fun `should serialize and deserialize InitializeResult without instructions`() { | ||
val serverInfo = Implementation(name = "test-server", version = "1.0.0") | ||
val result = InitializeResult( | ||
serverInfo = serverInfo, | ||
) | ||
|
||
val json = McpJson.encodeToString(result) | ||
val decoded = McpJson.decodeFromString<InitializeResult>(json) | ||
|
||
assertEquals(LATEST_PROTOCOL_VERSION, decoded.protocolVersion) | ||
assertEquals(serverInfo, decoded.serverInfo) | ||
assertEquals(null, decoded.instructions) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,8 +53,15 @@ public class ServerOptions(public val capabilities: ServerCapabilities, enforceS | |
* | ||
* @param serverInfo Information about this server implementation (name, version). | ||
* @param options Configuration options for the server. | ||
* @param instructionsProvider Optional provider for instructions from the server to the client about how to use | ||
* this server. The provider is called each time a new session is started to support dynamic instructions. | ||
*/ | ||
public open class Server(private val serverInfo: Implementation, private val options: ServerOptions) { | ||
|
||
public open class Server( | ||
protected val serverInfo: Implementation, | ||
protected val options: ServerOptions, | ||
protected val instructionsProvider: (() -> String)? = null, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this implemented as a lambda and not just a string? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is allowing the instructions to potentially be dynamically loaded from a database or string server. With the change to the ServerSession, the Server seems to take more of a Singleton scope vs a Server per stream. The idea is that the Server has a provider method for the string which it evaluates as a string value for the ServerSession.
I went ahead and added an alternative constructor that takes a static string |
||
) { | ||
private val sessions = atomic(persistentListOf<ServerSession>()) | ||
|
||
@Suppress("ktlint:standard:backing-property-naming") | ||
|
@@ -90,7 +97,7 @@ public open class Server(private val serverInfo: Implementation, private val opt | |
* @return The initialized and connected server session. | ||
*/ | ||
public suspend fun connect(transport: Transport): ServerSession { | ||
val session = ServerSession(serverInfo, options) | ||
val session = ServerSession(serverInfo, options, instructionsProvider?.invoke()) | ||
|
||
// Internal handlers for tools | ||
if (options.capabilities.tools != null) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import kotlinx.coroutines.CompletableDeferred | |
import kotlinx.coroutines.launch | ||
import kotlinx.coroutines.test.runTest | ||
import org.junit.jupiter.api.Test | ||
import org.junit.jupiter.api.assertNull | ||
import org.junit.jupiter.api.assertThrows | ||
import kotlin.test.assertEquals | ||
import kotlin.test.assertFalse | ||
|
@@ -471,4 +472,40 @@ class ServerTest { | |
} | ||
assertEquals("Server does not support resources capability.", exception.message) | ||
} | ||
|
||
@Test | ||
fun `Server constructor should accept instructions parameter`() = runTest { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the test! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The serverInstructions were added to the client as a property that can be verified in these tests. |
||
val serverInfo = Implementation(name = "test server", version = "1.0") | ||
val serverOptions = ServerOptions(capabilities = ServerCapabilities()) | ||
val instructions = "This is a test server. Use it for testing purposes only." | ||
|
||
val server = Server(serverInfo, serverOptions) { instructions } | ||
|
||
// The instructions should be stored internally and used in handleInitialize | ||
// We can't directly access the private field, but we can test it through initialization | ||
val (clientTransport, serverTransport) = InMemoryTransport.createLinkedPair() | ||
val client = Client(clientInfo = Implementation(name = "test client", version = "1.0")) | ||
|
||
server.connect(serverTransport) | ||
client.connect(clientTransport) | ||
|
||
assertEquals(instructions, client.serverInstructions) | ||
} | ||
|
||
@Test | ||
fun `Server constructor should work without instructions parameter`() = runTest { | ||
val serverInfo = Implementation(name = "test server", version = "1.0") | ||
val serverOptions = ServerOptions(capabilities = ServerCapabilities()) | ||
|
||
// Test that server works when instructions parameter is omitted (defaults to null) | ||
val server = Server(serverInfo, serverOptions) | ||
|
||
val (clientTransport, serverTransport) = InMemoryTransport.createLinkedPair() | ||
val client = Client(clientInfo = Implementation(name = "test client", version = "1.0")) | ||
|
||
server.connect(serverTransport) | ||
client.connect(clientTransport) | ||
|
||
assertNull(client.serverInstructions) | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.