diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt index da7d85b35..ab2a854e4 100644 --- a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractResourceIntegrationTest.kt @@ -77,23 +77,6 @@ abstract class AbstractResourceIntegrationTest : KotlinTestBase() { ) } - server.addResource( - uri = testResourceUri, - name = testResourceName, - description = testResourceDescription, - mimeType = "text/plain", - ) { request -> - ReadResourceResult( - contents = listOf( - TextResourceContents( - text = testResourceContent, - uri = request.params.uri, - mimeType = "text/plain", - ), - ), - ) - } - server.addResource( uri = binaryResourceUri, name = binaryResourceName, diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerBulkFeaturesTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerBulkFeaturesTest.kt index 1eb828f9c..52f6908a7 100644 --- a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerBulkFeaturesTest.kt +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerBulkFeaturesTest.kt @@ -1,5 +1,6 @@ package io.modelcontextprotocol.kotlin.sdk.server +import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.collections.shouldBeEmpty import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder import io.kotest.matchers.collections.shouldHaveSize @@ -57,6 +58,39 @@ class ServerBulkFeaturesTest : AbstractServerFeaturesTest() { tools.map { it.name } shouldContainExactlyInAnyOrder listOf("existing", "new-a", "new-b") } + @Test + fun `addTools should throw and register nothing when a tool is already registered`() = runTest { + server.addTool("existing", "Existing") { CallToolResult(emptyList()) } + + shouldThrow { + server.addTools( + listOf( + RegisteredTool(Tool("new-a", ToolSchema())) { CallToolResult(emptyList()) }, + RegisteredTool(Tool("existing", ToolSchema())) { CallToolResult(emptyList()) }, + ), + ) + } + + val tools = client.listTools().tools + + tools shouldHaveSize 1 + tools.map { it.name } shouldContainExactlyInAnyOrder listOf("existing") + } + + @Test + fun `addTools should throw when batch contains duplicate names`() = runTest { + shouldThrow { + server.addTools( + listOf( + RegisteredTool(Tool("dup", ToolSchema())) { CallToolResult(emptyList()) }, + RegisteredTool(Tool("dup", ToolSchema())) { CallToolResult(emptyList()) }, + ), + ) + } + + client.listTools().tools.shouldBeEmpty() + } + // ── addPrompts ───────────────────────────────────────────────────────────── @Test @@ -89,6 +123,25 @@ class ServerBulkFeaturesTest : AbstractServerFeaturesTest() { prompts.map { it.name } shouldContainExactlyInAnyOrder listOf("existing-prompt", "new-p") } + @Test + fun `addPrompts should throw and register nothing when a prompt is already registered`() = runTest { + server.addPrompt("existing-prompt", "Existing") { GetPromptResult(messages = emptyList()) } + + shouldThrow { + server.addPrompts( + listOf( + RegisteredPrompt(Prompt("new-p")) { GetPromptResult(messages = emptyList()) }, + RegisteredPrompt(Prompt("existing-prompt")) { GetPromptResult(messages = emptyList()) }, + ), + ) + } + + val prompts = client.listPrompts().prompts + + prompts shouldHaveSize 1 + prompts.map { it.name } shouldContainExactlyInAnyOrder listOf("existing-prompt") + } + // ── addResources ─────────────────────────────────────────────────────────── @Test @@ -123,6 +176,27 @@ class ServerBulkFeaturesTest : AbstractServerFeaturesTest() { resources.map { it.uri } shouldContainExactlyInAnyOrder listOf("test://existing-res", "test://new-res") } + @Test + fun `addResources should throw and register nothing when a resource is already registered`() = runTest { + server.addResource("test://existing-res", "Existing", "Existing resource") { + ReadResourceResult(emptyList()) + } + + shouldThrow { + server.addResources( + listOf( + RegisteredResource(Resource("test://new-res", "New")) { ReadResourceResult(emptyList()) }, + RegisteredResource(Resource("test://existing-res", "Dup")) { ReadResourceResult(emptyList()) }, + ), + ) + } + + val resources = client.listResources().resources + + resources shouldHaveSize 1 + resources.map { it.uri } shouldContainExactlyInAnyOrder listOf("test://existing-res") + } + // ── Partial-batch remove ─────────────────────────────────────────────────── @Test diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerPromptsTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerPromptsTest.kt index d7ae46df4..44b1e42db 100644 --- a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerPromptsTest.kt +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerPromptsTest.kt @@ -1,5 +1,8 @@ package io.modelcontextprotocol.kotlin.sdk.server +import io.kotest.matchers.string.shouldContain +import io.modelcontextprotocol.kotlin.sdk.types.GetPromptRequest +import io.modelcontextprotocol.kotlin.sdk.types.GetPromptRequestParams import io.modelcontextprotocol.kotlin.sdk.types.GetPromptResult import io.modelcontextprotocol.kotlin.sdk.types.Implementation import io.modelcontextprotocol.kotlin.sdk.types.Method @@ -110,6 +113,26 @@ class ServerPromptsTest : AbstractServerFeaturesTest() { assertFalse(promptListChangedNotificationReceived, "No notification should be sent when prompt doesn't exist") } + @Test + fun `addPrompt should throw when prompt with same name is already registered`() = runTest { + server.addPrompt(Prompt("test-prompt", "Original Prompt", null)) { + GetPromptResult(description = "original", messages = emptyList()) + } + + val exception = assertThrows { + server.addPrompt(Prompt("test-prompt", "Duplicate Prompt", null)) { + GetPromptResult(description = "duplicate", messages = emptyList()) + } + } + exception.message.orEmpty() shouldContain "Prompt \"test-prompt\" is already registered" + + // The original registration is intact + val prompts = client.listPrompts().prompts + assertEquals(1, prompts.size, "Only the original prompt should be registered") + val result = client.getPrompt(GetPromptRequest(GetPromptRequestParams("test-prompt"))) + assertEquals("original", result.description) + } + @Test fun `removePrompt should throw when prompts capability is not supported`() = runTest { var promptListChangedNotificationReceived = false diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerResourceTemplateTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerResourceTemplateTest.kt index c8ace1915..840546979 100644 --- a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerResourceTemplateTest.kt +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerResourceTemplateTest.kt @@ -5,6 +5,7 @@ import io.kotest.matchers.collections.shouldHaveSize import io.kotest.matchers.maps.shouldContainKey import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.shouldBe +import io.kotest.matchers.string.shouldContain import io.modelcontextprotocol.kotlin.sdk.types.Implementation import io.modelcontextprotocol.kotlin.sdk.types.ListResourceTemplatesRequest import io.modelcontextprotocol.kotlin.sdk.types.McpException @@ -162,6 +163,24 @@ class ServerResourceTemplateTest : AbstractServerFeaturesTest() { removed shouldBe false } + @Test + fun `addResourceTemplate should throw when template with same uriTemplate is already registered`() { + server.addResourceTemplate("test://items/{id}", "Item") { _, _ -> + ReadResourceResult(emptyList()) + } + + val exception = assertThrows { + server.addResourceTemplate("test://items/{id}", "Duplicate Item") { _, _ -> + ReadResourceResult(emptyList()) + } + } + exception.message.orEmpty() shouldContain "ResourceTemplate \"test://items/{id}\" is already registered" + + // The original registration is intact + server.resourceTemplates shouldHaveSize 1 + server.resourceTemplates[0].name shouldBe "Item" + } + @Test fun `addResourceTemplate should throw when resources capability is not supported`() { val noResourcesServer = Server( diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerResourcesTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerResourcesTest.kt index fafc3c868..a830788e5 100644 --- a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerResourcesTest.kt +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerResourcesTest.kt @@ -1,7 +1,10 @@ package io.modelcontextprotocol.kotlin.sdk.server +import io.kotest.matchers.string.shouldContain import io.modelcontextprotocol.kotlin.sdk.types.Implementation import io.modelcontextprotocol.kotlin.sdk.types.Method +import io.modelcontextprotocol.kotlin.sdk.types.ReadResourceRequest +import io.modelcontextprotocol.kotlin.sdk.types.ReadResourceRequestParams import io.modelcontextprotocol.kotlin.sdk.types.ReadResourceResult import io.modelcontextprotocol.kotlin.sdk.types.ResourceListChangedNotification import io.modelcontextprotocol.kotlin.sdk.types.ServerCapabilities @@ -115,6 +118,24 @@ class ServerResourcesTest : AbstractServerFeaturesTest() { ) } + @Test + fun `addResource should throw when resource with same uri is already registered`() = runTest { + server.addResource("test://resource", "Original", "Original resource") { + ReadResourceResult(listOf(TextResourceContents(text = "original", uri = "test://resource"))) + } + + val exception = assertThrows { + server.addResource("test://resource", "Duplicate", "Duplicate resource") { + ReadResourceResult(listOf(TextResourceContents(text = "duplicate", uri = "test://resource"))) + } + } + exception.message.orEmpty() shouldContain "Resource \"test://resource\" is already registered" + + // The original registration is intact + val result = client.readResource(ReadResourceRequest(ReadResourceRequestParams("test://resource"))) + assertEquals("original", (result.contents.single() as TextResourceContents).text) + } + @Test fun `removeResource should throw when resources capability is not supported`() = runTest { // Create server without resources capability diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsNotificationTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsNotificationTest.kt index 357208212..9ac744bf2 100644 --- a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsNotificationTest.kt +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsNotificationTest.kt @@ -11,6 +11,7 @@ import kotlinx.coroutines.test.runTest import org.awaitility.kotlin.await import org.awaitility.kotlin.untilAsserted import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -81,6 +82,27 @@ class ServerToolsNotificationTest : AbstractServerFeaturesTest() { } } + @Test + fun `addTool should not send notification when duplicate is rejected`() = runTest { + // Track notifications + val notifications = mutableListOf() + client.setNotificationHandler( + Method.Defined.NotificationsToolsListChanged, + ) { notification -> + notifications.add(notification) + CompletableDeferred(Unit) + } + + server.addTool("test-tool", "Test Tool") { CallToolResult(emptyList()) } + assertThrows { + server.addTool("test-tool", "Duplicate Tool") { CallToolResult(emptyList()) } + } + // Close the server to stop processing further events and flush notifications + server.close() + + assertEquals(1, notifications.size, "Only the first registration should send a notification") + } + @Test fun `notification should not be send when removed tool does not exists`() = runTest { // Track notifications diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsTest.kt index 54c998e94..1b3df9179 100644 --- a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsTest.kt +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsTest.kt @@ -1,5 +1,8 @@ package io.modelcontextprotocol.kotlin.sdk.server +import io.kotest.matchers.string.shouldContain +import io.modelcontextprotocol.kotlin.sdk.types.CallToolRequest +import io.modelcontextprotocol.kotlin.sdk.types.CallToolRequestParams import io.modelcontextprotocol.kotlin.sdk.types.CallToolResult import io.modelcontextprotocol.kotlin.sdk.types.Implementation import io.modelcontextprotocol.kotlin.sdk.types.Method @@ -108,6 +111,42 @@ class ServerToolsTest : AbstractServerFeaturesTest() { assertTrue(result, "Tool with non-conforming name should have been registered and removable") } + @Test + fun `addTool should throw when tool with same name is already registered`() = runTest { + server.addTool("test-tool", "Original Tool") { + CallToolResult(listOf(TextContent("Original result"))) + } + + val exception = assertThrows { + server.addTool("test-tool", "Duplicate Tool") { + CallToolResult(listOf(TextContent("Duplicate result"))) + } + } + exception.message.orEmpty() shouldContain "Tool \"test-tool\" is already registered" + + // The original registration is intact + val tools = client.listTools().tools + assertEquals(1, tools.size, "Only the original tool should be registered") + val result = client.callTool(CallToolRequest(CallToolRequestParams("test-tool"))) + assertEquals("Original result", (result.content.single() as TextContent).text) + } + + @Test + fun `addTool should succeed after removing existing tool`() = runTest { + server.addTool("test-tool", "Original Tool") { + CallToolResult(listOf(TextContent("Original result"))) + } + + server.removeTool("test-tool") + server.addTool("test-tool", "Replacement Tool") { + CallToolResult(listOf(TextContent("Replacement result"))) + } + + assertEquals(1, client.listTools().tools.size, "Re-adding after removal should not duplicate the tool") + val result = client.callTool(CallToolRequest(CallToolRequestParams("test-tool"))) + assertEquals("Replacement result", (result.content.single() as TextContent).text) + } + @Test fun `removeTool should throw when tools capability is not supported`() = runTest { var toolListChangedNotificationReceived = false diff --git a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/FeatureRegistry.kt b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/FeatureRegistry.kt index 0ed655884..4adbee1fa 100644 --- a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/FeatureRegistry.kt +++ b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/FeatureRegistry.kt @@ -20,9 +20,12 @@ internal interface FeatureListener { * A generic registry for managing features of a specified type. This class provides thread-safe * operations for adding, removing, and retrieving features from the registry. * + * Feature keys are unique: [add] and [addAll] reject keys that are already registered. + * To replace a feature, [remove] it first and add the new one. + * * @param T The type of the feature, constrained to implement the [Feature] interface. * @param featureType A string description of the type of feature being managed. - * Used primarily for logging purposes. + * Used for logging and error messages. */ internal class FeatureRegistry(private val featureType: String) { @@ -55,30 +58,48 @@ internal class FeatureRegistry(private val featureType: String) { * Adds the specified feature to the registry. * * @param feature The feature to be added to the registry. + * @throws IllegalArgumentException If a feature with the same key is already registered. */ internal fun add(feature: T) { logger.info { "Adding $featureType: \"${feature.key}\"" } - val oldMap = registry.getAndUpdate { current -> current.put(feature.key, feature) } - val oldFeature = oldMap[feature.key] + registry.update { current -> + require(!current.containsKey(feature.key)) { + "$featureType \"${feature.key}\" is already registered. Remove it first to replace it." + } + current.put(feature.key, feature) + } logger.info { "Added $featureType: \"${feature.key}\"" } - notifyFeatureUpdated(oldFeature, feature) + notifyFeatureUpdated(null, feature) } /** * Adds the given list of features to the registry. Each feature is mapped by its key - * and added to the current registry. + * and added to the current registry. The operation is all-or-nothing: if any key is + * rejected, no features are added. * * @param features The list of features to add to the registry. + * @throws IllegalArgumentException If the list contains duplicate keys or a feature with + * the same key is already registered. */ internal fun addAll(features: List) { logger.info { "Adding ${featureType}s: ${features.size}" } - val oldMap = registry.getAndUpdate { current -> current.putAll(features.associateBy { it.key }) } + val newEntries = features.associateBy { it.key } + require(newEntries.size == features.size) { + val duplicated = features.groupingBy { it.key }.eachCount().filterValues { it > 1 }.keys + "Duplicate $featureType keys in batch: $duplicated" + } + registry.update { current -> + val conflicting = newEntries.keys.filter { current.containsKey(it) } + require(conflicting.isEmpty()) { + "${featureType}s already registered: $conflicting. Remove them first to replace them." + } + current.putAll(newEntries) + } logger.info { "Added ${featureType}s: ${features.size}" } for (feature in features) { - val oldFeature = oldMap[feature.key] - notifyFeatureUpdated(oldFeature, feature) + notifyFeatureUpdated(null, feature) } } diff --git a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt index c0d3e8493..e9980c0b4 100644 --- a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt +++ b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt @@ -299,6 +299,8 @@ public open class Server( * @param tool A [Tool] object describing the tool. * @param handler A suspend function that handles executing the tool when called by the client. * @throws IllegalStateException If the server does not support tools. + * @throws IllegalArgumentException If a tool with the same name is already registered. + * Use [removeTool] first to replace it. */ public fun addTool(tool: Tool, handler: suspend ClientConnection.(CallToolRequest) -> CallToolResult) { checkNotNull(options.capabilities.tools) { @@ -322,6 +324,8 @@ public open class Server( * @param meta Optional metadata as a [JsonObject]. * @param handler A suspend function that handles executing the tool when called by the client. * @throws IllegalStateException If the server does not support tools. + * @throws IllegalArgumentException If a tool with the same name is already registered. + * Use [removeTool] first to replace it. */ public fun addTool( name: String, @@ -352,6 +356,8 @@ public open class Server( * * @param toolsToAdd A list of [RegisteredTool] objects representing the tools to register. * @throws IllegalStateException If the server does not support tools. + * @throws IllegalArgumentException If [toolsToAdd] contains duplicate names or a tool with + * the same name is already registered. No tools are registered in that case. */ public fun addTools(toolsToAdd: List) { checkNotNull(options.capabilities.tools) { @@ -400,6 +406,8 @@ public open class Server( * @param prompt A [Prompt] object describing the prompt. * @param promptProvider A suspend function that returns the prompt content when requested by the client. * @throws IllegalStateException If the server does not support prompts. + * @throws IllegalArgumentException If a prompt with the same name is already registered. + * Use [removePrompt] first to replace it. */ public fun addPrompt( prompt: Prompt, @@ -420,6 +428,8 @@ public open class Server( * @param arguments An optional list of [PromptArgument] that the prompt accepts. * @param promptProvider A suspend function that returns the prompt content when requested. * @throws IllegalStateException If the server does not support prompts. + * @throws IllegalArgumentException If a prompt with the same name is already registered. + * Use [removePrompt] first to replace it. */ public fun addPrompt( name: String, @@ -436,6 +446,8 @@ public open class Server( * * @param promptsToAdd A list of [RegisteredPrompt] objects representing the prompts to register. * @throws IllegalStateException If the server does not support prompts. + * @throws IllegalArgumentException If [promptsToAdd] contains duplicate names or a prompt with + * the same name is already registered. No prompts are registered in that case. */ public fun addPrompts(promptsToAdd: List) { checkNotNull(options.capabilities.prompts) { @@ -486,6 +498,8 @@ public open class Server( * @param mimeType The MIME type of the resource content. * @param readHandler A suspend function that returns the resource content when read by the client. * @throws IllegalStateException If the server does not support resources. + * @throws IllegalArgumentException If a resource with the same URI is already registered. + * Use [removeResource] first to replace it. */ public fun addResource( uri: String, @@ -507,6 +521,8 @@ public open class Server( * * @param resourcesToAdd A list of [RegisteredResource] objects representing the resources to register. * @throws IllegalStateException If the server does not support resources. + * @throws IllegalArgumentException If [resourcesToAdd] contains duplicate URIs or a resource with + * the same URI is already registered. No resources are registered in that case. */ public fun addResources(resourcesToAdd: List) { checkNotNull(options.capabilities.resources) { @@ -554,6 +570,8 @@ public open class Server( * @param readHandler A suspend function invoked when a client reads a URI that matches * the template. The second parameter contains the URI variables extracted from the match. * @throws IllegalStateException If the server does not support resources. + * @throws IllegalArgumentException If a template with the same URI template is already registered. + * Use [removeResourceTemplate] first to replace it. */ public fun addResourceTemplate( template: ResourceTemplate, @@ -582,6 +600,8 @@ public open class Server( * @param readHandler A suspend function invoked when a client reads a URI that matches * the template. The second parameter contains the URI variables extracted from the match. * @throws IllegalStateException If the server does not support resources. + * @throws IllegalArgumentException If a template with the same URI template is already registered. + * Use [removeResourceTemplate] first to replace it. */ public fun addResourceTemplate( uriTemplate: String, diff --git a/kotlin-sdk-server/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/FeatureRegistryTest.kt b/kotlin-sdk-server/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/FeatureRegistryTest.kt new file mode 100644 index 000000000..c6bf72b8d --- /dev/null +++ b/kotlin-sdk-server/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/FeatureRegistryTest.kt @@ -0,0 +1,106 @@ +package io.modelcontextprotocol.kotlin.sdk.server + +import io.kotest.matchers.shouldBe +import kotlin.test.Test +import kotlin.test.assertFailsWith + +private data class TestFeature(override val key: FeatureKey, val payload: String = "") : Feature + +private class RecordingListener : FeatureListener { + val updatedKeys = mutableListOf() + + override fun onFeatureUpdated(featureKey: String) { + updatedKeys.add(featureKey) + } +} + +class FeatureRegistryTest { + + private val registry = FeatureRegistry("Tool") + private val listener = RecordingListener() + + init { + registry.addListener(listener) + } + + @Test + fun `add should register feature and notify listener`() { + val feature = TestFeature("a") + + registry.add(feature) + + registry.values shouldBe mapOf("a" to feature) + listener.updatedKeys shouldBe listOf("a") + } + + @Test + fun `add should throw when key is already registered`() { + registry.add(TestFeature("a")) + + val exception = assertFailsWith { + registry.add(TestFeature("a", payload = "override")) + } + + exception.message shouldBe "Tool \"a\" is already registered. Remove it first to replace it." + } + + @Test + fun `add should keep original feature and not notify when duplicate is rejected`() { + val original = TestFeature("a", payload = "original") + registry.add(original) + + assertFailsWith { + registry.add(TestFeature("a", payload = "override")) + } + + registry.values shouldBe mapOf("a" to original) + listener.updatedKeys shouldBe listOf("a") + } + + @Test + fun `addAll should register all features and notify per feature`() { + val a = TestFeature("a") + val b = TestFeature("b") + + registry.addAll(listOf(a, b)) + + registry.values shouldBe mapOf("a" to a, "b" to b) + listener.updatedKeys shouldBe listOf("a", "b") + } + + @Test + fun `addAll should throw on duplicate keys within batch and register nothing`() { + val exception = assertFailsWith { + registry.addAll(listOf(TestFeature("a"), TestFeature("b"), TestFeature("a"))) + } + + exception.message shouldBe "Duplicate Tool keys in batch: [a]" + registry.values shouldBe emptyMap() + listener.updatedKeys shouldBe emptyList() + } + + @Test + fun `addAll should throw when any key is already registered and register nothing`() { + val existing = TestFeature("existing") + registry.add(existing) + + val exception = assertFailsWith { + registry.addAll(listOf(TestFeature("new"), TestFeature("existing", payload = "override"))) + } + + exception.message shouldBe "Tools already registered: [existing]. Remove them first to replace them." + registry.values shouldBe mapOf("existing" to existing) + listener.updatedKeys shouldBe listOf("existing") + } + + @Test + fun `add should succeed after the existing feature is removed`() { + registry.add(TestFeature("a", payload = "original")) + registry.remove("a") + + registry.add(TestFeature("a", payload = "replacement")) + + registry.values shouldBe mapOf("a" to TestFeature("a", payload = "replacement")) + listener.updatedKeys shouldBe listOf("a", "a", "a") + } +}