From f96eab7d630cbc03aa90a45efc889932e0eb32a3 Mon Sep 17 00:00:00 2001 From: LordKay-sudo Date: Sat, 27 Jun 2026 15:46:35 +0200 Subject: [PATCH 1/8] feat(mcp): expose DICE tools for MCP server integration (fixes #5) Context-scoped DiceMcpTools (recall, list, store, get) and opt-in dice-mcp-autoconfigure. MCP clients pass context_id per call; get refuses cross-context ids. Discovery and extraction stay off this surface. Signed-off-by: LordKay-sudo --- AGENTS.md | 5 +- README.md | 36 ++ dice-mcp-autoconfigure/AGENTS.md | 40 ++ dice-mcp-autoconfigure/pom.xml | 85 +++ .../autoconfigure/DiceMcpAutoConfiguration.kt | 84 +++ .../mcp/autoconfigure/DiceMcpProperties.kt | 39 ++ ...ot.autoconfigure.AutoConfiguration.imports | 1 + .../DiceMcpAutoConfigurationTest.kt | 232 +++++++ dice/AGENTS.md | 1 + .../com/embabel/dice/mcp/DiceMcpSupport.kt | 82 +++ .../com/embabel/dice/mcp/DiceMcpTools.kt | 174 ++++++ .../com/embabel/dice/mcp/DiceMcpToolsTest.kt | 575 ++++++++++++++++++ docs/design/INDEX.md | 6 +- docs/design/architecture.md | 24 +- docs/design/retrieval-and-discovery.md | 4 + pom.xml | 6 + 16 files changed, 1386 insertions(+), 8 deletions(-) create mode 100644 dice-mcp-autoconfigure/AGENTS.md create mode 100644 dice-mcp-autoconfigure/pom.xml create mode 100644 dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfiguration.kt create mode 100644 dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt create mode 100644 dice-mcp-autoconfigure/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports create mode 100644 dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt create mode 100644 dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt create mode 100644 dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt diff --git a/AGENTS.md b/AGENTS.md index a789a8b9..b6221e71 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,13 +6,14 @@ DICE (Domain-Integrated Context Engineering) is a proposition-first knowledge su | Module | What it owns | |---|---| -| `dice` | The entire domain: `Proposition` model, `PropositionStore`/`PropositionRepository` SPIs, extraction pipeline, revision/conflict detection, entity resolution, projectors (graph, Prolog, memory), graph and discovery query/retrieval, incremental analysis, in-memory and file-backed stores, tuProlog integration, REST endpoints | +| `dice` | The entire domain: `Proposition` model, `PropositionStore`/`PropositionRepository` SPIs, extraction pipeline, revision/conflict detection, entity resolution, projectors (graph, Prolog, memory), graph and discovery query/retrieval, incremental analysis, in-memory and file-backed stores, tuProlog integration, REST endpoints, MCP tool surface (`DiceMcpTools`) | | `dice-storage` | Drivine/Neo4j implementation of `PropositionRepository`, `ChunkHistoryStore`, and `DecayManager`; uses Kotlin 2.2 for the Drivine KSP-generated query DSL | | `dice-storage-autoconfigure` | Spring Boot auto-configuration that wires the right backend based on `embabel.dice.store.type`, schedules the decay tick, and provides auto-configuration for the multi-signal duplicate collector (properties prefix `embabel.dice.collector`) | | `dice-report` | Output projectors over propositions: rationale (why a fact is believed, with evidence), structured report, and surprising-link discovery | | `dice-ingestion` | Ingestion SPI (artifacts → chunks) with a content-hash dedup ledger so the same source isn't extracted twice | | `dice-metamodel` | Schema versioning: `MetamodelVersion` content-hash stamps over the governed types of a `DataDictionary`, `GovernedTypeSelector`, the `DeclaredSchemaSource` opt-in, and the `MetamodelVersionStore` contract. Pure JVM, with no dependency on `dice` | | `dice-integration-tests` | Test-only: the cross-feature end-to-end canonical-flow harness | +| `dice-mcp-autoconfigure` | Spring Boot auto-configuration that exports `DiceMcpTools` over MCP via embabel-agent when `embabel.dice.mcp.enabled=true` | ## Build & test @@ -66,6 +67,7 @@ The `dice` module is organized by responsibility: | `com.embabel.dice.provenance` | `ProvenanceEntry`, `SourceLocator` — rich evidence links from propositions back to source material | | `com.embabel.dice.query.oracle` | `Oracle`, `LlmOracle`, `PrologTools` — natural language question answering over propositions | | `com.embabel.dice.web.rest` | Optional REST endpoints for the pipeline and memory; activated by `spring-webmvc` on the classpath | +| `com.embabel.dice.mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `context_id` on every call | ## Conventions @@ -85,6 +87,7 @@ The `dice` module is organized by responsibility: - **Adding or changing extraction logic** → `com.embabel.dice.proposition.extraction.LlmPropositionExtractor` and the Mustache prompt templates in `dice/src/main/resources/dice/`. - **Wiring a new Spring Boot app** → `dice-storage-autoconfigure`, specifically `DiceStorageAutoConfiguration` (backend selection) and `DiceStoreProperties` (property keys). Set `embabel.dice.store.type=graph` for Neo4j. +- **Exposing DICE over MCP** → `DiceMcpTools` in `com.embabel.dice.mcp` for the tool surface; `dice-mcp-autoconfigure` + `embabel-agent-starter-mcpserver` for zero-code export (`embabel.dice.mcp.enabled=true`). - **Understanding the proposition data model** → `Proposition.kt` in `com.embabel.dice.proposition`. Every field is documented inline. - **Adding a new entity resolver strategy** → implement `CandidateSearcher` in `com.embabel.dice.common.resolver.searcher`, then compose it into an `EscalatingEntityResolver`. - **Writing integration tests against Neo4j** → look at `DrivinePropositionStoreIntegrationTest` in `dice-storage/src/test`; it shows the `@SpringBootTest` + Testcontainers pattern in use. diff --git a/README.md b/README.md index fd9e296c..d4f222bb 100644 --- a/README.md +++ b/README.md @@ -2414,6 +2414,42 @@ Everything is pushed into the database rather than scanned in memory: > `dice-storage/HANDOFF.md` for architecture and `dice-storage/INTEGRATE-INTO-ASSISTANT.md` for a > migration walkthrough. +### MCP Server + +Expose DICE recall/list/store/get to an MCP client (Claude Desktop, Cursor, etc.) with +`dice-mcp-autoconfigure` and embabel-agent's MCP server starter. Off until you set +`embabel.dice.mcp.enabled=true`. Every tool takes a `context_id` — MCP clients are stateless, +unlike in-process `Memory` / `DiscoveryTools` which bake context in at construction. + +```xml + + com.embabel.dice + dice-mcp-autoconfigure + ${dice.version} + + + com.embabel.agent + embabel-agent-starter-mcpserver + ${embabel-agent.version} + +``` + +```yaml +embabel: + dice: + mcp: + enabled: true +``` + +| Tool | Description | +|------|-------------| +| `dice_recall` | Hybrid semantic + keyword search in a `context_id` | +| `dice_list` | List active propositions for a context | +| `dice_store` | Store a proposition directly | +| `dice_get` | Fetch one proposition by id, refused if it belongs to another context | + +Discovery and graph tools stay on `DiscoveryTools.asTools(...)` / `GraphQueryTools.asTools(...)`. + ### API Key Security DICE provides API key authentication for the REST endpoints. Enable it via configuration: diff --git a/dice-mcp-autoconfigure/AGENTS.md b/dice-mcp-autoconfigure/AGENTS.md new file mode 100644 index 00000000..caa5afcd --- /dev/null +++ b/dice-mcp-autoconfigure/AGENTS.md @@ -0,0 +1,40 @@ +# `dice-mcp-autoconfigure` module — Agent Navigation Guide + +Spring Boot wiring that exports [DiceMcpTools](../dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt) +over embabel-agent's MCP server. No domain logic — just an `@AutoConfiguration` that assembles beans +from `dice`. The isolation rule (`context_id` on every tool) lives on `DiceMcpTools` itself. + +## What's here + +- **`DiceMcpAutoConfiguration`** — `DiceMcpTools` + named `diceMcpToolExport` (`McpToolExport`). + Opt-in via `embabel.dice.mcp.enabled=true`. Requires `McpToolExport` on the classpath and a + `PropositionRepository` bean. `afterName` waits for `DiceStorageAutoConfiguration` when that + module is present so the store bean exists before `@ConditionalOnBean` is asked. +- **`DiceMcpProperties`** — `embabel.dice.mcp`: `enabled` (default false), `min-confidence` + (default 0.5), `default-limit` (default 10). + +## Property reference + +| Property | Default | Meaning | +|---|---|---| +| `embabel.dice.mcp.enabled` | `false` | Master switch. Off means no beans. | +| `embabel.dice.mcp.min-confidence` | `0.5` | Minimum effective confidence for recall/list | +| `embabel.dice.mcp.default-limit` | `10` | Default result cap for recall/list | + +Every collaborator is `@ConditionalOnMissingBean`, so an app's own `DiceMcpTools` or +`diceMcpToolExport` bean wins. + +## Dependencies + +- `dice` — `DiceMcpTools` and the proposition store SPI. +- `embabel-agent-mcpserver` (optional) — `McpToolExport`. The host also adds + `embabel-agent-starter-mcpserver`. +- `embabel-agent-api` (provided) — supplied at runtime by the consuming application. + +## Gotchas + +- MCP export is **opt-in**. Unlike the collector (`enabled` default true), this stays dark until + `embabel.dice.mcp.enabled=true`. +- Without a `PropositionRepository` bean the auto-config class may load but it exports nothing. +- Discovery and graph tools are not on this path. They bake context in at construction; use + `DiscoveryTools.asTools(...)` / `GraphQueryTools.asTools(...)` for in-process agents. diff --git a/dice-mcp-autoconfigure/pom.xml b/dice-mcp-autoconfigure/pom.xml new file mode 100644 index 00000000..39aef2c6 --- /dev/null +++ b/dice-mcp-autoconfigure/pom.xml @@ -0,0 +1,85 @@ + + + 4.0.0 + + com.embabel.dice + dice-parent + 0.2.0-SNAPSHOT + + dice-mcp-autoconfigure + jar + Dice MCP Autoconfigure + Spring Boot auto-configuration that exports DICE tools over MCP via embabel-agent + + + + com.embabel.dice + dice + + + + com.embabel.agent + embabel-agent-mcpserver + true + + + + com.embabel.agent + embabel-agent-api + provided + + + com.embabel.agent + embabel-agent-rag-core + provided + + + + org.springframework.boot + spring-boot-autoconfigure + + + org.springframework.boot + spring-boot-configuration-processor + true + + + + org.jetbrains.kotlin + kotlin-stdlib + + + + org.slf4j + slf4j-api + + + + org.springframework.boot + spring-boot-starter-test + test + + + org.assertj + assertj-core + test + + + + + + + org.jetbrains.kotlin + kotlin-maven-plugin + + + -Xjsr305=strict + -Xjvm-default=all + + + + + + + diff --git a/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfiguration.kt b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfiguration.kt new file mode 100644 index 00000000..15776a23 --- /dev/null +++ b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfiguration.kt @@ -0,0 +1,84 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.mcp.autoconfigure + +import com.embabel.agent.api.tool.ToolObject +import com.embabel.agent.mcpserver.McpToolExport +import com.embabel.dice.mcp.DiceMcpTools +import com.embabel.dice.proposition.PropositionRepository +import org.slf4j.LoggerFactory +import org.springframework.boot.autoconfigure.AutoConfiguration +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty +import org.springframework.boot.context.properties.EnableConfigurationProperties +import org.springframework.context.annotation.Bean + +/** + * Registers [DiceMcpTools] and exports them as MCP tools when embabel-agent's MCP server is present. + * + * Typical application dependencies: + * ```xml + * + * com.embabel.dice + * dice-mcp-autoconfigure + * + * + * com.embabel.agent + * embabel-agent-starter-mcpserver + * + * ``` + * + * ```yaml + * embabel: + * dice: + * mcp: + * enabled: true + * ``` + * + * `afterName` waits for `dice-storage-autoconfigure` when that module is on the classpath, so + * `@ConditionalOnBean(PropositionRepository)` sees the store bean. If storage autoconfig is absent, + * the named class is ignored and the host supplies its own repository. + */ +@AutoConfiguration(afterName = ["com.embabel.dice.storage.autoconfigure.DiceStorageAutoConfiguration"]) +@ConditionalOnClass(McpToolExport::class) +@ConditionalOnProperty(prefix = "embabel.dice.mcp", name = ["enabled"], havingValue = "true") +@EnableConfigurationProperties(DiceMcpProperties::class) +class DiceMcpAutoConfiguration { + + private val logger = LoggerFactory.getLogger(DiceMcpAutoConfiguration::class.java) + + @Bean + @ConditionalOnBean(PropositionRepository::class) + @ConditionalOnMissingBean(DiceMcpTools::class) + fun diceMcpTools( + repository: PropositionRepository, + properties: DiceMcpProperties, + ): DiceMcpTools = DiceMcpTools( + repository = repository, + minConfidence = properties.minConfidence, + defaultLimit = properties.defaultLimit, + ) + + @Bean("diceMcpToolExport") + @ConditionalOnBean(DiceMcpTools::class) + @ConditionalOnMissingBean(name = ["diceMcpToolExport"]) + fun diceMcpToolExport(tools: DiceMcpTools): McpToolExport { + logger.info("Exporting DICE MCP tools: {}", DiceMcpTools.TOOL_NAMES.sorted()) + return McpToolExport.fromToolObject(ToolObject(objects = listOf(tools))) + } +} diff --git a/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt new file mode 100644 index 00000000..4d72f54c --- /dev/null +++ b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt @@ -0,0 +1,39 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.mcp.autoconfigure + +import org.springframework.boot.context.properties.ConfigurationProperties + +/** + * Configuration for exporting DICE as MCP tools. + * + * Requires `embabel-agent-starter-mcpserver` (or `embabel-agent-mcpserver`) on the classpath + * and `embabel.dice.mcp.enabled=true`. + */ +@ConfigurationProperties(prefix = "embabel.dice.mcp") +data class DiceMcpProperties( + /** Master switch. Default false so MCP export is opt-in. */ + val enabled: Boolean = false, + /** Minimum effective confidence for recall/list tools (0.0–1.0). */ + val minConfidence: Double = 0.5, + /** Default result limit for recall/list tools. */ + val defaultLimit: Int = 10, +) { + init { + require(minConfidence in 0.0..1.0) { "embabel.dice.mcp.min-confidence must be between 0.0 and 1.0" } + require(defaultLimit > 0) { "embabel.dice.mcp.default-limit must be positive" } + } +} diff --git a/dice-mcp-autoconfigure/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports b/dice-mcp-autoconfigure/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports new file mode 100644 index 00000000..33211003 --- /dev/null +++ b/dice-mcp-autoconfigure/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports @@ -0,0 +1 @@ +com.embabel.dice.mcp.autoconfigure.DiceMcpAutoConfiguration diff --git a/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt b/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt new file mode 100644 index 00000000..2b2c7e9b --- /dev/null +++ b/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt @@ -0,0 +1,232 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.mcp.autoconfigure + +import com.embabel.agent.api.tool.ToolObject +import com.embabel.agent.mcpserver.McpToolExport +import com.embabel.dice.mcp.DiceMcpTools +import com.embabel.dice.proposition.PropositionRepository +import com.embabel.dice.proposition.store.InMemoryPropositionRepository +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.getBean +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.FilteredClassLoader +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration + +/** + * `ApplicationContextRunner` wiring tests for [DiceMcpAutoConfiguration]: no MCP server process, + * no Neo4j — the autoconfiguration plus a stub [PropositionRepository] and `embabel.dice.mcp.*`. + */ +class DiceMcpAutoConfigurationTest { + + private val runner = ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(DiceMcpAutoConfiguration::class.java)) + + @Test + fun `disabled by default so no tools or export beans`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .run { ctx -> + assertThat(ctx).doesNotHaveBean(DiceMcpAutoConfiguration::class.java) + assertThat(ctx).doesNotHaveBean(DiceMcpTools::class.java) + assertThat(ctx).doesNotHaveBean(McpToolExport::class.java) + } + } + + @Test + fun `master switch off means no beans`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues("embabel.dice.mcp.enabled=false") + .run { ctx -> + assertThat(ctx).doesNotHaveBean(DiceMcpTools::class.java) + assertThat(ctx).doesNotHaveBean(McpToolExport::class.java) + } + } + + @Test + fun `enabled without a PropositionRepository exports nothing`() { + runner + .withPropertyValues("embabel.dice.mcp.enabled=true") + .run { ctx -> + assertThat(ctx).hasSingleBean(DiceMcpAutoConfiguration::class.java) + assertThat(ctx).doesNotHaveBean(DiceMcpTools::class.java) + assertThat(ctx).doesNotHaveBean("diceMcpToolExport") + } + } + + @Test + fun `enabled with a repository wires tools and exports the four names`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues("embabel.dice.mcp.enabled=true") + .run { ctx -> + assertThat(ctx).hasSingleBean(DiceMcpTools::class.java) + assertThat(ctx).hasBean("diceMcpToolExport") + + val export = ctx.getBean("diceMcpToolExport") + val names = export.toolCallbacks.map { it.toolDefinition.name() }.toSet() + assertThat(names).isEqualTo(DiceMcpTools.TOOL_NAMES) + } + } + + @Test + fun `minConfidence and defaultLimit bind from properties`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.min-confidence=0.7", + "embabel.dice.mcp.default-limit=3", + ) + .run { ctx -> + val props = ctx.getBean() + assertThat(props.minConfidence).isEqualTo(0.7) + assertThat(props.defaultLimit).isEqualTo(3) + } + } + + @Test + fun `invalid minConfidence fails context startup`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.min-confidence=1.5", + ) + .run { ctx -> + assertThat(ctx).hasFailed() + } + } + + @Test + fun `invalid defaultLimit fails context startup`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.default-limit=0", + ) + .run { ctx -> + assertThat(ctx).hasFailed() + } + } + + @Test + fun `bound minConfidence is applied to the tools bean`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.min-confidence=0.7", + ) + .run { ctx -> + val tools = ctx.getBean() + tools.storeMemory("session-1", "Weak guess", confidence = 0.6) + tools.storeMemory("session-1", "Strong fact", confidence = 0.9) + val listed = tools.listMemories("session-1", limit = 10) + assertThat(listed).contains("Strong fact") + assertThat(listed).doesNotContain("Weak guess") + } + } + + @Test + fun `bound defaultLimit is applied when list is called without a limit`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.min-confidence=0.0", + "embabel.dice.mcp.default-limit=2", + ) + .run { ctx -> + val tools = ctx.getBean() + tools.storeMemory("session-1", "Fact one", confidence = 0.9) + tools.storeMemory("session-1", "Fact two", confidence = 0.8) + tools.storeMemory("session-1", "Fact three", confidence = 0.7) + val listed = tools.listMemories("session-1") + val numbered = listed.lines().count { it.matches(Regex("^\\d+\\..*")) } + assertThat(numbered).isEqualTo(2) + } + } + + @Test + fun `no McpToolExport on the classpath means no auto-config`() { + runner + .withClassLoader(FilteredClassLoader(McpToolExport::class.java)) + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues("embabel.dice.mcp.enabled=true") + .run { ctx -> + assertThat(ctx).doesNotHaveBean(DiceMcpAutoConfiguration::class.java) + assertThat(ctx).doesNotHaveBean(DiceMcpTools::class.java) + } + } + + @Test + fun `a custom DiceMcpTools bean wins over the default`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java, CustomToolsConfig::class.java) + .withPropertyValues("embabel.dice.mcp.enabled=true") + .run { ctx -> + assertThat(ctx).hasSingleBean(DiceMcpTools::class.java) + assertThat(ctx.getBean()).isSameAs(CustomToolsConfig.INSTANCE) + assertThat(ctx).hasBean("diceMcpToolExport") + } + } + + @Test + fun `a custom diceMcpToolExport bean wins over the default`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java, CustomExportConfig::class.java) + .withPropertyValues("embabel.dice.mcp.enabled=true") + .run { ctx -> + assertThat(ctx).hasBean("diceMcpToolExport") + assertThat(ctx.getBean("diceMcpToolExport")) + .isSameAs(CustomExportConfig.INSTANCE) + } + } + + @Configuration(proxyBeanMethods = false) + private class StubPropositionRepositoryConfig { + @Bean + fun propositionRepository(): PropositionRepository = InMemoryPropositionRepository() + } + + @Configuration(proxyBeanMethods = false) + private class CustomToolsConfig { + @Bean + fun diceMcpTools(): DiceMcpTools = INSTANCE + + companion object { + val INSTANCE = DiceMcpTools(InMemoryPropositionRepository()) + } + } + + @Configuration(proxyBeanMethods = false) + private class CustomExportConfig { + @Bean(name = ["diceMcpToolExport"]) + fun diceMcpToolExport(): McpToolExport = INSTANCE + + companion object { + val INSTANCE: McpToolExport = McpToolExport.fromToolObject( + ToolObject(objects = listOf(DiceMcpTools(InMemoryPropositionRepository()))), + ) + } + } +} diff --git a/dice/AGENTS.md b/dice/AGENTS.md index 364fa122..1d058063 100644 --- a/dice/AGENTS.md +++ b/dice/AGENTS.md @@ -76,6 +76,7 @@ These map onto the lifecycle in [proposition-lifecycle](../docs/design/propositi | `query.discovery` | `RetrievalRouter`, `DiscoveryQuery`, `RetrievalMode`, discovery DTOs — mode-routed retrieval entry point | | `temporal` | `TemporalMetadata` — bitemporal valid/observed windows, explicit retraction | | `agent` | `Memory`, `MemoryRetriever` (agent-facing view), `ProvenanceResolver`; **`DiscoveryTools`** — `@LlmTool`-annotated tools wrapping `RetrievalRouter` (query propositions, graph path, why-explain, projection health, collector dry-run) with context baked in at construction so an agent can't cross context boundaries; **`GraphQueryTools`** — `@LlmTool`-annotated tools wrapping the `GraphQuery` facade (entity neighbourhood, path between entities, why-explain) | +| `mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `context_id` on every call so a stateless client cannot cross tenants | | `web.rest` | `PropositionPipelineController`, `MemoryController`, **`DiscoveryController`** — REST surface for discovery operations (`/api/v1/contexts/{contextId}/discovery`; routes query, path, why, projection health, and collector dry-run; context comes from the URL path only), API key security — optional, activated by `spring-webmvc` | | `operations` | `PropositionAbstractor`, `PropositionContraster` — higher-level proposition management | | `operations.consolidation` | The dream-loop steps as composable passes: `ConsolidationPass`/`ConsolidationPassResult`, `SessionConsolidationPass`, `AbstractionPass`, `ContradictionResolutionPass`, `DecaySweepPass` | diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt new file mode 100644 index 00000000..b455ec4f --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt @@ -0,0 +1,82 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.mcp + +import com.embabel.agent.api.tool.Tool +import com.embabel.agent.core.ContextId +import com.embabel.dice.agent.MemoryRetriever +import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.PropositionQuery +import com.embabel.dice.proposition.PropositionRepository +import com.embabel.dice.proposition.PropositionStatus + +/** + * Shared retrieval and formatting helpers for [DiceMcpTools]. + * + * Delegates hybrid recall to [MemoryRetriever] so MCP and in-process [com.embabel.dice.agent.Memory] + * stay aligned on vector + keyword + entity-expansion behaviour without duplicating ranking logic. + */ +internal object DiceMcpSupport { + + /** + * Base query for MCP recall/list: scoped to one [contextId], filtered to [PropositionStatus.ACTIVE] + * propositions at or above [minConfidence] effective confidence. + * + * STALE / SUPERSEDED / CONTRADICTED propositions are excluded by default — the same guard + * [com.embabel.dice.agent.Memory] applies before results reach an LLM. + */ + fun baseQuery(contextId: String, minConfidence: Double): PropositionQuery = + PropositionQuery.forContextId(ContextId(requireContextId(contextId))) + .withMinEffectiveConfidence(minConfidence) + .withStatuses(setOf(PropositionStatus.ACTIVE)) + + fun requireContextId(contextId: String): String { + val scoped = contextId.trim() + require(scoped.isNotBlank()) { "context_id must not be blank" } + return scoped + } + + fun recall( + repository: PropositionRepository, + contextId: String, + query: String?, + limit: Int, + minConfidence: Double, + ): String { + val scoped = requireContextId(contextId) + val base = baseQuery(scoped, minConfidence) + val retriever = MemoryRetriever(repository, provenanceResolver = null, topic = scoped, eagerIds = emptySet()) + val result = if (query.isNullOrBlank()) { + retriever.listAll(base, limit) + } else { + retriever.search(query.trim(), base, limit) + } + return (result as? Tool.Result.Text)?.content ?: result.toString() + } + + fun formatProposition(proposition: Proposition): String = + buildString { + append("id=${proposition.id}") + append(" | confidence=${"%.2f".format(proposition.effectiveConfidence())}") + append(" | ${proposition.text}") + if (proposition.mentions.isNotEmpty()) { + val entities = proposition.mentions.joinToString("; ") { mention -> + "${mention.span} (${mention.type})" + } + append(" | entities: $entities") + } + } +} diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt new file mode 100644 index 00000000..413423c6 --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt @@ -0,0 +1,174 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.mcp + +import com.embabel.agent.api.annotation.LlmTool +import com.embabel.agent.api.tool.Tool +import com.embabel.agent.core.ContextId +import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.PropositionRepository + +/** + * Simplified DICE tools for external MCP clients. + * + * In-process [com.embabel.dice.agent.Memory] and [com.embabel.dice.agent.DiscoveryTools] bake + * [ContextId] in at construction so an agent cannot cross a tenant boundary. MCP clients are + * stateless and may serve many sessions, so every tool takes an explicit `context_id` — the same + * isolation [com.embabel.dice.incremental.ChunkHistoryStore] gained in #6 / #33. + * + * Rod's #5: expose tools with simplified parameters. This class is that surface: recall, list, + * store, get. Extraction and discovery stay on the existing in-process `asTools()` path. + * + * Export through embabel-agent's [com.embabel.agent.mcpserver.McpToolExport], or add + * `dice-mcp-autoconfigure` with `embabel.dice.mcp.enabled=true`. + * + * @param repository proposition store (required) + * @param minConfidence minimum effective confidence for recall/list + * @param defaultLimit default result cap for recall/list + */ +class DiceMcpTools( + private val repository: PropositionRepository, + private val minConfidence: Double = DEFAULT_MIN_CONFIDENCE, + private val defaultLimit: Int = DEFAULT_LIMIT, +) { + + init { + require(minConfidence in 0.0..1.0) { "minConfidence must be between 0.0 and 1.0" } + require(defaultLimit > 0) { "defaultLimit must be positive" } + } + + /** + * Hybrid semantic + keyword recall over stored propositions in a context. + */ + @LlmTool( + name = RECALL, + description = "Search stored knowledge (propositions) in a DICE context. " + + "Pass a natural-language query to run hybrid semantic + keyword retrieval. " + + "Omit query to list memories ordered by confidence.", + ) + fun recall( + @LlmTool.Param(description = "Context to search within (session, user, or tenant id).") + contextId: String, + @LlmTool.Param(description = "What to recall, in natural language. Omit to list all memories.") + query: String? = null, + @LlmTool.Param(description = "Maximum results (default 10).") + limit: Int = defaultLimit, + ): String = DiceMcpSupport.recall( + repository = repository, + contextId = contextId, + query = query, + limit = limit.coerceAtLeast(1), + minConfidence = minConfidence, + ) + + /** + * List active propositions for a context, ordered by effective confidence. + */ + @LlmTool( + name = LIST, + description = "List stored propositions for a DICE context, ordered by effective confidence.", + ) + fun listMemories( + @LlmTool.Param(description = "Context to list.") + contextId: String, + @LlmTool.Param(description = "Maximum results (default 10).") + limit: Int = defaultLimit, + ): String { + val scoped = DiceMcpSupport.requireContextId(contextId) + val query = DiceMcpSupport.baseQuery(scoped, minConfidence) + .orderedByEffectiveConfidence() + .withLimit(limit.coerceAtLeast(1)) + val propositions = repository.query(query) + if (propositions.isEmpty()) { + return "No memories in context '$scoped'." + } + return buildString { + appendLine("Found ${propositions.size} memories in context '$scoped':") + propositions.forEachIndexed { index, proposition -> + appendLine("${index + 1}. ${DiceMcpSupport.formatProposition(proposition)}") + } + }.trimEnd() + } + + /** + * Store a proposition directly without running the extraction pipeline. + */ + @LlmTool( + name = STORE, + description = "Store a natural-language proposition in a DICE context without running extraction.", + ) + fun storeMemory( + @LlmTool.Param(description = "Context to store into.") + contextId: String, + @LlmTool.Param(description = "The fact to remember, in natural language.") + text: String, + @LlmTool.Param(description = "Confidence between 0 and 1 (default 0.8).") + confidence: Double = 0.8, + ): String { + val scoped = DiceMcpSupport.requireContextId(contextId) + require(text.isNotBlank()) { "text must not be blank" } + val proposition = Proposition( + contextId = ContextId(scoped), + text = text.trim(), + mentions = emptyList(), + confidence = confidence.coerceIn(0.0, 1.0), + ) + val saved = repository.save(proposition) + return "Stored proposition ${saved.id}: ${saved.text}" + } + + /** + * Fetch a single proposition by id within a context. + */ + @LlmTool( + name = GET, + description = "Get one stored proposition by id within a DICE context.", + ) + fun getProposition( + @LlmTool.Param(description = "Context the proposition belongs to.") + contextId: String, + @LlmTool.Param(description = "Proposition id returned by recall, list, or store.") + propositionId: String, + ): String { + val scoped = DiceMcpSupport.requireContextId(contextId) + require(propositionId.isNotBlank()) { "proposition_id must not be blank" } + val proposition = repository.findById(propositionId) + ?: return "No proposition with id '$propositionId'." + if (proposition.contextIdValue != scoped) { + return "Proposition '$propositionId' is not in context '$scoped'." + } + return DiceMcpSupport.formatProposition(proposition) + } + + companion object { + const val RECALL = "dice_recall" + const val LIST = "dice_list" + const val STORE = "dice_store" + const val GET = "dice_get" + + val TOOL_NAMES: Set = setOf(RECALL, LIST, STORE, GET) + + const val DEFAULT_MIN_CONFIDENCE = 0.5 + const val DEFAULT_LIMIT = 10 + + /** + * Create [Tool] instances for agent runtimes that register tools by hand + * rather than through `dice-mcp-autoconfigure`. + */ + @JvmStatic + fun asTools(tools: DiceMcpTools): List = Tool.fromInstance(tools) + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt new file mode 100644 index 00000000..ed16d1c9 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt @@ -0,0 +1,575 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.mcp + +import com.embabel.agent.core.ContextId +import com.embabel.dice.proposition.EntityMention +import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.PropositionRepository +import com.embabel.dice.proposition.PropositionStatus +import com.embabel.dice.proposition.store.InMemoryPropositionRepository +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows + +class DiceMcpToolsTest { + + private lateinit var repository: PropositionRepository + private lateinit var tools: DiceMcpTools + + @BeforeEach + fun setUp() { + repository = InMemoryPropositionRepository() + tools = DiceMcpTools(repository, minConfidence = 0.0) + } + + @Nested + inner class StoreAndGetTests { + + @Test + fun `store and get round trip`() { + val stored = tools.storeMemory("session-1", "User likes jazz", confidence = 0.9) + assertTrue(stored.startsWith("Stored proposition")) + + val listed = tools.listMemories("session-1", limit = 5) + assertTrue(listed.contains("User likes jazz")) + + val id = stored.substringAfter("Stored proposition ").substringBefore(":") + val fetched = tools.getProposition("session-1", id) + assertTrue(fetched.contains("User likes jazz")) + assertTrue(fetched.contains("confidence=0.90")) + } + + @Test + fun `store trims text and clamps confidence`() { + val stored = tools.storeMemory("session-1", " trimmed fact ", confidence = 1.7) + val id = stored.substringAfter("Stored proposition ").substringBefore(":") + val fetched = tools.getProposition("session-1", id) + assertTrue(fetched.contains("trimmed fact")) + assertFalse(fetched.contains(" trimmed")) + assertTrue(fetched.contains("confidence=1.00")) + } + + @Test + fun `store rejects blank text`() { + assertThrows { + tools.storeMemory("session-1", " ") + } + } + + @Test + fun `store rejects blank context`() { + assertThrows { + tools.storeMemory(" ", "A fact") + } + } + + @Test + fun `get rejects wrong context`() { + val proposition = repository.save( + Proposition( + contextId = ContextId("other"), + text = "Secret fact", + mentions = emptyList(), + confidence = 0.8, + ), + ) + val result = tools.getProposition("session-1", proposition.id) + assertTrue(result.contains("not in context")) + } + + @Test + fun `get reports missing id`() { + val result = tools.getProposition("session-1", "no-such-id") + assertTrue(result.contains("No proposition with id 'no-such-id'")) + } + + @Test + fun `get rejects blank proposition id`() { + assertThrows { + tools.getProposition("session-1", " ") + } + } + + @Test + fun `get rejects blank context`() { + assertThrows { + tools.getProposition(" ", "any-id") + } + } + + @Test + fun `store clamps negative confidence`() { + val stored = tools.storeMemory("session-1", "Clamped low", confidence = -0.4) + val id = stored.substringAfter("Stored proposition ").substringBefore(":") + val fetched = tools.getProposition("session-1", id) + assertTrue(fetched.contains("confidence=0.00")) + } + + @Test + fun `get of a stale proposition in the same context is allowed for inspection`() { + val proposition = repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Old fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.STALE, + ), + ) + val fetched = tools.getProposition("session-1", proposition.id) + assertTrue(fetched.contains("Old fact")) + } + + @Test + fun `whitespace around context_id is trimmed`() { + val stored = tools.storeMemory(" session-1 ", "Padded context fact") + val id = stored.substringAfter("Stored proposition ").substringBefore(":") + val fetched = tools.getProposition(" session-1", id) + assertTrue(fetched.contains("Padded context fact")) + assertTrue(tools.listMemories("session-1 ", limit = 5).contains("Padded context fact")) + } + } + + @Nested + inner class RecallTests { + + @Test + fun `recall finds keyword match`() { + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Alice works at Acme Corp", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.ACTIVE, + ), + ) + + val result = tools.recall("session-1", query = "Acme", limit = 5) + assertTrue(result.contains("Acme")) + } + + @Test + fun `recall without query lists memories`() { + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "First fact", + mentions = emptyList(), + confidence = 0.8, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Second fact", + mentions = emptyList(), + confidence = 0.7, + ), + ) + + val result = tools.recall("session-1", query = null, limit = 10) + assertTrue(result.contains("First fact")) + assertTrue(result.contains("Second fact")) + } + + @Test + fun `recall blank query lists memories`() { + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Only fact", + mentions = emptyList(), + confidence = 0.8, + ), + ) + val result = tools.recall("session-1", query = " ", limit = 10) + assertTrue(result.contains("Only fact")) + } + + @Test + fun `recall does not leak across contexts`() { + repository.save( + Proposition( + contextId = ContextId("tenant-a"), + text = "Tenant A knows Canva", + mentions = emptyList(), + confidence = 0.9, + ), + ) + repository.save( + Proposition( + contextId = ContextId("tenant-b"), + text = "Tenant B knows Canva", + mentions = emptyList(), + confidence = 0.9, + ), + ) + + val result = tools.recall("tenant-a", query = "Canva", limit = 10) + assertTrue(result.contains("Tenant A knows Canva")) + assertFalse(result.contains("Tenant B knows Canva")) + } + + @Test + fun `recall rejects blank context`() { + assertThrows { + tools.recall(" ", query = "anything", limit = 5) + } + } + + @Test + fun `recall excludes stale superseded and contradicted`() { + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Active Canva fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.ACTIVE, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Stale Canva fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.STALE, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Superseded Canva fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.SUPERSEDED, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Contradicted Canva fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.CONTRADICTED, + ), + ) + + val result = tools.recall("session-1", query = "Canva", limit = 10) + assertTrue(result.contains("Active Canva fact")) + assertFalse(result.contains("Stale Canva fact")) + assertFalse(result.contains("Superseded Canva fact")) + assertFalse(result.contains("Contradicted Canva fact")) + } + + @Test + fun `recall respects minConfidence`() { + val filtered = DiceMcpTools(repository, minConfidence = 0.7) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Weak Canva guess", + mentions = emptyList(), + confidence = 0.4, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Strong Canva fact", + mentions = emptyList(), + confidence = 0.9, + ), + ) + val result = filtered.recall("session-1", query = "Canva", limit = 10) + assertTrue(result.contains("Strong Canva fact")) + assertFalse(result.contains("Weak Canva guess")) + } + + @Test + fun `recall unmatched query does not leak other-context hits`() { + repository.save( + Proposition( + contextId = ContextId("tenant-b"), + text = "Only tenant B knows Zephyr", + mentions = emptyList(), + confidence = 0.9, + ), + ) + val result = tools.recall("tenant-a", query = "Zephyr", limit = 10) + assertFalse(result.contains("Only tenant B knows Zephyr")) + assertTrue(result.contains("No memories matched") || result.contains("No memories")) + } + } + + @Nested + inner class ListAndFilterTests { + + @Test + fun `list empty context`() { + val result = tools.listMemories("empty-session", limit = 10) + assertEquals("No memories in context 'empty-session'.", result) + } + + @Test + fun `list orders by effective confidence`() { + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Low", + mentions = emptyList(), + confidence = 0.2, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "High", + mentions = emptyList(), + confidence = 0.9, + ), + ) + val listed = tools.listMemories("session-1", limit = 10) + assertTrue(listed.indexOf("High") < listed.indexOf("Low")) + } + + @Test + fun `list respects limit`() { + repeat(5) { i -> + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Fact $i", + mentions = emptyList(), + confidence = 0.5, + ), + ) + } + val listed = tools.listMemories("session-1", limit = 2) + val numbered = listed.lines().count { it.matches(Regex("^\\d+\\..*")) } + assertEquals(2, numbered) + } + + @Test + fun `zero limit is coerced to at least one`() { + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Kept", + mentions = emptyList(), + confidence = 0.9, + ), + ) + val listed = tools.listMemories("session-1", limit = 0) + assertTrue(listed.contains("Kept")) + } + + @Test + fun `default minConfidence excludes weak propositions`() { + val filtered = DiceMcpTools(repository) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Weak guess", + mentions = emptyList(), + confidence = 0.2, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Strong fact", + mentions = emptyList(), + confidence = 0.9, + ), + ) + val listed = filtered.listMemories("session-1", limit = 10) + assertTrue(listed.contains("Strong fact")) + assertFalse(listed.contains("Weak guess")) + } + + @Test + fun `list rejects blank context`() { + assertThrows { + tools.listMemories(" ", limit = 10) + } + } + + @Test + fun `non-active statuses are excluded from list`() { + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Active fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.ACTIVE, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Stale fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.STALE, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Superseded fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.SUPERSEDED, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Contradicted fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.CONTRADICTED, + ), + ) + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Promoted fact", + mentions = emptyList(), + confidence = 0.9, + status = PropositionStatus.PROMOTED, + ), + ) + val listed = tools.listMemories("session-1", limit = 10) + assertTrue(listed.contains("Active fact")) + assertFalse(listed.contains("Stale fact")) + assertFalse(listed.contains("Superseded fact")) + assertFalse(listed.contains("Contradicted fact")) + assertFalse(listed.contains("Promoted fact")) + } + + @Test + fun `list does not leak across contexts`() { + repository.save( + Proposition( + contextId = ContextId("tenant-a"), + text = "Tenant A secret", + mentions = emptyList(), + confidence = 0.9, + ), + ) + repository.save( + Proposition( + contextId = ContextId("tenant-b"), + text = "Tenant B fact", + mentions = emptyList(), + confidence = 0.9, + ), + ) + + val listA = tools.listMemories("tenant-a", limit = 10) + assertTrue(listA.contains("Tenant A secret")) + assertFalse(listA.contains("Tenant B fact")) + + val listB = tools.listMemories("tenant-b", limit = 10) + assertTrue(listB.contains("Tenant B fact")) + assertFalse(listB.contains("Tenant A secret")) + } + + @Test + fun `format includes entity mentions`() { + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Jim knows Neo4j", + mentions = listOf(EntityMention(span = "Jim", type = "Person")), + confidence = 0.9, + ), + ) + val listed = tools.listMemories("session-1", limit = 10) + assertTrue(listed.contains("Jim (Person)")) + } + } + + @Nested + inner class ToolSurfaceTests { + + @Test + fun `asTools exposes exactly the four named tools`() { + val exported = DiceMcpTools.asTools(tools) + assertEquals(DiceMcpTools.TOOL_NAMES, exported.map { it.definition.name }.toSet()) + assertEquals(4, exported.size) + } + + @Test + fun `constructor rejects invalid minConfidence`() { + assertThrows { + DiceMcpTools(repository, minConfidence = 1.1) + } + assertThrows { + DiceMcpTools(repository, minConfidence = -0.1) + } + } + + @Test + fun `constructor rejects non-positive defaultLimit`() { + assertThrows { + DiceMcpTools(repository, defaultLimit = 0) + } + assertThrows { + DiceMcpTools(repository, defaultLimit = -3) + } + } + + @Test + fun `asTools names are the stable dice_ contract`() { + val names = DiceMcpTools.asTools(tools).map { it.definition.name }.toSet() + assertEquals( + setOf("dice_recall", "dice_list", "dice_store", "dice_get"), + names, + ) + } + } + + @Nested + inner class ContextIsolationTests { + + @Test + fun `store in one context cannot be listed recalled or fetched from another`() { + val stored = tools.storeMemory("tenant-a", "Tenant A only") + val id = stored.substringAfter("Stored proposition ").substringBefore(":") + + val listed = tools.listMemories("tenant-b", limit = 10) + assertFalse(listed.contains("Tenant A only")) + + val recalled = tools.recall("tenant-b", query = "Tenant", limit = 10) + assertFalse(recalled.contains("Tenant A only")) + + val fetched = tools.getProposition("tenant-b", id) + assertTrue(fetched.contains("not in context")) + assertFalse(fetched.contains("Tenant A only")) + } + } +} diff --git a/docs/design/INDEX.md b/docs/design/INDEX.md index c8f62f55..676d893a 100644 --- a/docs/design/INDEX.md +++ b/docs/design/INDEX.md @@ -71,7 +71,8 @@ you need. - [events.md](events.md) — the domain events DICE emits (fact persisted, status changed, batch finished) and how they loosely couple the substrate to observers. - [web-api.md](web-api.md) — the opt-in REST surface over the pipeline, memory, and discovery - layers, gated by an API-key filter. + layers, gated by an API-key filter. MCP export (`DiceMcpTools`, `dice-mcp-autoconfigure`) is the + sibling opt-in for stateless MCP clients; see [architecture.md](architecture.md#expose-agent-tools-rest-and-mcp). - [report.md](report.md) — `dice-report`'s pure projectors that turn queried propositions into human-facing artifacts: structured breakdowns, discovered links, LLM-generated rationale. - [metamodel-versioning.md](metamodel-versioning.md) — stamping a schema with a content hash so it @@ -90,7 +91,7 @@ you need. ## Modules -DICE ships as seven Maven modules; [architecture.md](architecture.md#module-map) has the full +DICE ships as eight Maven modules; [architecture.md](architecture.md#module-map) has the full dependency map. Quick pointer to where each is documented: | Module | Documented in | @@ -98,6 +99,7 @@ dependency map. Quick pointer to where each is documented: | `dice` (core) | most notes above — propositions, pipeline, projections, hygiene, retrieval | | `dice-storage` | [durable-storage.md](durable-storage.md), [graph-projection.md](graph-projection.md), [prolog-projection.md](prolog-projection.md) | | `dice-storage-autoconfigure` | [durable-storage.md](durable-storage.md), [metamodel-wiring.md](metamodel-wiring.md) | +| `dice-mcp-autoconfigure` | [architecture.md](architecture.md#expose-agent-tools-rest-and-mcp) | | `dice-ingestion` | [ingestion.md](ingestion.md) | | `dice-report` | [report.md](report.md) | | `dice-metamodel` | [metamodel-versioning.md](metamodel-versioning.md), [metamodel-diff.md](metamodel-diff.md), [metamodel-drift.md](metamodel-drift.md), [metamodel-wiring.md](metamodel-wiring.md) | diff --git a/docs/design/architecture.md b/docs/design/architecture.md index 299239e2..b98ea115 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -11,9 +11,10 @@ DICE is a multi-module Maven build. Each module's intent, and what it's allowed | Module | Intent | |---|---| -| `dice` | The core: proposition model, pipeline, gates, projection interfaces, query facades, agent tools, REST controllers. In-memory implementations only — no database driver. | +| `dice` | The core: proposition model, pipeline, gates, projection interfaces, query facades, agent tools, REST controllers, MCP tool surface (`DiceMcpTools`). In-memory implementations only — no database driver. | | `dice-storage` | The durable Neo4j backend: `Drivine`-based repository, graph/Prolog/lineage projectors, schema and index bootstrap, and the governance persistence side: `MetamodelVersionStore`, the `DriftReportStore` drift log, and the `ObservedSchemaSource` that asks the live graph what it holds, excluding dice's own bookkeeping labels and edges so governance doesn't observe itself. Depends on `dice` and `dice-metamodel`. | | `dice-storage-autoconfigure` | Spring Boot autoconfiguration that wires `dice-storage`'s beans (repository, projectors, trust scorer) into a host application, plus the schema-governance loop: version store, drift log, observed-schema source, differ, quarantine policy and drift runner. That loop is wired only when the application supplies a `DeclaredSchemaSource` bean, and one property, `off` / `observe`, defaulting to `observe`, picks whether a drift runner bean is registered. No property quarantines anything; a host quarantines by calling `DriftSweepCapable.sweep`. See [metamodel-wiring.md](metamodel-wiring.md). Depends on `dice-storage`. | +| `dice-mcp-autoconfigure` | Spring Boot autoconfiguration that exports `DiceMcpTools` over embabel-agent MCP when `embabel.dice.mcp.enabled=true`. Depends on `dice`. | | `dice-ingestion` | Content-hash dedup ledger and source adapters that sit in front of `PropositionPipeline`, so the same artifact is never extracted twice concurrently. Depends on `dice`. | | `dice-report` | Rationale and structured report generation over propositions and their lineage. Depends on `dice`. | | `dice-metamodel` | Schema governance: content-hash stamps over the governed part of a `DataDictionary`, the declared-schema contract, the version and drift-report store contracts, diffing, drift checking, and non-destructive quarantine. A leaf over `embabel-agent-api`, with no dependency on `dice`; `dice-storage` implements its store contracts. | @@ -24,6 +25,7 @@ flowchart TB dice["dice
(core)"] storage["dice-storage
(Neo4j backend)"] autoconf["dice-storage-autoconfigure
(Spring Boot wiring)"] + mcp["dice-mcp-autoconfigure
(MCP export)"] ingestion["dice-ingestion
(dedup ledger)"] report["dice-report
(rationale/reports)"] metamodel["dice-metamodel
(schema governance)"] @@ -33,6 +35,7 @@ flowchart TB storage --> metamodel dice --> metamodel autoconf --> storage + mcp --> dice ingestion --> dice report --> dice itest --> dice @@ -45,7 +48,8 @@ graph driver. `dice` depends on `dice-metamodel` to read a `MetamodelDiff`. One `dice-storage`, which implements the `MetamodelVersionStore` and `DriftReportStore` contracts against Neo4j. Quarantine marks a proposition `PropositionStatus.QUARANTINED` and is declared in `dice`, so the machinery stays in core logic without back-depending to the schema model. -`dice-storage-autoconfigure` is the only module that knows about Spring Boot autoconfiguration; +Spring Boot autoconfiguration lives in dedicated `*-autoconfigure` modules +(`dice-storage-autoconfigure`, `dice-mcp-autoconfigure`); plain `dice-storage` stays framework-neutral so it can be wired by hand outside Spring Boot. ### Subsystem design docs @@ -224,7 +228,7 @@ sequenceDiagram See [retrieval-and-discovery](retrieval-and-discovery.md). -### Expose: agent tools and REST +### Expose: agent tools, REST, and MCP ```mermaid flowchart LR @@ -238,17 +242,26 @@ flowchart LR PC["PropositionPipelineController"] MC["MemoryController"] end + subgraph mcp ["MCP (contextId per call)"] + MCP["DiceMcpTools
recall, list, store, get"] + end GQT --> GQ[GraphQuery] DT --> RR[RetrievalRouter] DC --> RR MT --> PS[PropositionStore] + MCP --> PS PC --> PIPE[PropositionPipeline] MC --> PS ``` Agent tools and REST share the same underlying routers and stores. The contextId is structurally isolated — agent tools bake it in at construction, REST takes it from the URL path only. Neither -surface accepts a context override in the request body. +of those surfaces accepts a context override in the request body. + +External MCP clients are stateless and may serve many sessions, so they cannot bake a context in +at construction. `DiceMcpTools` takes `context_id` on every call and refuses `get` when the id +belongs to another context. Export is opt-in (`dice-mcp-autoconfigure`, +`embabel.dice.mcp.enabled=true`). Discovery and graph tools stay on the in-process `asTools()` path. ## Events @@ -297,7 +310,8 @@ and `CollectorRecord` MERGE on their natural keys so replayed writes are idempot | Retrieval router | `dice/query/discovery/RetrievalRouter.kt` | | Graph query facade | `dice/query/graph/GraphQuery.kt` | | Agent tools | `dice/agent/DiscoveryTools.kt`, `GraphQueryTools.kt` | +| MCP tools | `dice/mcp/DiceMcpTools.kt` | | REST surface | `dice/web/rest/DiscoveryController.kt` | | Events | `dice/common/` (event types), `EventEmittingPropositionRepository` | -| Spring Boot wiring | `dice-storage-autoconfigure/DiceStorageAutoConfiguration.kt` | +| Spring Boot wiring | `dice-storage-autoconfigure/DiceStorageAutoConfiguration.kt`, `dice-mcp-autoconfigure/DiceMcpAutoConfiguration.kt` | | Schema-governance wiring | `dice-storage-autoconfigure/MetamodelAutoConfiguration.kt` | diff --git a/docs/design/retrieval-and-discovery.md b/docs/design/retrieval-and-discovery.md index 90f1e896..676d12e7 100644 --- a/docs/design/retrieval-and-discovery.md +++ b/docs/design/retrieval-and-discovery.md @@ -158,6 +158,10 @@ the request body has no context field, so a caller *cannot* ask one context's en context's data. Cross-context reads aren't forbidden by a check; they're structurally impossible, and an LLM given the agent tools can't wander across context boundaries either. +The exception is external MCP: those clients are stateless, so `DiceMcpTools` takes `context_id` on +every call and checks it on `get`. That is a parameter, not a body-level override of a baked-in +context, and recall/list still start from `PropositionQuery.forContextId`. + Two concerns drive this: a stable external contract (internal types can evolve without breaking the wire, and a leak-check guards against a domain type sneaking into a DTO by accident) and that structural isolation. diff --git a/pom.xml b/pom.xml index 663a238f..8842d9ee 100644 --- a/pom.xml +++ b/pom.xml @@ -31,6 +31,7 @@ dice-metamodel dice-integration-tests dice-user-guide + dice-mcp-autoconfigure @@ -97,6 +98,11 @@ dice-metamodel ${project.version} + + com.embabel.dice + dice-mcp-autoconfigure + ${project.version} + From b3895ba6db0df37099d2189826300ea19fb7d347 Mon Sep 17 00:00:00 2001 From: LordKay-sudo Date: Sat, 5 Sep 2026 07:07:05 +0200 Subject: [PATCH 2/8] fix(mcp): trim proposition id on get MCP clients often pad copied ids; lookup should match the stored id. Signed-off-by: LordKay-sudo --- .../src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt | 9 +++++---- .../test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt | 8 ++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt index 413423c6..ec42f9d6 100644 --- a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt @@ -144,11 +144,12 @@ class DiceMcpTools( propositionId: String, ): String { val scoped = DiceMcpSupport.requireContextId(contextId) - require(propositionId.isNotBlank()) { "proposition_id must not be blank" } - val proposition = repository.findById(propositionId) - ?: return "No proposition with id '$propositionId'." + val id = propositionId.trim() + require(id.isNotBlank()) { "proposition_id must not be blank" } + val proposition = repository.findById(id) + ?: return "No proposition with id '$id'." if (proposition.contextIdValue != scoped) { - return "Proposition '$propositionId' is not in context '$scoped'." + return "Proposition '$id' is not in context '$scoped'." } return DiceMcpSupport.formatProposition(proposition) } diff --git a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt index ed16d1c9..1898c369 100644 --- a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt @@ -146,6 +146,14 @@ class DiceMcpToolsTest { assertTrue(fetched.contains("Padded context fact")) assertTrue(tools.listMemories("session-1 ", limit = 5).contains("Padded context fact")) } + + @Test + fun `get trims proposition id`() { + val stored = tools.storeMemory("session-1", "Padded id fact") + val id = stored.substringAfter("Stored proposition ").substringBefore(":") + val fetched = tools.getProposition("session-1", " $id ") + assertTrue(fetched.contains("Padded id fact")) + } } @Nested From 663c552c10aedb06181311508a64634421609859 Mon Sep 17 00:00:00 2001 From: LordKay-sudo Date: Sat, 5 Sep 2026 08:12:45 +0200 Subject: [PATCH 3/8] test(mcp): prove storage autoconfig ordering for MCP export afterName is a string so production stays free of a storage compile dep. The combined runner lists MCP first so a broken name cannot hide behind declaration order. Signed-off-by: LordKay-sudo --- dice-mcp-autoconfigure/AGENTS.md | 3 + dice-mcp-autoconfigure/pom.xml | 16 +++ .../DiceMcpAutoConfigurationTest.kt | 100 ++++++++++++++++++ pom.xml | 5 + 4 files changed, 124 insertions(+) diff --git a/dice-mcp-autoconfigure/AGENTS.md b/dice-mcp-autoconfigure/AGENTS.md index caa5afcd..0cf017a1 100644 --- a/dice-mcp-autoconfigure/AGENTS.md +++ b/dice-mcp-autoconfigure/AGENTS.md @@ -36,5 +36,8 @@ Every collaborator is `@ConditionalOnMissingBean`, so an app's own `DiceMcpTools - MCP export is **opt-in**. Unlike the collector (`enabled` default true), this stays dark until `embabel.dice.mcp.enabled=true`. - Without a `PropositionRepository` bean the auto-config class may load but it exports nothing. +- `afterName` is a string, not `after = [DiceStorageAutoConfiguration::class]`, so this module + has no compile dependency on `dice-storage-autoconfigure`. The combined wiring test (test-scope + only) is what proves the name is right and the store bean is visible. - Discovery and graph tools are not on this path. They bake context in at construction; use `DiscoveryTools.asTools(...)` / `GraphQueryTools.asTools(...)` for in-process agents. diff --git a/dice-mcp-autoconfigure/pom.xml b/dice-mcp-autoconfigure/pom.xml index 39aef2c6..61c57ed2 100644 --- a/dice-mcp-autoconfigure/pom.xml +++ b/dice-mcp-autoconfigure/pom.xml @@ -65,6 +65,22 @@ assertj-core test + + + com.embabel.dice + dice-storage-autoconfigure + test + + + org.mockito.kotlin + mockito-kotlin + 5.4.0 + test + diff --git a/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt b/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt index 2b2c7e9b..e87b7d5b 100644 --- a/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt +++ b/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt @@ -15,14 +15,20 @@ */ package com.embabel.dice.mcp.autoconfigure +import com.embabel.agent.api.common.Ai import com.embabel.agent.api.tool.ToolObject import com.embabel.agent.mcpserver.McpToolExport +import com.embabel.common.ai.model.EmbeddingService import com.embabel.dice.mcp.DiceMcpTools import com.embabel.dice.proposition.PropositionRepository import com.embabel.dice.proposition.store.InMemoryPropositionRepository +import com.embabel.dice.storage.autoconfigure.DiceStorageAutoConfiguration import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever import org.springframework.beans.factory.getBean +import org.springframework.boot.autoconfigure.AutoConfiguration import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.test.context.FilteredClassLoader import org.springframework.boot.test.context.runner.ApplicationContextRunner @@ -112,6 +118,8 @@ class DiceMcpAutoConfigurationTest { ) .run { ctx -> assertThat(ctx).hasFailed() + assertThat(ctx.startupFailure) + .hasStackTraceContaining("embabel.dice.mcp.min-confidence") } } @@ -125,6 +133,83 @@ class DiceMcpAutoConfigurationTest { ) .run { ctx -> assertThat(ctx).hasFailed() + assertThat(ctx.startupFailure) + .hasStackTraceContaining("embabel.dice.mcp.default-limit") + } + } + + @Test + fun `afterName names DiceStorageAutoConfiguration so there is no compile dep`() { + val afterName = DiceMcpAutoConfiguration::class.java + .getAnnotation(AutoConfiguration::class.java) + .afterName + assertThat(afterName).containsExactly( + "com.embabel.dice.storage.autoconfigure.DiceStorageAutoConfiguration", + ) + } + + /** + * Runs [DiceStorageAutoConfiguration] and [DiceMcpAutoConfiguration] together, not the + * isolated stub the other tests use, to prove `afterName` on [DiceMcpAutoConfiguration] + * actually resolves `@ConditionalOnBean(PropositionRepository::class)`. MCP is listed + * first so a broken `afterName` cannot hide behind declaration order — without the wait, + * the store bean is not there yet and export silently drops out (the #102 failure). + */ + @Test + fun `combining storage and MCP autoconfig wires tools from the store bean`() { + ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of( + DiceMcpAutoConfiguration::class.java, + DiceStorageAutoConfiguration::class.java, + ) + ) + .withUserConfiguration(StubAiConfig::class.java) + .withPropertyValues("embabel.dice.mcp.enabled=true") + .run { ctx -> + assertThat(ctx).hasSingleBean(PropositionRepository::class.java) + assertThat(ctx).hasSingleBean(DiceMcpTools::class.java) + assertThat(ctx).hasBean("diceMcpToolExport") + + val export = ctx.getBean("diceMcpToolExport") + val names = export.toolCallbacks.map { it.toolDefinition.name() }.toSet() + assertThat(names).isEqualTo(DiceMcpTools.TOOL_NAMES) + } + } + + @Test + fun `combining storage and MCP autoconfig stays dark when the switch is off`() { + ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of( + DiceMcpAutoConfiguration::class.java, + DiceStorageAutoConfiguration::class.java, + ) + ) + .withUserConfiguration(StubAiConfig::class.java) + .run { ctx -> + assertThat(ctx).hasSingleBean(PropositionRepository::class.java) + assertThat(ctx).doesNotHaveBean(DiceMcpAutoConfiguration::class.java) + assertThat(ctx).doesNotHaveBean(DiceMcpTools::class.java) + assertThat(ctx).doesNotHaveBean(McpToolExport::class.java) + } + } + + @Test + fun `combining storage and MCP autoconfig exports nothing when storage has no Ai`() { + ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of( + DiceMcpAutoConfiguration::class.java, + DiceStorageAutoConfiguration::class.java, + ) + ) + .withPropertyValues("embabel.dice.mcp.enabled=true") + .run { ctx -> + assertThat(ctx).doesNotHaveBean(PropositionRepository::class.java) + assertThat(ctx).hasSingleBean(DiceMcpAutoConfiguration::class.java) + assertThat(ctx).doesNotHaveBean(DiceMcpTools::class.java) + assertThat(ctx).doesNotHaveBean("diceMcpToolExport") } } @@ -229,4 +314,19 @@ class DiceMcpAutoConfigurationTest { ) } } + + /** + * Satisfies [DiceStorageAutoConfiguration]'s `@ConditionalOnBean(Ai::class)` gate on + * `inMemoryPropositionRepository`, so the real in-memory store bean is registered and + * the cross-auto-configuration ordering path is exercised. + */ + @Configuration(proxyBeanMethods = false) + private class StubAiConfig { + @Bean + fun ai(): Ai { + val ai = mock() + whenever(ai.withDefaultEmbeddingService()).thenReturn(mock()) + return ai + } + } } diff --git a/pom.xml b/pom.xml index 8842d9ee..ca818351 100644 --- a/pom.xml +++ b/pom.xml @@ -98,6 +98,11 @@ dice-metamodel ${project.version} + + com.embabel.dice + dice-storage-autoconfigure + ${project.version} + com.embabel.dice dice-mcp-autoconfigure From 922d6eb3e9249dce44c51d407387445c6513f60c Mon Sep 17 00:00:00 2001 From: LordKay-sudo Date: Sat, 5 Sep 2026 08:31:27 +0200 Subject: [PATCH 4/8] fix(mcp): return ids from recall and bound the result limit dice_get advertises an id "returned by recall, list, or store", but recall rendered through MemoryRetriever, which never emits one, so the documented recall-then-get flow could not actually be performed. Split ranking from rendering in MemoryRetriever and let the MCP surface format hits with the same helper dice_list uses; all four tools now share one output shape. limit was only coerced upward. An MCP caller is external and its limit is whatever the client model wrote, so clamp it to 100, mirroring the MAX_TOP_K RetrievalRouter already applies before it does any work. default-limit is validated against the same ceiling so a larger value fails startup instead of being silently truncated on every call. Signed-off-by: LordKay-sudo --- README.md | 4 + dice-mcp-autoconfigure/AGENTS.md | 5 +- .../mcp/autoconfigure/DiceMcpProperties.kt | 10 ++- .../DiceMcpAutoConfigurationTest.kt | 34 ++++++++ .../com/embabel/dice/agent/MemoryRetriever.kt | 35 ++++++-- .../com/embabel/dice/mcp/DiceMcpSupport.kt | 35 ++++++-- .../com/embabel/dice/mcp/DiceMcpTools.kt | 29 ++++--- .../com/embabel/dice/mcp/DiceMcpToolsTest.kt | 82 +++++++++++++++++++ 8 files changed, 210 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index d4f222bb..a49172fd 100644 --- a/README.md +++ b/README.md @@ -2448,6 +2448,10 @@ embabel: | `dice_store` | Store a proposition directly | | `dice_get` | Fetch one proposition by id, refused if it belongs to another context | +`dice_recall` and `dice_list` share one result format, each line carrying the `id=` that +`dice_get` takes, so a client can search and then drill into a single fact. Their `limit` is +clamped to 100. + Discovery and graph tools stay on `DiscoveryTools.asTools(...)` / `GraphQueryTools.asTools(...)`. ### API Key Security diff --git a/dice-mcp-autoconfigure/AGENTS.md b/dice-mcp-autoconfigure/AGENTS.md index 0cf017a1..4994c208 100644 --- a/dice-mcp-autoconfigure/AGENTS.md +++ b/dice-mcp-autoconfigure/AGENTS.md @@ -19,7 +19,7 @@ from `dice`. The isolation rule (`context_id` on every tool) lives on `DiceMcpTo |---|---|---| | `embabel.dice.mcp.enabled` | `false` | Master switch. Off means no beans. | | `embabel.dice.mcp.min-confidence` | `0.5` | Minimum effective confidence for recall/list | -| `embabel.dice.mcp.default-limit` | `10` | Default result cap for recall/list | +| `embabel.dice.mcp.default-limit` | `10` | Default result cap for recall/list. Must be `1..100` | Every collaborator is `@ConditionalOnMissingBean`, so an app's own `DiceMcpTools` or `diceMcpToolExport` bean wins. @@ -36,6 +36,9 @@ Every collaborator is `@ConditionalOnMissingBean`, so an app's own `DiceMcpTools - MCP export is **opt-in**. Unlike the collector (`enabled` default true), this stays dark until `embabel.dice.mcp.enabled=true`. - Without a `PropositionRepository` bean the auto-config class may load but it exports nothing. +- `default-limit` is bounded by `DiceMcpTools.MAX_LIMIT` (100), the ceiling the tools clamp every + caller-supplied `limit` to. A larger default would bind and then be silently truncated on every + call, so it fails startup instead. - `afterName` is a string, not `after = [DiceStorageAutoConfiguration::class]`, so this module has no compile dependency on `dice-storage-autoconfigure`. The combined wiring test (test-scope only) is what proves the name is right and the store bean is visible. diff --git a/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt index 4d72f54c..1f36606b 100644 --- a/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt +++ b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt @@ -15,6 +15,7 @@ */ package com.embabel.dice.mcp.autoconfigure +import com.embabel.dice.mcp.DiceMcpTools import org.springframework.boot.context.properties.ConfigurationProperties /** @@ -29,11 +30,16 @@ data class DiceMcpProperties( val enabled: Boolean = false, /** Minimum effective confidence for recall/list tools (0.0–1.0). */ val minConfidence: Double = 0.5, - /** Default result limit for recall/list tools. */ + /** + * Default result limit for recall/list tools. Bounded by [DiceMcpTools.MAX_LIMIT]: a larger + * value would be silently clamped at call time, so it fails startup instead. + */ val defaultLimit: Int = 10, ) { init { require(minConfidence in 0.0..1.0) { "embabel.dice.mcp.min-confidence must be between 0.0 and 1.0" } - require(defaultLimit > 0) { "embabel.dice.mcp.default-limit must be positive" } + require(defaultLimit in 1..DiceMcpTools.MAX_LIMIT) { + "embabel.dice.mcp.default-limit must be between 1 and ${DiceMcpTools.MAX_LIMIT}" + } } } diff --git a/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt b/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt index e87b7d5b..40433a89 100644 --- a/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt +++ b/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt @@ -138,6 +138,40 @@ class DiceMcpAutoConfigurationTest { } } + /** + * The tools clamp `limit` to [DiceMcpTools.MAX_LIMIT], so a configured default above it + * would bind cleanly and then be silently truncated on every call. Fail at startup instead. + */ + @Test + fun `defaultLimit above the clamp fails context startup`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.default-limit=${DiceMcpTools.MAX_LIMIT + 1}", + ) + .run { ctx -> + assertThat(ctx).hasFailed() + assertThat(ctx.startupFailure) + .hasStackTraceContaining("embabel.dice.mcp.default-limit") + } + } + + @Test + fun `defaultLimit at the clamp binds`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.default-limit=${DiceMcpTools.MAX_LIMIT}", + ) + .run { ctx -> + assertThat(ctx).hasNotFailed() + assertThat(ctx.getBean().defaultLimit) + .isEqualTo(DiceMcpTools.MAX_LIMIT) + } + } + @Test fun `afterName names DiceStorageAutoConfiguration so there is no compile dep`() { val afterName = DiceMcpAutoConfiguration::class.java diff --git a/dice/src/main/kotlin/com/embabel/dice/agent/MemoryRetriever.kt b/dice/src/main/kotlin/com/embabel/dice/agent/MemoryRetriever.kt index 8e11bc7f..42577a1c 100644 --- a/dice/src/main/kotlin/com/embabel/dice/agent/MemoryRetriever.kt +++ b/dice/src/main/kotlin/com/embabel/dice/agent/MemoryRetriever.kt @@ -55,6 +55,25 @@ internal class MemoryRetriever( /** Hybrid search for a freeform [query] within the [base] scope. */ fun search(query: String, base: PropositionQuery, limit: Int): Tool.Result { + val hits = rank(query, base, limit) + return if (hits.isEmpty()) noMatch(query, base) else renderHits(query, hits) + } + + /** + * Ranked propositions for a freeform [query], without rendering. + * + * The ranking is the part that must not drift between surfaces; the rendering is not. + * [search] renders these as [Memory]'s tool result, while the MCP surface + * ([com.embabel.dice.mcp.DiceMcpTools]) formats them its own way so its four tools share + * one output shape. Probe tags are dropped here — callers that need them use [search]. + */ + fun rankedPropositions(query: String, base: PropositionQuery, limit: Int): List = + rank(query, base, limit).map { it.prop } + + /** + * Run the three retrieval tiers and fuse them into one ranked list. + */ + private fun rank(query: String, base: PropositionQuery, limit: Int): List { val ordered = LinkedHashMap() // Tier 1+2 — direct probes. Vector (similarity order) and @@ -72,19 +91,25 @@ internal class MemoryRetriever( // Reciprocal Rank Fusion across the tiers: consensus hits (found // by more than one probe) outrank a single probe's high-but-lone // hit. Stable sort, so equal scores keep tier insertion order. - val hits = ordered.values + return ordered.values .filter { it.prop.id !in eagerIds } .sortedByDescending { it.rrf } .take(limit) - return if (hits.isEmpty()) noMatch(query, base) else renderHits(query, hits) } - /** List all in-scope memories by confidence (no query supplied). */ - fun listAll(base: PropositionQuery, limit: Int): Tool.Result { - val results = repository.query(base.orderedByEffectiveConfidence().withLimit(limit)) + /** + * In-scope memories by confidence, without rendering. The no-query counterpart to + * [rankedPropositions]. + */ + fun listRanked(base: PropositionQuery, limit: Int): List = + repository.query(base.orderedByEffectiveConfidence().withLimit(limit)) .filter { it.id !in eagerIds } .take(limit) + /** List all in-scope memories by confidence (no query supplied). */ + fun listAll(base: PropositionQuery, limit: Int): Tool.Result { + val results = listRanked(base, limit) + if (results.isEmpty()) { return Tool.Result.text( if (eagerIds.isNotEmpty()) "No additional memories beyond those already provided." diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt index b455ec4f..f2a7f51e 100644 --- a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt @@ -15,7 +15,6 @@ */ package com.embabel.dice.mcp -import com.embabel.agent.api.tool.Tool import com.embabel.agent.core.ContextId import com.embabel.dice.agent.MemoryRetriever import com.embabel.dice.proposition.Proposition @@ -49,6 +48,15 @@ internal object DiceMcpSupport { return scoped } + /** + * Hybrid recall, or confidence-ordered listing when [query] is absent. + * + * Ranking is delegated to [MemoryRetriever] so MCP and in-process + * [com.embabel.dice.agent.Memory] cannot drift, but the results are rendered here with + * [formatProposition] — the same shape `dice_list` and `dice_get` emit. That keeps one + * output format across the four tools and, critically, puts the proposition id on every + * line so a client can follow a recall hit into `dice_get`. + */ fun recall( repository: PropositionRepository, contextId: String, @@ -59,14 +67,31 @@ internal object DiceMcpSupport { val scoped = requireContextId(contextId) val base = baseQuery(scoped, minConfidence) val retriever = MemoryRetriever(repository, provenanceResolver = null, topic = scoped, eagerIds = emptySet()) - val result = if (query.isNullOrBlank()) { - retriever.listAll(base, limit) + val trimmed = query?.trim()?.takeIf { it.isNotBlank() } + val hits = if (trimmed == null) { + retriever.listRanked(base, limit) } else { - retriever.search(query.trim(), base, limit) + retriever.rankedPropositions(trimmed, base, limit) + } + if (hits.isEmpty()) { + return if (trimmed == null) "No memories in context '$scoped'." + else "No memories matched '$trimmed' in context '$scoped'." } - return (result as? Tool.Result.Text)?.content ?: result.toString() + val header = + if (trimmed == null) "Found ${hits.size} memories in context '$scoped':" + else "Found ${hits.size} memories matching '$trimmed' in context '$scoped':" + return render(header, hits) } + /** Numbered rendering shared by `dice_recall` and `dice_list`. */ + fun render(header: String, propositions: List): String = + buildString { + appendLine(header) + propositions.forEachIndexed { index, proposition -> + appendLine("${index + 1}. ${formatProposition(proposition)}") + } + }.trimEnd() + fun formatProposition(proposition: Proposition): String = buildString { append("id=${proposition.id}") diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt index ec42f9d6..5ea0c96e 100644 --- a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt @@ -47,7 +47,7 @@ class DiceMcpTools( init { require(minConfidence in 0.0..1.0) { "minConfidence must be between 0.0 and 1.0" } - require(defaultLimit > 0) { "defaultLimit must be positive" } + require(defaultLimit in 1..MAX_LIMIT) { "defaultLimit must be between 1 and $MAX_LIMIT" } } /** @@ -64,13 +64,13 @@ class DiceMcpTools( contextId: String, @LlmTool.Param(description = "What to recall, in natural language. Omit to list all memories.") query: String? = null, - @LlmTool.Param(description = "Maximum results (default 10).") + @LlmTool.Param(description = "Maximum results (default 10, capped at 100).") limit: Int = defaultLimit, ): String = DiceMcpSupport.recall( repository = repository, contextId = contextId, query = query, - limit = limit.coerceAtLeast(1), + limit = limit.coerceIn(1, MAX_LIMIT), minConfidence = minConfidence, ) @@ -84,23 +84,21 @@ class DiceMcpTools( fun listMemories( @LlmTool.Param(description = "Context to list.") contextId: String, - @LlmTool.Param(description = "Maximum results (default 10).") + @LlmTool.Param(description = "Maximum results (default 10, capped at 100).") limit: Int = defaultLimit, ): String { val scoped = DiceMcpSupport.requireContextId(contextId) val query = DiceMcpSupport.baseQuery(scoped, minConfidence) .orderedByEffectiveConfidence() - .withLimit(limit.coerceAtLeast(1)) + .withLimit(limit.coerceIn(1, MAX_LIMIT)) val propositions = repository.query(query) if (propositions.isEmpty()) { return "No memories in context '$scoped'." } - return buildString { - appendLine("Found ${propositions.size} memories in context '$scoped':") - propositions.forEachIndexed { index, proposition -> - appendLine("${index + 1}. ${DiceMcpSupport.formatProposition(proposition)}") - } - }.trimEnd() + return DiceMcpSupport.render( + "Found ${propositions.size} memories in context '$scoped':", + propositions, + ) } /** @@ -165,6 +163,15 @@ class DiceMcpTools( const val DEFAULT_MIN_CONFIDENCE = 0.5 const val DEFAULT_LIMIT = 10 + /** + * Hard ceiling on `limit` for recall/list. An MCP caller is external and its limit + * arrives as whatever number the client model wrote, so the bound is enforced here + * rather than trusted. Mirrors the `MAX_TOP_K` that + * [com.embabel.dice.query.discovery.RetrievalRouter] clamps to — that one is private, + * so the value is duplicated rather than referenced. + */ + const val MAX_LIMIT = 100 + /** * Create [Tool] instances for agent runtimes that register tools by hand * rather than through `dice-mcp-autoconfigure`. diff --git a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt index 1898c369..a6762cf2 100644 --- a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt @@ -328,6 +328,79 @@ class DiceMcpToolsTest { assertFalse(result.contains("Only tenant B knows Zephyr")) assertTrue(result.contains("No memories matched") || result.contains("No memories")) } + + /** + * `dice_get` advertises its id as "returned by recall, list, or store", so recall has to + * actually emit one. Recall renders through the same formatter as list for that reason. + */ + @Test + fun `recall hits carry ids that get accepts`() { + val stored = tools.storeMemory("session-1", "Acme uses Canva for design", confidence = 0.9) + val id = stored.substringAfter("Stored proposition ").substringBefore(":") + + val recalled = tools.recall("session-1", query = "Canva", limit = 5) + assertTrue(recalled.contains("id=$id"), "recall output should carry the proposition id") + + val fetched = tools.getProposition("session-1", id) + assertTrue(fetched.contains("Acme uses Canva for design")) + } + + @Test + fun `recall with no query carries ids too`() { + val stored = tools.storeMemory("session-1", "Listable fact", confidence = 0.9) + val id = stored.substringAfter("Stored proposition ").substringBefore(":") + + val recalled = tools.recall("session-1", query = null, limit = 5) + assertTrue(recalled.contains("id=$id")) + } + } + + @Nested + inner class LimitTests { + + private fun seed(count: Int) { + repeat(count) { i -> + repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Canva fact $i", + mentions = emptyList(), + confidence = 0.9, + ), + ) + } + } + + /** + * A limit from an MCP client is whatever number the caller's model wrote, so it is + * clamped rather than trusted — the same bound [com.embabel.dice.query.discovery]'s + * router applies before it does any work. + */ + @Test + fun `list caps an absurd limit at MAX_LIMIT`() { + seed(DiceMcpTools.MAX_LIMIT + 5) + val listed = tools.listMemories("session-1", limit = 10_000) + assertEquals(DiceMcpTools.MAX_LIMIT, listed.lines().size - 1) + } + + @Test + fun `recall caps an absurd limit at MAX_LIMIT`() { + seed(DiceMcpTools.MAX_LIMIT + 5) + + val listing = tools.recall("session-1", query = null, limit = 10_000) + assertEquals(DiceMcpTools.MAX_LIMIT, listing.lines().size - 1) + + val searched = tools.recall("session-1", query = "Canva", limit = 10_000) + assertTrue(searched.lines().size - 1 <= DiceMcpTools.MAX_LIMIT) + } + + @Test + fun `a limit below one is raised to one`() { + seed(3) + assertEquals(1, tools.listMemories("session-1", limit = 0).lines().size - 1) + assertEquals(1, tools.listMemories("session-1", limit = -5).lines().size - 1) + assertEquals(1, tools.recall("session-1", query = null, limit = 0).lines().size - 1) + } } @Nested @@ -551,6 +624,15 @@ class DiceMcpToolsTest { } } + /** A default above the clamp would be silently truncated at call time; reject it instead. */ + @Test + fun `constructor rejects a defaultLimit above MAX_LIMIT`() { + assertThrows { + DiceMcpTools(repository, defaultLimit = DiceMcpTools.MAX_LIMIT + 1) + } + DiceMcpTools(repository, defaultLimit = DiceMcpTools.MAX_LIMIT) + } + @Test fun `asTools names are the stable dice_ contract`() { val names = DiceMcpTools.asTools(tools).map { it.definition.name }.toSet() From 586467172c8bbc4c098cf33949fe4be465320409 Mon Sep 17 00:00:00 2001 From: LordKay-sudo Date: Sat, 5 Sep 2026 15:20:13 +0200 Subject: [PATCH 5/8] fix(mcp): keep store failures and foreign ids off the wire A live store or driver error used to reach the MCP client as exception text. Guard the four tools the way DiscoveryController sanitizes its 500s: caller validation still throws IllegalArgumentException, everything else becomes a cause-free generic failure so Cypher and bolt hosts cannot leak. get now answers a missing id and a foreign-context id the same way MemoryController collapses both into a 404, so the tool cannot confirm that an id it does not own exists. A query miss also restores the in-scope count and retry nudge that MemoryRetriever used to provide, without putting a raw context id into a slot meant for a human-readable topic. Signed-off-by: LordKay-sudo --- .../com/embabel/dice/mcp/DiceMcpSupport.kt | 25 +++- .../com/embabel/dice/mcp/DiceMcpTools.kt | 77 +++++++--- .../com/embabel/dice/mcp/DiceMcpToolsTest.kt | 134 +++++++++++++++++- 3 files changed, 201 insertions(+), 35 deletions(-) diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt index f2a7f51e..17f034ba 100644 --- a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt @@ -31,14 +31,17 @@ import com.embabel.dice.proposition.PropositionStatus internal object DiceMcpSupport { /** - * Base query for MCP recall/list: scoped to one [contextId], filtered to [PropositionStatus.ACTIVE] - * propositions at or above [minConfidence] effective confidence. + * Base query for MCP recall/list: scoped to one [scopedContextId], filtered to + * [PropositionStatus.ACTIVE] propositions at or above [minConfidence] effective confidence. + * + * Takes an id the caller has already put through [requireContextId], so validation happens + * once per tool call rather than once per query built. * * STALE / SUPERSEDED / CONTRADICTED propositions are excluded by default — the same guard * [com.embabel.dice.agent.Memory] applies before results reach an LLM. */ - fun baseQuery(contextId: String, minConfidence: Double): PropositionQuery = - PropositionQuery.forContextId(ContextId(requireContextId(contextId))) + fun baseQuery(scopedContextId: String, minConfidence: Double): PropositionQuery = + PropositionQuery.forContextId(ContextId(scopedContextId)) .withMinEffectiveConfidence(minConfidence) .withStatuses(setOf(PropositionStatus.ACTIVE)) @@ -74,8 +77,18 @@ internal object DiceMcpSupport { retriever.rankedPropositions(trimmed, base, limit) } if (hits.isEmpty()) { - return if (trimmed == null) "No memories in context '$scoped'." - else "No memories matched '$trimmed' in context '$scoped'." + // The no-query wording matches dice_list's for the same situation. A query miss adds + // how much *is* in scope, so the caller can tell "your query was wrong, try again" + // apart from "this context is empty, stop asking". + return if (trimmed == null) { + "No memories in context '$scoped'." + } else { + when (val total = repository.count(base)) { + 0 -> "No memories matched '$trimmed'. Context '$scoped' is empty." + else -> "No memories matched '$trimmed' in context '$scoped'. " + + "$total memories are stored there — try rephrasing or a broader query." + } + } } val header = if (trimmed == null) "Found ${hits.size} memories in context '$scoped':" diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt index 5ea0c96e..452f8b0e 100644 --- a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt @@ -20,14 +20,18 @@ import com.embabel.agent.api.tool.Tool import com.embabel.agent.core.ContextId import com.embabel.dice.proposition.Proposition import com.embabel.dice.proposition.PropositionRepository +import org.slf4j.LoggerFactory /** * Simplified DICE tools for external MCP clients. * * In-process [com.embabel.dice.agent.Memory] and [com.embabel.dice.agent.DiscoveryTools] bake - * [ContextId] in at construction so an agent cannot cross a tenant boundary. MCP clients are - * stateless and may serve many sessions, so every tool takes an explicit `context_id` — the same - * isolation [com.embabel.dice.incremental.ChunkHistoryStore] gained in #6 / #33. + * [ContextId] in at construction, so an agent cannot name another tenant. MCP clients are + * stateless and may serve many sessions, so every tool takes an explicit `context_id`. That is + * a caller-supplied scope, not a credential: it keeps one call from crossing contexts, and + * authorization is the host MCP server's job. Recall and list start from + * [com.embabel.dice.proposition.PropositionQuery.forContextId]; get collapses a missing id and + * a foreign id into one answer so the tool cannot confirm that an id it does not own exists. * * Rod's #5: expose tools with simplified parameters. This class is that surface: recall, list, * store, get. Extraction and discovery stay on the existing in-process `asTools()` path. @@ -45,11 +49,32 @@ class DiceMcpTools( private val defaultLimit: Int = DEFAULT_LIMIT, ) { + private val logger = LoggerFactory.getLogger(DiceMcpTools::class.java) + init { require(minConfidence in 0.0..1.0) { "minConfidence must be between 0.0 and 1.0" } require(defaultLimit in 1..MAX_LIMIT) { "defaultLimit must be between 1 and $MAX_LIMIT" } } + /** + * Run a tool body, keeping store and driver detail away from the caller. + * + * [IllegalArgumentException] is ours — a blank `context_id`, a blank `text` — so it passes + * through and tells the model what to fix. Anything else came from the store: the cause is + * logged here and the exception thrown on has **no cause attached**, so a stack trace + * serialized back by the MCP layer cannot carry Cypher, hostnames, or credentials to an + * external client. Same rule `DiscoveryController` applies to its own 500s. + */ + private fun guarded(tool: String, block: () -> String): String = + try { + block() + } catch (e: IllegalArgumentException) { + throw e + } catch (e: Exception) { + logger.error("MCP tool {} failed", tool, e) + throw IllegalStateException("$tool failed: the knowledge store is unavailable") + } + /** * Hybrid semantic + keyword recall over stored propositions in a context. */ @@ -66,13 +91,15 @@ class DiceMcpTools( query: String? = null, @LlmTool.Param(description = "Maximum results (default 10, capped at 100).") limit: Int = defaultLimit, - ): String = DiceMcpSupport.recall( - repository = repository, - contextId = contextId, - query = query, - limit = limit.coerceIn(1, MAX_LIMIT), - minConfidence = minConfidence, - ) + ): String = guarded(RECALL) { + DiceMcpSupport.recall( + repository = repository, + contextId = contextId, + query = query, + limit = limit.coerceIn(1, MAX_LIMIT), + minConfidence = minConfidence, + ) + } /** * List active propositions for a context, ordered by effective confidence. @@ -86,19 +113,20 @@ class DiceMcpTools( contextId: String, @LlmTool.Param(description = "Maximum results (default 10, capped at 100).") limit: Int = defaultLimit, - ): String { + ): String = guarded(LIST) { val scoped = DiceMcpSupport.requireContextId(contextId) val query = DiceMcpSupport.baseQuery(scoped, minConfidence) .orderedByEffectiveConfidence() .withLimit(limit.coerceIn(1, MAX_LIMIT)) val propositions = repository.query(query) if (propositions.isEmpty()) { - return "No memories in context '$scoped'." + "No memories in context '$scoped'." + } else { + DiceMcpSupport.render( + "Found ${propositions.size} memories in context '$scoped':", + propositions, + ) } - return DiceMcpSupport.render( - "Found ${propositions.size} memories in context '$scoped':", - propositions, - ) } /** @@ -115,7 +143,7 @@ class DiceMcpTools( text: String, @LlmTool.Param(description = "Confidence between 0 and 1 (default 0.8).") confidence: Double = 0.8, - ): String { + ): String = guarded(STORE) { val scoped = DiceMcpSupport.requireContextId(contextId) require(text.isNotBlank()) { "text must not be blank" } val proposition = Proposition( @@ -125,7 +153,7 @@ class DiceMcpTools( confidence = confidence.coerceIn(0.0, 1.0), ) val saved = repository.save(proposition) - return "Stored proposition ${saved.id}: ${saved.text}" + "Stored proposition ${saved.id}: ${saved.text}" } /** @@ -140,16 +168,19 @@ class DiceMcpTools( contextId: String, @LlmTool.Param(description = "Proposition id returned by recall, list, or store.") propositionId: String, - ): String { + ): String = guarded(GET) { val scoped = DiceMcpSupport.requireContextId(contextId) val id = propositionId.trim() require(id.isNotBlank()) { "proposition_id must not be blank" } val proposition = repository.findById(id) - ?: return "No proposition with id '$id'." - if (proposition.contextIdValue != scoped) { - return "Proposition '$id' is not in context '$scoped'." + // One answer for "no such id" and "that id lives in another context". Distinguishing + // them would confirm to a caller that an id it does not own exists somewhere, and + // MemoryController collapses both into a 404 for exactly that reason. + if (proposition == null || proposition.contextIdValue != scoped) { + "No proposition with id '$id' in context '$scoped'." + } else { + DiceMcpSupport.formatProposition(proposition) } - return DiceMcpSupport.formatProposition(proposition) } companion object { diff --git a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt index a6762cf2..e601fdf1 100644 --- a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt @@ -18,6 +18,7 @@ package com.embabel.dice.mcp import com.embabel.agent.core.ContextId import com.embabel.dice.proposition.EntityMention import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.PropositionQuery import com.embabel.dice.proposition.PropositionRepository import com.embabel.dice.proposition.PropositionStatus import com.embabel.dice.proposition.store.InMemoryPropositionRepository @@ -81,8 +82,13 @@ class DiceMcpToolsTest { } } + /** + * A cross-context id and an id that does not exist must be indistinguishable, or the + * tool confirms to a caller that an id it does not own exists somewhere else. + * `MemoryController` collapses both into a 404 for the same reason. + */ @Test - fun `get rejects wrong context`() { + fun `get does not reveal that a foreign id exists`() { val proposition = repository.save( Proposition( contextId = ContextId("other"), @@ -91,14 +97,23 @@ class DiceMcpToolsTest { confidence = 0.8, ), ) - val result = tools.getProposition("session-1", proposition.id) - assertTrue(result.contains("not in context")) + + val foreign = tools.getProposition("session-1", proposition.id) + val absent = tools.getProposition("session-1", proposition.id.reversed()) + + assertFalse(foreign.contains("Secret fact")) + assertFalse(foreign.contains("other")) + assertEquals( + absent.replace(proposition.id.reversed(), proposition.id), + foreign, + "a foreign id and an unknown id must produce the same answer", + ) } @Test fun `get reports missing id`() { val result = tools.getProposition("session-1", "no-such-id") - assertTrue(result.contains("No proposition with id 'no-such-id'")) + assertEquals("No proposition with id 'no-such-id' in context 'session-1'.", result) } @Test @@ -326,7 +341,30 @@ class DiceMcpToolsTest { ) val result = tools.recall("tenant-a", query = "Zephyr", limit = 10) assertFalse(result.contains("Only tenant B knows Zephyr")) - assertTrue(result.contains("No memories matched") || result.contains("No memories")) + assertEquals( + "No memories matched 'Zephyr'. Context 'tenant-a' is empty.", + result, + ) + } + + @Test + fun `recall miss on a populated context includes the count and a retry nudge`() { + tools.storeMemory("session-1", "Alice works at Acme", confidence = 0.9) + tools.storeMemory("session-1", "Bob prefers Kotlin", confidence = 0.8) + val result = tools.recall("session-1", query = "Zephyr", limit = 10) + assertEquals( + "No memories matched 'Zephyr' in context 'session-1'. " + + "2 memories are stored there — try rephrasing or a broader query.", + result, + ) + } + + @Test + fun `recall without a query on an empty context matches list`() { + assertEquals( + tools.listMemories("empty-session", limit = 10), + tools.recall("empty-session", query = null, limit = 10), + ) } /** @@ -658,8 +696,92 @@ class DiceMcpToolsTest { assertFalse(recalled.contains("Tenant A only")) val fetched = tools.getProposition("tenant-b", id) - assertTrue(fetched.contains("not in context")) + assertTrue(fetched.contains("No proposition with id")) assertFalse(fetched.contains("Tenant A only")) } } + + /** + * A live store or driver failure must not reach the MCP client. The leak string is + * deliberately a Cypher fragment plus a bolt host — the same class of detail + * [com.embabel.dice.web.rest.DiscoveryController] sanitizes out of its 500s. + */ + @Nested + inner class StoreFailureTests { + + private val leak = "MATCH (n) RETURN n; bolt://neo4j-prod.internal:7687" + private lateinit var failing: DiceMcpTools + + @BeforeEach + fun failingTools() { + failing = DiceMcpTools(LeakingStore(leak), minConfidence = 0.0) + } + + @Test + fun `store failure is a generic error with no cause and no driver text`() { + assertSanitized { failing.storeMemory("session-1", "A fact") } + } + + @Test + fun `list failure is a generic error with no cause and no driver text`() { + assertSanitized { failing.listMemories("session-1", limit = 10) } + } + + @Test + fun `get failure is a generic error with no cause and no driver text`() { + assertSanitized { failing.getProposition("session-1", "any-id") } + } + + @Test + fun `recall failure is a generic error with no cause and no driver text`() { + assertSanitized { failing.recall("session-1", query = "Canva", limit = 10) } + } + + @Test + fun `recall listing failure is a generic error with no cause and no driver text`() { + assertSanitized { failing.recall("session-1", query = null, limit = 10) } + } + + @Test + fun `caller validation still surfaces as IllegalArgumentException`() { + assertThrows { + failing.storeMemory(" ", "A fact") + } + assertThrows { + failing.recall(" ", query = "anything", limit = 5) + } + } + + private fun assertSanitized(call: () -> String) { + val thrown = assertThrows { call() } + assertTrue(thrown.message!!.endsWith("failed: the knowledge store is unavailable")) + assertEquals(null, thrown.cause) + assertFalse(thrown.message!!.contains(leak)) + assertFalse(thrown.stackTraceToString().contains(leak)) + } + } + + /** + * Implements [PropositionRepository] by delegation so only the MCP entry points + * have to throw. [keywordOverlap] is overridden too: Kotlin `by` would otherwise + * run the default on the in-memory delegate, and MemoryRetriever's keyword probe + * would never see the failure. The leak text rides on the exception message the + * way a real driver error would. + */ + private class LeakingStore( + private val leak: String, + ) : PropositionRepository by InMemoryPropositionRepository() { + override fun save(proposition: Proposition): Proposition = explode() + override fun findById(id: String): Proposition? = explode() + override fun query(query: PropositionQuery): List = explode() + override fun findAll(): List = explode() + override fun keywordOverlap( + base: PropositionQuery, + tokens: List, + limit: Int, + ): List = explode() + + private fun explode(): Nothing = + throw RuntimeException(leak) + } } From 7a70c941207ad1f9459adb80613d542a02debe97 Mon Sep 17 00:00:00 2001 From: LordKay-sudo Date: Sat, 5 Sep 2026 15:20:38 +0200 Subject: [PATCH 6/8] docs(mcp): treat context_id as a scope, not a credential MCP clients can name any tenant; the per-call check only stops one call from crossing contexts. Authorization is the host MCP server's job. Also note that dice_store writes a fact with no mentions or provenance, so it is retrievable by vector and keyword only. Signed-off-by: LordKay-sudo --- AGENTS.md | 2 +- README.md | 13 +++++++++---- dice-mcp-autoconfigure/AGENTS.md | 3 ++- dice/AGENTS.md | 2 +- docs/design/architecture.md | 5 +++-- docs/design/retrieval-and-discovery.md | 3 ++- 6 files changed, 18 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b6221e71..5d6a4526 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -67,7 +67,7 @@ The `dice` module is organized by responsibility: | `com.embabel.dice.provenance` | `ProvenanceEntry`, `SourceLocator` — rich evidence links from propositions back to source material | | `com.embabel.dice.query.oracle` | `Oracle`, `LlmOracle`, `PrologTools` — natural language question answering over propositions | | `com.embabel.dice.web.rest` | Optional REST endpoints for the pipeline and memory; activated by `spring-webmvc` on the classpath | -| `com.embabel.dice.mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `context_id` on every call | +| `com.embabel.dice.mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `context_id` on every call is a scope, not a credential | ## Conventions diff --git a/README.md b/README.md index a49172fd..9dd4686a 100644 --- a/README.md +++ b/README.md @@ -2418,8 +2418,9 @@ Everything is pushed into the database rather than scanned in memory: Expose DICE recall/list/store/get to an MCP client (Claude Desktop, Cursor, etc.) with `dice-mcp-autoconfigure` and embabel-agent's MCP server starter. Off until you set -`embabel.dice.mcp.enabled=true`. Every tool takes a `context_id` — MCP clients are stateless, -unlike in-process `Memory` / `DiscoveryTools` which bake context in at construction. +`embabel.dice.mcp.enabled=true`. Every tool takes a `context_id` — that is a scope, not a +credential. It keeps one call from reading another context; authorization is the host MCP +server's job. In-process `Memory` / `DiscoveryTools` bake context in at construction instead. ```xml @@ -2445,13 +2446,17 @@ embabel: |------|-------------| | `dice_recall` | Hybrid semantic + keyword search in a `context_id` | | `dice_list` | List active propositions for a context | -| `dice_store` | Store a proposition directly | -| `dice_get` | Fetch one proposition by id, refused if it belongs to another context | +| `dice_store` | Store a proposition directly (no mentions or provenance — see below) | +| `dice_get` | Fetch one proposition by id; a miss and a foreign-context id look the same | `dice_recall` and `dice_list` share one result format, each line carrying the `id=` that `dice_get` takes, so a client can search and then drill into a single fact. Their `limit` is clamped to 100. +`dice_store` writes a fact with empty mentions and no provenance, so it is retrievable by +vector and keyword only — not by entity expansion or graph projection. Use the ingestion +pipeline when the fact needs to be wired into the rest of the knowledge flow. + Discovery and graph tools stay on `DiscoveryTools.asTools(...)` / `GraphQueryTools.asTools(...)`. ### API Key Security diff --git a/dice-mcp-autoconfigure/AGENTS.md b/dice-mcp-autoconfigure/AGENTS.md index 4994c208..1916e02e 100644 --- a/dice-mcp-autoconfigure/AGENTS.md +++ b/dice-mcp-autoconfigure/AGENTS.md @@ -2,7 +2,8 @@ Spring Boot wiring that exports [DiceMcpTools](../dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt) over embabel-agent's MCP server. No domain logic — just an `@AutoConfiguration` that assembles beans -from `dice`. The isolation rule (`context_id` on every tool) lives on `DiceMcpTools` itself. +from `dice`. `context_id` on every tool is a caller-supplied scope, not a credential — the +check lives on `DiceMcpTools` itself; authorization is the host MCP server's job. ## What's here diff --git a/dice/AGENTS.md b/dice/AGENTS.md index 1d058063..bdd87d33 100644 --- a/dice/AGENTS.md +++ b/dice/AGENTS.md @@ -76,7 +76,7 @@ These map onto the lifecycle in [proposition-lifecycle](../docs/design/propositi | `query.discovery` | `RetrievalRouter`, `DiscoveryQuery`, `RetrievalMode`, discovery DTOs — mode-routed retrieval entry point | | `temporal` | `TemporalMetadata` — bitemporal valid/observed windows, explicit retraction | | `agent` | `Memory`, `MemoryRetriever` (agent-facing view), `ProvenanceResolver`; **`DiscoveryTools`** — `@LlmTool`-annotated tools wrapping `RetrievalRouter` (query propositions, graph path, why-explain, projection health, collector dry-run) with context baked in at construction so an agent can't cross context boundaries; **`GraphQueryTools`** — `@LlmTool`-annotated tools wrapping the `GraphQuery` facade (entity neighbourhood, path between entities, why-explain) | -| `mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `context_id` on every call so a stateless client cannot cross tenants | +| `mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `context_id` on every call is a caller-supplied scope (not a credential) so one call cannot read another context; authorization is the host MCP server's job | | `web.rest` | `PropositionPipelineController`, `MemoryController`, **`DiscoveryController`** — REST surface for discovery operations (`/api/v1/contexts/{contextId}/discovery`; routes query, path, why, projection health, and collector dry-run; context comes from the URL path only), API key security — optional, activated by `spring-webmvc` | | `operations` | `PropositionAbstractor`, `PropositionContraster` — higher-level proposition management | | `operations.consolidation` | The dream-loop steps as composable passes: `ConsolidationPass`/`ConsolidationPassResult`, `SessionConsolidationPass`, `AbstractionPass`, `ContradictionResolutionPass`, `DecaySweepPass` | diff --git a/docs/design/architecture.md b/docs/design/architecture.md index b98ea115..b0559848 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -259,8 +259,9 @@ isolated — agent tools bake it in at construction, REST takes it from the URL of those surfaces accepts a context override in the request body. External MCP clients are stateless and may serve many sessions, so they cannot bake a context in -at construction. `DiceMcpTools` takes `context_id` on every call and refuses `get` when the id -belongs to another context. Export is opt-in (`dice-mcp-autoconfigure`, +at construction. `DiceMcpTools` takes `context_id` on every call — a caller-supplied scope, not +a credential — and `get` treats a missing id and a foreign-context id the same way. Authorization +is the host MCP server's job. Export is opt-in (`dice-mcp-autoconfigure`, `embabel.dice.mcp.enabled=true`). Discovery and graph tools stay on the in-process `asTools()` path. ## Events diff --git a/docs/design/retrieval-and-discovery.md b/docs/design/retrieval-and-discovery.md index 676d12e7..b36ff8fc 100644 --- a/docs/design/retrieval-and-discovery.md +++ b/docs/design/retrieval-and-discovery.md @@ -160,7 +160,8 @@ an LLM given the agent tools can't wander across context boundaries either. The exception is external MCP: those clients are stateless, so `DiceMcpTools` takes `context_id` on every call and checks it on `get`. That is a parameter, not a body-level override of a baked-in -context, and recall/list still start from `PropositionQuery.forContextId`. +context, and recall/list still start from `PropositionQuery.forContextId`. `context_id` is a scope, +not a credential — any client can name any tenant; authorization is the host MCP server's job. Two concerns drive this: a stable external contract (internal types can evolve without breaking the wire, and a leak-check guards against a domain type sneaking into a DTO by accident) and that From 48033ffd76901090fe21671650ee3e126bf1fe14 Mon Sep 17 00:00:00 2001 From: LordKay-sudo Date: Sat, 5 Sep 2026 15:20:39 +0200 Subject: [PATCH 7/8] chore: manage mockito-kotlin version in the parent POM Three modules pinned 5.4.0 locally. One property, one dependencyManagement entry. Signed-off-by: LordKay-sudo --- dice-mcp-autoconfigure/pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/dice-mcp-autoconfigure/pom.xml b/dice-mcp-autoconfigure/pom.xml index 61c57ed2..0d4ed893 100644 --- a/dice-mcp-autoconfigure/pom.xml +++ b/dice-mcp-autoconfigure/pom.xml @@ -78,7 +78,6 @@ org.mockito.kotlin mockito-kotlin - 5.4.0 test From f0a5169cc4b7be8e66c9dd8ecea3a90d1440f4af Mon Sep 17 00:00:00 2001 From: LordKay-sudo Date: Sun, 6 Sep 2026 12:33:22 +0200 Subject: [PATCH 8/8] fix(mcp): honor review on schema, writes, and store leaks Optional tool fields were required in the published schema, docs used snake_case names the binder does not accept, and a store IllegalArgumentException could still reach the client. Mark query, limit, and confidence optional, document contextId/propositionId, sanitize every repository exception, and keep dice_store off unless writes-enabled is set. Get now shows status so a stale fact does not look active. Signed-off-by: LordKay-sudo --- AGENTS.md | 2 +- README.md | 19 +-- dice-mcp-autoconfigure/AGENTS.md | 8 +- .../autoconfigure/DiceMcpAutoConfiguration.kt | 13 +- .../mcp/autoconfigure/DiceMcpProperties.kt | 6 + .../DiceMcpAutoConfigurationTest.kt | 47 ++++++- dice/AGENTS.md | 2 +- .../com/embabel/dice/mcp/DiceMcpSupport.kt | 56 +++++++- .../com/embabel/dice/mcp/DiceMcpTools.kt | 129 ++++++++++-------- .../com/embabel/dice/mcp/DiceMcpToolsTest.kt | 96 ++++++++++++- docs/design/architecture.md | 6 +- docs/design/retrieval-and-discovery.md | 6 +- 12 files changed, 302 insertions(+), 88 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5d6a4526..07287b70 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -67,7 +67,7 @@ The `dice` module is organized by responsibility: | `com.embabel.dice.provenance` | `ProvenanceEntry`, `SourceLocator` — rich evidence links from propositions back to source material | | `com.embabel.dice.query.oracle` | `Oracle`, `LlmOracle`, `PrologTools` — natural language question answering over propositions | | `com.embabel.dice.web.rest` | Optional REST endpoints for the pipeline and memory; activated by `spring-webmvc` on the classpath | -| `com.embabel.dice.mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `context_id` on every call is a scope, not a credential | +| `com.embabel.dice.mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `contextId` on every call is a scope, not a credential | ## Conventions diff --git a/README.md b/README.md index 9dd4686a..b7ecc9b5 100644 --- a/README.md +++ b/README.md @@ -2418,9 +2418,11 @@ Everything is pushed into the database rather than scanned in memory: Expose DICE recall/list/store/get to an MCP client (Claude Desktop, Cursor, etc.) with `dice-mcp-autoconfigure` and embabel-agent's MCP server starter. Off until you set -`embabel.dice.mcp.enabled=true`. Every tool takes a `context_id` — that is a scope, not a -credential. It keeps one call from reading another context; authorization is the host MCP -server's job. In-process `Memory` / `DiscoveryTools` bake context in at construction instead. +`embabel.dice.mcp.enabled=true`. Every tool takes a `contextId` (the Kotlin parameter name +`KotlinMethodTool` publishes). That is a scope, not a credential. It keeps one call from +reading another context; authorization is the host MCP server's job. In-process `Memory` / +`DiscoveryTools` bake context in at construction instead. `dice_store` is off until you set +`embabel.dice.mcp.writes-enabled=true`. ```xml @@ -2444,17 +2446,18 @@ embabel: | Tool | Description | |------|-------------| -| `dice_recall` | Hybrid semantic + keyword search in a `context_id` | +| `dice_recall` | Hybrid semantic + keyword search in a `contextId` | | `dice_list` | List active propositions for a context | -| `dice_store` | Store a proposition directly (no mentions or provenance — see below) | -| `dice_get` | Fetch one proposition by id; a miss and a foreign-context id look the same | +| `dice_store` | Store a proposition directly (off unless `writes-enabled=true`) | +| `dice_get` | Fetch one proposition by `propositionId`; includes status so a stale fact does not look active | `dice_recall` and `dice_list` share one result format, each line carrying the `id=` that `dice_get` takes, so a client can search and then drill into a single fact. Their `limit` is clamped to 100. -`dice_store` writes a fact with empty mentions and no provenance, so it is retrievable by -vector and keyword only — not by entity expansion or graph projection. Use the ingestion +`dice_store` is omitted from the export unless `embabel.dice.mcp.writes-enabled=true`. When +it is on, it writes a fact with empty mentions and no provenance, so it is retrievable by +vector and keyword only, not by entity expansion or graph projection. Use the ingestion pipeline when the fact needs to be wired into the rest of the knowledge flow. Discovery and graph tools stay on `DiscoveryTools.asTools(...)` / `GraphQueryTools.asTools(...)`. diff --git a/dice-mcp-autoconfigure/AGENTS.md b/dice-mcp-autoconfigure/AGENTS.md index 1916e02e..a6943d27 100644 --- a/dice-mcp-autoconfigure/AGENTS.md +++ b/dice-mcp-autoconfigure/AGENTS.md @@ -2,7 +2,7 @@ Spring Boot wiring that exports [DiceMcpTools](../dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt) over embabel-agent's MCP server. No domain logic — just an `@AutoConfiguration` that assembles beans -from `dice`. `context_id` on every tool is a caller-supplied scope, not a credential — the +from `dice`. `contextId` on every tool is a caller-supplied scope, not a credential. The check lives on `DiceMcpTools` itself; authorization is the host MCP server's job. ## What's here @@ -12,7 +12,7 @@ check lives on `DiceMcpTools` itself; authorization is the host MCP server's job `PropositionRepository` bean. `afterName` waits for `DiceStorageAutoConfiguration` when that module is present so the store bean exists before `@ConditionalOnBean` is asked. - **`DiceMcpProperties`** — `embabel.dice.mcp`: `enabled` (default false), `min-confidence` - (default 0.5), `default-limit` (default 10). + (default 0.5), `default-limit` (default 10), `writes-enabled` (default false). ## Property reference @@ -21,6 +21,7 @@ check lives on `DiceMcpTools` itself; authorization is the host MCP server's job | `embabel.dice.mcp.enabled` | `false` | Master switch. Off means no beans. | | `embabel.dice.mcp.min-confidence` | `0.5` | Minimum effective confidence for recall/list | | `embabel.dice.mcp.default-limit` | `10` | Default result cap for recall/list. Must be `1..100` | +| `embabel.dice.mcp.writes-enabled` | `false` | When true, export also includes `dice_store` | Every collaborator is `@ConditionalOnMissingBean`, so an app's own `DiceMcpTools` or `diceMcpToolExport` bean wins. @@ -35,7 +36,8 @@ Every collaborator is `@ConditionalOnMissingBean`, so an app's own `DiceMcpTools ## Gotchas - MCP export is **opt-in**. Unlike the collector (`enabled` default true), this stays dark until - `embabel.dice.mcp.enabled=true`. + `embabel.dice.mcp.enabled=true`. `dice_store` is a second switch (`writes-enabled`, default + false) because a direct write skips extraction, admission, and provenance. - Without a `PropositionRepository` bean the auto-config class may load but it exports nothing. - `default-limit` is bounded by `DiceMcpTools.MAX_LIMIT` (100), the ceiling the tools clamp every caller-supplied `limit` to. A larger default would bind and then be silently truncated on every diff --git a/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfiguration.kt b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfiguration.kt index 15776a23..b4c6e367 100644 --- a/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfiguration.kt +++ b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfiguration.kt @@ -48,6 +48,7 @@ import org.springframework.context.annotation.Bean * dice: * mcp: * enabled: true + * writes-enabled: true # optional; dice_store stays off otherwise * ``` * * `afterName` waits for `dice-storage-autoconfigure` when that module is on the classpath, so @@ -77,8 +78,14 @@ class DiceMcpAutoConfiguration { @Bean("diceMcpToolExport") @ConditionalOnBean(DiceMcpTools::class) @ConditionalOnMissingBean(name = ["diceMcpToolExport"]) - fun diceMcpToolExport(tools: DiceMcpTools): McpToolExport { - logger.info("Exporting DICE MCP tools: {}", DiceMcpTools.TOOL_NAMES.sorted()) - return McpToolExport.fromToolObject(ToolObject(objects = listOf(tools))) + fun diceMcpToolExport(tools: DiceMcpTools, properties: DiceMcpProperties): McpToolExport { + val exported = if (properties.writesEnabled) { + ToolObject(objects = listOf(tools)) + } else { + ToolObject(objects = listOf(tools)).withFilter { it != DiceMcpTools.STORE } + } + val names = if (properties.writesEnabled) DiceMcpTools.TOOL_NAMES else DiceMcpTools.READ_TOOL_NAMES + logger.info("Exporting DICE MCP tools: {}", names.sorted()) + return McpToolExport.fromToolObject(exported) } } diff --git a/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt index 1f36606b..c767dba8 100644 --- a/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt +++ b/dice-mcp-autoconfigure/src/main/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpProperties.kt @@ -35,6 +35,12 @@ data class DiceMcpProperties( * value would be silently clamped at call time, so it fails startup instead. */ val defaultLimit: Int = 10, + /** + * When false (the default), `dice_store` is omitted from the export. Store writes an ACTIVE + * proposition without extraction, admission, or provenance; that is a different capability + * from recall and is independently opt-in. + */ + val writesEnabled: Boolean = false, ) { init { require(minConfidence in 0.0..1.0) { "embabel.dice.mcp.min-confidence must be between 0.0 and 1.0" } diff --git a/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt b/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt index 40433a89..bba99793 100644 --- a/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt +++ b/dice-mcp-autoconfigure/src/test/kotlin/com/embabel/dice/mcp/autoconfigure/DiceMcpAutoConfigurationTest.kt @@ -78,7 +78,7 @@ class DiceMcpAutoConfigurationTest { } @Test - fun `enabled with a repository wires tools and exports the four names`() { + fun `enabled with a repository wires tools and exports the read names`() { runner .withUserConfiguration(StubPropositionRepositoryConfig::class.java) .withPropertyValues("embabel.dice.mcp.enabled=true") @@ -86,12 +86,53 @@ class DiceMcpAutoConfigurationTest { assertThat(ctx).hasSingleBean(DiceMcpTools::class.java) assertThat(ctx).hasBean("diceMcpToolExport") + val export = ctx.getBean("diceMcpToolExport") + val names = export.toolCallbacks.map { it.toolDefinition.name() }.toSet() + assertThat(names).isEqualTo(DiceMcpTools.READ_TOOL_NAMES) + assertThat(names).doesNotContain(DiceMcpTools.STORE) + assertThat(ctx.getBean().writesEnabled).isFalse() + } + } + + @Test + fun `writes-enabled exports store as well`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.writes-enabled=true", + ) + .run { ctx -> + assertThat(ctx.getBean().writesEnabled).isTrue() val export = ctx.getBean("diceMcpToolExport") val names = export.toolCallbacks.map { it.toolDefinition.name() }.toSet() assertThat(names).isEqualTo(DiceMcpTools.TOOL_NAMES) } } + @Test + fun `exported list callback accepts documented camelCase JSON with optionals omitted`() { + runner + .withUserConfiguration(StubPropositionRepositoryConfig::class.java) + .withPropertyValues( + "embabel.dice.mcp.enabled=true", + "embabel.dice.mcp.min-confidence=0.0", + ) + .run { ctx -> + val tools = ctx.getBean() + tools.storeMemory("session-1", "Only fact", confidence = 0.9) + + val export = ctx.getBean("diceMcpToolExport") + val list = export.toolCallbacks.single { it.toolDefinition.name() == DiceMcpTools.LIST } + val schema = list.toolDefinition.inputSchema() + assertThat(schema).contains("contextId") + assertThat(schema).doesNotContain("context_id") + + val result = list.call("""{"contextId":"session-1"}""") + assertThat(result).contains("Only fact") + } + } + @Test fun `minConfidence and defaultLimit bind from properties`() { runner @@ -100,11 +141,13 @@ class DiceMcpAutoConfigurationTest { "embabel.dice.mcp.enabled=true", "embabel.dice.mcp.min-confidence=0.7", "embabel.dice.mcp.default-limit=3", + "embabel.dice.mcp.writes-enabled=true", ) .run { ctx -> val props = ctx.getBean() assertThat(props.minConfidence).isEqualTo(0.7) assertThat(props.defaultLimit).isEqualTo(3) + assertThat(props.writesEnabled).isTrue() } } @@ -207,7 +250,7 @@ class DiceMcpAutoConfigurationTest { val export = ctx.getBean("diceMcpToolExport") val names = export.toolCallbacks.map { it.toolDefinition.name() }.toSet() - assertThat(names).isEqualTo(DiceMcpTools.TOOL_NAMES) + assertThat(names).isEqualTo(DiceMcpTools.READ_TOOL_NAMES) } } diff --git a/dice/AGENTS.md b/dice/AGENTS.md index bdd87d33..a8e05d9a 100644 --- a/dice/AGENTS.md +++ b/dice/AGENTS.md @@ -76,7 +76,7 @@ These map onto the lifecycle in [proposition-lifecycle](../docs/design/propositi | `query.discovery` | `RetrievalRouter`, `DiscoveryQuery`, `RetrievalMode`, discovery DTOs — mode-routed retrieval entry point | | `temporal` | `TemporalMetadata` — bitemporal valid/observed windows, explicit retraction | | `agent` | `Memory`, `MemoryRetriever` (agent-facing view), `ProvenanceResolver`; **`DiscoveryTools`** — `@LlmTool`-annotated tools wrapping `RetrievalRouter` (query propositions, graph path, why-explain, projection health, collector dry-run) with context baked in at construction so an agent can't cross context boundaries; **`GraphQueryTools`** — `@LlmTool`-annotated tools wrapping the `GraphQuery` facade (entity neighbourhood, path between entities, why-explain) | -| `mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `context_id` on every call is a caller-supplied scope (not a credential) so one call cannot read another context; authorization is the host MCP server's job | +| `mcp` | `DiceMcpTools` — simplified MCP tool surface (`dice_recall`, `dice_list`, `dice_store`, `dice_get`); `contextId` on every call is a caller-supplied scope (not a credential) so one call cannot read another context; authorization is the host MCP server's job | | `web.rest` | `PropositionPipelineController`, `MemoryController`, **`DiscoveryController`** — REST surface for discovery operations (`/api/v1/contexts/{contextId}/discovery`; routes query, path, why, projection health, and collector dry-run; context comes from the URL path only), API key security — optional, activated by `spring-webmvc` | | `operations` | `PropositionAbstractor`, `PropositionContraster` — higher-level proposition management | | `operations.consolidation` | The dream-loop steps as composable passes: `ConsolidationPass`/`ConsolidationPassResult`, `SessionConsolidationPass`, `AbstractionPass`, `ContradictionResolutionPass`, `DecaySweepPass` | diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt index 17f034ba..7871cfba 100644 --- a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpSupport.kt @@ -21,6 +21,7 @@ import com.embabel.dice.proposition.Proposition import com.embabel.dice.proposition.PropositionQuery import com.embabel.dice.proposition.PropositionRepository import com.embabel.dice.proposition.PropositionStatus +import java.util.Locale /** * Shared retrieval and formatting helpers for [DiceMcpTools]. @@ -37,8 +38,8 @@ internal object DiceMcpSupport { * Takes an id the caller has already put through [requireContextId], so validation happens * once per tool call rather than once per query built. * - * STALE / SUPERSEDED / CONTRADICTED propositions are excluded by default — the same guard - * [com.embabel.dice.agent.Memory] applies before results reach an LLM. + * `STALE` / `SUPERSEDED` / `CONTRADICTED` propositions are excluded by default. The same + * guard [com.embabel.dice.agent.Memory] applies before results reach an LLM. */ fun baseQuery(scopedContextId: String, minConfidence: Double): PropositionQuery = PropositionQuery.forContextId(ContextId(scopedContextId)) @@ -47,7 +48,7 @@ internal object DiceMcpSupport { fun requireContextId(contextId: String): String { val scoped = contextId.trim() - require(scoped.isNotBlank()) { "context_id must not be blank" } + require(scoped.isNotBlank()) { "contextId must not be blank" } return scoped } @@ -77,7 +78,7 @@ internal object DiceMcpSupport { retriever.rankedPropositions(trimmed, base, limit) } if (hits.isEmpty()) { - // The no-query wording matches dice_list's for the same situation. A query miss adds + // The no-query wording matches `dice_list` for the same situation. A query miss adds // how much *is* in scope, so the caller can tell "your query was wrong, try again" // apart from "this context is empty, stop asking". return if (trimmed == null) { @@ -108,7 +109,7 @@ internal object DiceMcpSupport { fun formatProposition(proposition: Proposition): String = buildString { append("id=${proposition.id}") - append(" | confidence=${"%.2f".format(proposition.effectiveConfidence())}") + append(" | confidence=${"%.2f".format(Locale.ROOT, proposition.effectiveConfidence())}") append(" | ${proposition.text}") if (proposition.mentions.isNotEmpty()) { val entities = proposition.mentions.joinToString("; ") { mention -> @@ -117,4 +118,49 @@ internal object DiceMcpSupport { append(" | entities: $entities") } } + + /** + * Detail contract for `dice_get`. `dice_list` and `dice_recall` stay compact; get must show + * [Proposition.status] so a stale or contradicted fact does not look active. + */ + fun formatDetail(proposition: Proposition): String { + val detail = McpPropositionDetail.from(proposition) + return buildString { + append("id=${detail.id}") + append(" | status=${detail.status}") + append(" | confidence=${"%.2f".format(Locale.ROOT, detail.effectiveConfidence)}") + append(" | ${detail.text}") + if (detail.evidence.isNotEmpty()) { + append(" | evidence: ${detail.evidence.joinToString("; ")}") + } + } + } +} + +/** + * Outward get payload. MCP still returns text today; this type is the contract so we do not + * publish [Proposition] or lock callers to a one-line summary. + */ +data class McpPropositionDetail( + val id: String, + val contextId: String, + val text: String, + val status: String, + val confidence: Double, + val effectiveConfidence: Double, + val evidence: List, +) { + companion object { + fun from(proposition: Proposition): McpPropositionDetail = McpPropositionDetail( + id = proposition.id, + contextId = proposition.contextIdValue, + text = proposition.text, + status = proposition.status.name, + confidence = proposition.confidence, + effectiveConfidence = proposition.effectiveConfidence(), + evidence = (proposition.grounding + proposition.provenanceEntries.map { entry -> + entry.chunkId ?: entry.locator.toString() + }).filter { it.isNotBlank() }.distinct(), + ) + } } diff --git a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt index 452f8b0e..fb51d0e8 100644 --- a/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt +++ b/dice/src/main/kotlin/com/embabel/dice/mcp/DiceMcpTools.kt @@ -27,14 +27,16 @@ import org.slf4j.LoggerFactory * * In-process [com.embabel.dice.agent.Memory] and [com.embabel.dice.agent.DiscoveryTools] bake * [ContextId] in at construction, so an agent cannot name another tenant. MCP clients are - * stateless and may serve many sessions, so every tool takes an explicit `context_id`. That is + * stateless and may serve many sessions, so every tool takes an explicit `contextId`. That is * a caller-supplied scope, not a credential: it keeps one call from crossing contexts, and - * authorization is the host MCP server's job. Recall and list start from - * [com.embabel.dice.proposition.PropositionQuery.forContextId]; get collapses a missing id and - * a foreign id into one answer so the tool cannot confirm that an id it does not own exists. + * authorization is the host MCP server's job. `dice_recall` and `dice_list` start from + * [com.embabel.dice.proposition.PropositionQuery.forContextId]; `dice_get` collapses a missing + * id and a foreign id into one answer so the tool cannot confirm that an id it does not own + * exists. * - * Rod's #5: expose tools with simplified parameters. This class is that surface: recall, list, - * store, get. Extraction and discovery stay on the existing in-process `asTools()` path. + * Rod's #5: expose tools with simplified parameters. This class is that surface: + * `dice_recall`, `dice_list`, `dice_store`, `dice_get`. Extraction and discovery stay on the + * existing in-process `asTools()` path. * * Export through embabel-agent's [com.embabel.agent.mcpserver.McpToolExport], or add * `dice-mcp-autoconfigure` with `embabel.dice.mcp.enabled=true`. @@ -57,21 +59,20 @@ class DiceMcpTools( } /** - * Run a tool body, keeping store and driver detail away from the caller. + * Run a store call, keeping driver detail away from the caller. * - * [IllegalArgumentException] is ours — a blank `context_id`, a blank `text` — so it passes - * through and tells the model what to fix. Anything else came from the store: the cause is - * logged here and the exception thrown on has **no cause attached**, so a stack trace - * serialized back by the MCP layer cannot carry Cypher, hostnames, or credentials to an - * external client. Same rule `DiscoveryController` applies to its own 500s. + * Caller validation happens **before** this wrapper so a blank `contextId` still throws + * [IllegalArgumentException]. Everything thrown from here, including a store + * [IllegalArgumentException], becomes a cause-free generic failure. + * + * [MethodTool] then logs that sanitized exception, not the store one. This method logs + * `e.message` only, so operators still see the driver text without a second stack. */ private fun guarded(tool: String, block: () -> String): String = try { block() - } catch (e: IllegalArgumentException) { - throw e } catch (e: Exception) { - logger.error("MCP tool {} failed", tool, e) + logger.error("MCP tool {} failed: {}", tool, e.message ?: e.toString()) throw IllegalStateException("$tool failed: the knowledge store is unavailable") } @@ -87,18 +88,21 @@ class DiceMcpTools( fun recall( @LlmTool.Param(description = "Context to search within (session, user, or tenant id).") contextId: String, - @LlmTool.Param(description = "What to recall, in natural language. Omit to list all memories.") + @LlmTool.Param(description = "What to recall, in natural language. Omit to list all memories.", required = false) query: String? = null, - @LlmTool.Param(description = "Maximum results (default 10, capped at 100).") + @LlmTool.Param(description = "Maximum results (default 10, capped at 100).", required = false) limit: Int = defaultLimit, - ): String = guarded(RECALL) { - DiceMcpSupport.recall( - repository = repository, - contextId = contextId, - query = query, - limit = limit.coerceIn(1, MAX_LIMIT), - minConfidence = minConfidence, - ) + ): String { + DiceMcpSupport.requireContextId(contextId) + return guarded(RECALL) { + DiceMcpSupport.recall( + repository = repository, + contextId = contextId, + query = query, + limit = limit.coerceIn(1, MAX_LIMIT), + minConfidence = minConfidence, + ) + } } /** @@ -111,21 +115,23 @@ class DiceMcpTools( fun listMemories( @LlmTool.Param(description = "Context to list.") contextId: String, - @LlmTool.Param(description = "Maximum results (default 10, capped at 100).") + @LlmTool.Param(description = "Maximum results (default 10, capped at 100).", required = false) limit: Int = defaultLimit, - ): String = guarded(LIST) { + ): String { val scoped = DiceMcpSupport.requireContextId(contextId) - val query = DiceMcpSupport.baseQuery(scoped, minConfidence) - .orderedByEffectiveConfidence() - .withLimit(limit.coerceIn(1, MAX_LIMIT)) - val propositions = repository.query(query) - if (propositions.isEmpty()) { - "No memories in context '$scoped'." - } else { - DiceMcpSupport.render( - "Found ${propositions.size} memories in context '$scoped':", - propositions, - ) + return guarded(LIST) { + val query = DiceMcpSupport.baseQuery(scoped, minConfidence) + .orderedByEffectiveConfidence() + .withLimit(limit.coerceIn(1, MAX_LIMIT)) + val propositions = repository.query(query) + if (propositions.isEmpty()) { + "No memories in context '$scoped'." + } else { + DiceMcpSupport.render( + "Found ${propositions.size} memories in context '$scoped':", + propositions, + ) + } } } @@ -141,19 +147,21 @@ class DiceMcpTools( contextId: String, @LlmTool.Param(description = "The fact to remember, in natural language.") text: String, - @LlmTool.Param(description = "Confidence between 0 and 1 (default 0.8).") + @LlmTool.Param(description = "Confidence between 0 and 1 (default 0.8).", required = false) confidence: Double = 0.8, - ): String = guarded(STORE) { + ): String { val scoped = DiceMcpSupport.requireContextId(contextId) require(text.isNotBlank()) { "text must not be blank" } - val proposition = Proposition( - contextId = ContextId(scoped), - text = text.trim(), - mentions = emptyList(), - confidence = confidence.coerceIn(0.0, 1.0), - ) - val saved = repository.save(proposition) - "Stored proposition ${saved.id}: ${saved.text}" + return guarded(STORE) { + val proposition = Proposition( + contextId = ContextId(scoped), + text = text.trim(), + mentions = emptyList(), + confidence = confidence.coerceIn(0.0, 1.0), + ) + val saved = repository.save(proposition) + "Stored proposition ${saved.id}: ${saved.text}" + } } /** @@ -168,18 +176,20 @@ class DiceMcpTools( contextId: String, @LlmTool.Param(description = "Proposition id returned by recall, list, or store.") propositionId: String, - ): String = guarded(GET) { + ): String { val scoped = DiceMcpSupport.requireContextId(contextId) val id = propositionId.trim() - require(id.isNotBlank()) { "proposition_id must not be blank" } - val proposition = repository.findById(id) - // One answer for "no such id" and "that id lives in another context". Distinguishing - // them would confirm to a caller that an id it does not own exists somewhere, and - // MemoryController collapses both into a 404 for exactly that reason. - if (proposition == null || proposition.contextIdValue != scoped) { - "No proposition with id '$id' in context '$scoped'." - } else { - DiceMcpSupport.formatProposition(proposition) + require(id.isNotBlank()) { "propositionId must not be blank" } + return guarded(GET) { + val proposition = repository.findById(id) + // One answer for "no such id" and "that id lives in another context". Distinguishing + // them would confirm to a caller that an id it does not own exists somewhere, and + // MemoryController collapses both into a 404 for exactly that reason. + if (proposition == null || proposition.contextIdValue != scoped) { + "No proposition with id '$id' in context '$scoped'." + } else { + DiceMcpSupport.formatDetail(proposition) + } } } @@ -191,6 +201,9 @@ class DiceMcpTools( val TOOL_NAMES: Set = setOf(RECALL, LIST, STORE, GET) + /** Names exported when writes are off. */ + val READ_TOOL_NAMES: Set = setOf(RECALL, LIST, GET) + const val DEFAULT_MIN_CONFIDENCE = 0.5 const val DEFAULT_LIMIT = 10 diff --git a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt index e601fdf1..2cdbaebb 100644 --- a/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/mcp/DiceMcpToolsTest.kt @@ -15,6 +15,7 @@ */ package com.embabel.dice.mcp +import com.embabel.agent.api.tool.Tool import com.embabel.agent.core.ContextId import com.embabel.dice.proposition.EntityMention import com.embabel.dice.proposition.Proposition @@ -151,10 +152,28 @@ class DiceMcpToolsTest { ) val fetched = tools.getProposition("session-1", proposition.id) assertTrue(fetched.contains("Old fact")) + assertTrue(fetched.contains("status=STALE"), fetched) + assertFalse(fetched.contains("status=ACTIVE"), fetched) } @Test - fun `whitespace around context_id is trimmed`() { + fun `get of a contradicted proposition shows that status`() { + val proposition = repository.save( + Proposition( + contextId = ContextId("session-1"), + text = "Disputed fact", + mentions = emptyList(), + confidence = 0.8, + status = PropositionStatus.CONTRADICTED, + ), + ) + val fetched = tools.getProposition("session-1", proposition.id) + assertTrue(fetched.contains("status=CONTRADICTED"), fetched) + assertTrue(fetched.contains("Disputed fact")) + } + + @Test + fun `whitespace around contextId is trimmed`() { val stored = tools.storeMemory(" session-1 ", "Padded context fact") val id = stored.substringAfter("Stored proposition ").substringBefore(":") val fetched = tools.getProposition(" session-1", id) @@ -629,6 +648,7 @@ class DiceMcpToolsTest { ) val listed = tools.listMemories("session-1", limit = 10) assertTrue(listed.contains("Jim (Person)")) + assertTrue(listed.contains("confidence=0.90"), listed) } } @@ -679,6 +699,65 @@ class DiceMcpToolsTest { names, ) } + + @Test + fun `exported schema uses camelCase names and marks optionals not required`() { + val exported = DiceMcpTools.asTools(tools).associateBy { it.definition.name } + + val recall = params(exported, DiceMcpTools.RECALL) + assertEquals(setOf("contextId", "query", "limit"), recall.keys) + assertTrue(recall.getValue("contextId").required) + assertFalse(recall.getValue("query").required) + assertFalse(recall.getValue("limit").required) + + val list = params(exported, DiceMcpTools.LIST) + assertEquals(setOf("contextId", "limit"), list.keys) + assertTrue(list.getValue("contextId").required) + assertFalse(list.getValue("limit").required) + + val store = params(exported, DiceMcpTools.STORE) + assertEquals(setOf("contextId", "text", "confidence"), store.keys) + assertTrue(store.getValue("contextId").required) + assertTrue(store.getValue("text").required) + assertFalse(store.getValue("confidence").required) + + val get = params(exported, DiceMcpTools.GET) + assertEquals(setOf("contextId", "propositionId"), get.keys) + assertTrue(get.getValue("contextId").required) + assertTrue(get.getValue("propositionId").required) + } + + @Test + fun `documented camelCase JSON invokes list and recall with optionals omitted`() { + tools.storeMemory("session-1", "Only fact", confidence = 0.9) + val exported = DiceMcpTools.asTools(tools).associateBy { it.definition.name } + + val listed = exported.getValue(DiceMcpTools.LIST).call("""{"contextId":"session-1"}""") + assertTrue((listed as Tool.Result.Text).content.contains("Only fact"), listed.content) + + val recalled = exported.getValue(DiceMcpTools.RECALL).call("""{"contextId":"session-1"}""") + assertTrue((recalled as Tool.Result.Text).content.contains("Only fact"), recalled.content) + } + + @Test + fun `documented camelCase JSON invokes get and store`() { + val exported = DiceMcpTools.asTools(tools).associateBy { it.definition.name } + val stored = exported.getValue(DiceMcpTools.STORE).call( + """{"contextId":"session-1","text":"Documented store"}""", + ) + val storedText = (stored as Tool.Result.Text).content + assertTrue(storedText.startsWith("Stored proposition"), storedText) + val id = storedText.substringAfter("Stored proposition ").substringBefore(":") + + val fetched = exported.getValue(DiceMcpTools.GET).call( + """{"contextId":"session-1","propositionId":"$id"}""", + ) + assertTrue((fetched as Tool.Result.Text).content.contains("Documented store"), fetched.content) + assertTrue(fetched.content.contains("status=ACTIVE"), fetched.content) + } + + private fun params(exported: Map, name: String) = + exported.getValue(name).definition.inputSchema.parameters.associateBy { it.name } } @Nested @@ -752,6 +831,18 @@ class DiceMcpToolsTest { } } + @Test + fun `store IllegalArgumentException from the repository is sanitized`() { + val iaeFailing = DiceMcpTools(LeakingStore(leak, asArgument = true), minConfidence = 0.0) + val thrown = assertThrows { + iaeFailing.storeMemory("session-1", "A fact") + } + assertTrue(thrown.message!!.endsWith("failed: the knowledge store is unavailable")) + assertEquals(null, thrown.cause) + assertFalse(thrown.message!!.contains(leak)) + assertFalse(thrown.stackTraceToString().contains(leak)) + } + private fun assertSanitized(call: () -> String) { val thrown = assertThrows { call() } assertTrue(thrown.message!!.endsWith("failed: the knowledge store is unavailable")) @@ -770,6 +861,7 @@ class DiceMcpToolsTest { */ private class LeakingStore( private val leak: String, + private val asArgument: Boolean = false, ) : PropositionRepository by InMemoryPropositionRepository() { override fun save(proposition: Proposition): Proposition = explode() override fun findById(id: String): Proposition? = explode() @@ -782,6 +874,6 @@ class DiceMcpToolsTest { ): List = explode() private fun explode(): Nothing = - throw RuntimeException(leak) + if (asArgument) throw IllegalArgumentException(leak) else throw RuntimeException(leak) } } diff --git a/docs/design/architecture.md b/docs/design/architecture.md index b0559848..dfefd8be 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -259,10 +259,12 @@ isolated — agent tools bake it in at construction, REST takes it from the URL of those surfaces accepts a context override in the request body. External MCP clients are stateless and may serve many sessions, so they cannot bake a context in -at construction. `DiceMcpTools` takes `context_id` on every call — a caller-supplied scope, not +at construction. `DiceMcpTools` takes `contextId` on every call — a caller-supplied scope, not a credential — and `get` treats a missing id and a foreign-context id the same way. Authorization is the host MCP server's job. Export is opt-in (`dice-mcp-autoconfigure`, -`embabel.dice.mcp.enabled=true`). Discovery and graph tools stay on the in-process `asTools()` path. +`embabel.dice.mcp.enabled=true`). `dice_store` is a second switch +(`embabel.dice.mcp.writes-enabled`, default false) because a direct write skips extraction, +admission, and provenance. Discovery and graph tools stay on the in-process `asTools()` path. ## Events diff --git a/docs/design/retrieval-and-discovery.md b/docs/design/retrieval-and-discovery.md index b36ff8fc..80109b8d 100644 --- a/docs/design/retrieval-and-discovery.md +++ b/docs/design/retrieval-and-discovery.md @@ -158,10 +158,10 @@ the request body has no context field, so a caller *cannot* ask one context's en context's data. Cross-context reads aren't forbidden by a check; they're structurally impossible, and an LLM given the agent tools can't wander across context boundaries either. -The exception is external MCP: those clients are stateless, so `DiceMcpTools` takes `context_id` on +The exception is external MCP: those clients are stateless, so `DiceMcpTools` takes `contextId` on every call and checks it on `get`. That is a parameter, not a body-level override of a baked-in -context, and recall/list still start from `PropositionQuery.forContextId`. `context_id` is a scope, -not a credential — any client can name any tenant; authorization is the host MCP server's job. +context, and recall/list still start from `PropositionQuery.forContextId`. `contextId` is a scope, +not a credential; any client can name any tenant; authorization is the host MCP server's job. Two concerns drive this: a stable external contract (internal types can evolve without breaking the wire, and a leak-check guards against a domain type sneaking into a DTO by accident) and that