diff --git a/CLAUDE.md b/CLAUDE.md index d90d62e..2b774a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,7 +56,8 @@ extraction, Meilisearch indexing with scheduled refresh, and four MCP tools │ ├── MeilisearchContentIndexStore.java # ContentIndexStore backed by MeilisearchClient │ ├── ContentBootstrapStep.java # SearchIndexBootstrapStep: startup full reindex │ ├── ContentRefreshScheduler.java # @Scheduled incremental re-crawl (cron, guarded) -│ └── ContentSearchService.java # read facade: multiSearch + Highlighter (+ result records) +│ ├── ContentSearchService.java # read facade: multiSearch + Highlighter (+ result records) +│ └── ContentMcpToolProvider.java # McpToolProvider: the 4 tools on /mcp ├── src/main/resources/application.yaml # datasource, JPA, OAuth2, MCP, Meilisearch, content config ├── src/test/java/com/openelements/content/ # behavior tests (context, MCP enabled/disabled, search-down, jsoup) └── docs/ @@ -119,6 +120,15 @@ bootstrapping and to never overlap, with per-source fault isolation. On the read `multi-search` bodies (AND-combined `source`/`locale`/`categories`/`since` filters, `publishedDate:desc` tie-breaker, `Highlighter` boundary markers) and returns `SearchHit`s with HTML-safe snippets, plus `listPosts`, `getByUrlOrId`, and `categoryFacets`. Read-only; the scoped key comes in spec 013. +`ContentMcpToolProvider` implements the library `McpToolProvider` (auto-aggregated by `McpServerConfig`) +and exposes the four tools on `/mcp` — `search_content`, `list_posts`, `get_post`, `list_categories` — +as thin adapters over `ContentSearchService`, using the house helpers for schemas/paging/error mapping +(invalid-argument / not-found / temporary-unavailable during bootstrap). + +> **Build note:** `spring-services` is pinned to a specific snapshot timestamp +> (`1.3.0-20260712.175350-13`) in `pom.xml`, not the floating `1.3.0-SNAPSHOT`, because a later +> snapshot was published with a broken (dependency-less) POM. Keep it pinned until a released `1.3.0` +> exists. > **Key gotcha:** the library ships no Spring Boot auto-configuration and couples MCP to a JPA > datasource, so `@Import({ McpConfiguration, SearchConfig })` alone does **not** boot. See diff --git a/docs/specs/001-project-skeleton/behaviors.md b/docs/specs/001-project-skeleton/behaviors.md index de80ee4..b875464 100644 --- a/docs/specs/001-project-skeleton/behaviors.md +++ b/docs/specs/001-project-skeleton/behaviors.md @@ -47,3 +47,14 @@ - **Given** `openelements.mcp.enabled=false` - **When** the app starts - **Then** no `/mcp` endpoint is exposed (the MCP server beans are not created) + +--- + +## Drift Log + +### 2026-07-13 — Caused by spec `012-content-mcp-tools` + +- **Affected scenario:** /mcp endpoint is exposed +- **Original behavior:** the `/mcp` server advertised the configured `server-name`/`server-version` with an **empty content-tool catalog** (no `McpToolProvider` beans registered). +- **Current behavior:** spec 012 registers `ContentMcpToolProvider`, so the context now contains one `McpToolProvider` bean and `/mcp` advertises the four content tools. The skeleton test `ContentMcpApplicationTests` was updated to assert the provider is present rather than that the catalog is empty. +- **Reason:** spec 012 (`ContentMcpToolProvider`) is the planned step that adds the content tools; the skeleton's "empty catalog" was only ever a temporary property of the pre-tools state. diff --git a/docs/specs/012-content-mcp-tools/steps.md b/docs/specs/012-content-mcp-tools/steps.md new file mode 100644 index 0000000..b29133f --- /dev/null +++ b/docs/specs/012-content-mcp-tools/steps.md @@ -0,0 +1,51 @@ +# Implementation Steps: Content MCP Tools + +## Step 1: `ContentMcpToolProvider` + +- [x] `@Component implements McpToolProvider` (gated on `openelements.mcp.enabled`), auto-aggregated by `McpServerConfig` +- [x] `toolSpecifications()` returns the four tools built with `McpTools.tool` + `McpToolSupport.spec` +- [x] `search_content` (query required; locale/source/category filters; paging) → `ContentSearchService.search` +- [x] `list_posts` (locale/source/category/since; paging) → `ContentSearchService.listPosts` +- [x] `get_post` (url or id; neither → `IllegalArgumentException`; absent → `NoSuchElementException`) → `getByUrlOrId` +- [x] `list_categories` (source/locale) → `categoryFacets` +- [x] Paging via `McpPaging.resolvePage/resolveSize`; bootstrap guard throws `McpUnavailableException` for search/list +- [x] Tool logic extracted to package-private methods for direct unit testing + +**Related behaviors:** all + +--- + +## Step 2: Tests + +- [x] `ContentMcpToolProviderTest` (capturing `ContentSearchService` subclass + real `McpToolSupport`/`McpPaging`): four tools registered; per-tool delegation, filter pass-through, size clamp, missing-arg/not-found/unavailable errors + +**Acceptance criteria:** +- [x] All tests pass (`mvn test`); build green + +--- + +## Step 3: Drift + dependency fixes + +- [x] Update spec 001's `ContentMcpApplicationTests` — the skeleton asserted an empty `McpToolProvider` catalog; a provider now exists. Drift recorded in `docs/specs/001-project-skeleton/behaviors.md`. +- [x] Pin `spring-services` to `1.3.0-20260712.175350-13` (the floating `1.3.0-SNAPSHOT` published a broken, dependency-less `-14` POM that broke the build). + +--- + +## Behavior Coverage + +| Scenario | Layer | Covered in Step | +|----------|-------|-----------------| +| Four tools appear on /mcp | Backend | Step 2 (`fourToolsAreRegistered`) | +| search_content returns highlighted hits | Backend | Step 2 (`searchPassesFiltersAndClampsSize`) + `ContentSearchServiceTest` (spec 011 highlighting) | +| Missing required query is rejected | Backend | Step 2 (`searchWithoutQueryThrows`) | +| Filters are passed through | Backend | Step 2 (`searchPassesFiltersAndClampsSize`) | +| Paging is honored and clamped | Backend | Step 2 (`searchPassesFiltersAndClampsSize`) | +| list_posts lists newest first / since filter | Backend | Step 2 (`listPostsFilterAndAvailability`) + spec 011 sort | +| get_post by url / id returns full post | Backend | Step 2 (`getPostByUrl`) | +| Neither url nor id is an error | Backend | Step 2 (`getPostWithoutArgsThrows`) | +| Unknown post is not-found | Backend | Step 2 (`getPostUnknownIsNotFound`) | +| list_categories returns counts / scoped | Backend | Step 2 (`listCategoriesDelegates`) | +| Tool reports unavailable during bootstrap | Backend | Step 2 (`searchUnavailableWhileBootstrapping`, `listPostsFilterAndAvailability`) | +| Each tool call is access-logged | Backend | Delegated to `McpToolSupport.spec` (library); tools are built via it. | + +All scenarios are backend; there is no frontend in this spec. diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index cafadfd..b04f17b 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -17,7 +17,7 @@ roadmap step, to be implemented sequentially (each builds on the previous). | 009 | 009-content-bootstrap-step | Content bootstrap step | backend, search | `ContentBootstrapStep` — initial reindex via `SearchIndexBootstrapStep` | #17 | done | | 010 | 010-content-refresh-scheduler | Content refresh scheduler | backend, scheduling | `ContentRefreshScheduler` — `@Scheduled` incremental re-crawl | #19 | done | | 011 | 011-content-search-service | Content search service | backend, search | `ContentSearchService` — `multiSearch` + highlighting facade | #21 | done | -| 012 | 012-content-mcp-tools | Content MCP tools | backend, mcp, api | `ContentMcpToolProvider` — the 4 MCP tools | — | open | +| 012 | 012-content-mcp-tools | Content MCP tools | backend, mcp, api | `ContentMcpToolProvider` — the 4 MCP tools | #23 | done | | 013 | 013-scoped-search-key | Scoped search key | backend, search, security | Read-only scoped Meilisearch key for the content index | — | open | | 014 | 014-ops-robustness | Ops & robustness | backend, infrastructure, observability | robots.txt handling, fault tolerance, logging/metrics | — | open | | 015 | 015-additional-web-sources | Additional web sources | backend, configuration | hiero.org/blog + Support & Care as config-only sources | — | open | diff --git a/pom.xml b/pom.xml index 43795cb..ebc72d2 100644 --- a/pom.xml +++ b/pom.xml @@ -42,7 +42,10 @@ - 1.3.0-SNAPSHOT + + 1.3.0-20260712.175350-13 1.18.3 0.8.13 diff --git a/src/main/java/com/openelements/content/ContentMcpToolProvider.java b/src/main/java/com/openelements/content/ContentMcpToolProvider.java new file mode 100644 index 0000000..b4ead87 --- /dev/null +++ b/src/main/java/com/openelements/content/ContentMcpToolProvider.java @@ -0,0 +1,143 @@ +package com.openelements.content; + +import static com.openelements.spring.base.mcp.McpTools.integer; +import static com.openelements.spring.base.mcp.McpTools.paginationProps; +import static com.openelements.spring.base.mcp.McpTools.prop; +import static com.openelements.spring.base.mcp.McpTools.requiredString; +import static com.openelements.spring.base.mcp.McpTools.string; +import static com.openelements.spring.base.mcp.McpTools.tool; + +import com.openelements.spring.base.mcp.McpPaging; +import com.openelements.spring.base.mcp.McpToolProvider; +import com.openelements.spring.base.mcp.McpToolSupport; +import com.openelements.spring.base.mcp.McpUnavailableException; +import com.openelements.spring.base.services.search.SearchReadinessState; +import io.modelcontextprotocol.server.McpServerFeatures.SyncToolSpecification; +import io.modelcontextprotocol.spec.McpSchema.Tool; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.stereotype.Component; + +/** + * Exposes the four content tools on the {@code /mcp} endpoint by implementing the library + * {@link McpToolProvider} (which {@code McpServerConfig} auto-aggregates). The tools are thin + * adapters over {@link ContentSearchService}; schemas, argument parsing, paging, access logging, and + * JSON-RPC error mapping are handled by the house helpers. + * + *

Error mapping (via {@link McpToolSupport#spec}): {@link IllegalArgumentException} → + * invalid-argument, {@link NoSuchElementException} → not-found, {@link McpUnavailableException} → + * temporary-unavailable. Search and list report unavailable while the index is bootstrapping. + * + *

Only created when the MCP server is enabled. + */ +@Component +@ConditionalOnProperty(prefix = "openelements.mcp", name = "enabled", havingValue = "true", matchIfMissing = true) +public class ContentMcpToolProvider implements McpToolProvider { + + private final ContentSearchService searchService; + private final McpToolSupport support; + private final McpPaging paging; + private final SearchReadinessState readinessState; + + public ContentMcpToolProvider(ContentSearchService searchService, McpToolSupport support, + McpPaging paging, SearchReadinessState readinessState) { + this.searchService = searchService; + this.support = support; + this.paging = paging; + this.readinessState = readinessState; + } + + @Override + public List toolSpecifications() { + return List.of(searchContent(), listPosts(), getPost(), listCategories()); + } + + // ---- tool definitions ---- + + private SyncToolSpecification searchContent() { + Map props = new LinkedHashMap<>(paginationProps()); + props.put("query", prop("string", "Search query (required).")); + props.put("locale", prop("string", "Filter by locale, e.g. en or de.")); + props.put("source", prop("string", "Filter by source id.")); + props.put("category", prop("string", "Filter by category.")); + Tool tool = tool("search_content", + "Full-text search across the indexed content; returns highlighted snippets.", + props, List.of("query")); + return support.spec(tool, this::searchContentLogic); + } + + private SyncToolSpecification listPosts() { + Map props = new LinkedHashMap<>(paginationProps()); + props.put("locale", prop("string", "Filter by locale, e.g. en or de.")); + props.put("source", prop("string", "Filter by source id.")); + props.put("category", prop("string", "Filter by category.")); + props.put("since", prop("string", "Only posts published on/after this ISO date (YYYY-MM-DD).")); + Tool tool = tool("list_posts", + "List indexed posts, newest first.", props, List.of()); + return support.spec(tool, this::listPostsLogic); + } + + private SyncToolSpecification getPost() { + Map props = new LinkedHashMap<>(); + props.put("url", prop("string", "The post URL (provide url or id).")); + props.put("id", prop("string", "The post id (provide url or id).")); + Tool tool = tool("get_post", + "Get the full content and metadata of a single post by url or id.", props, List.of()); + return support.spec(tool, this::getPostLogic); + } + + private SyncToolSpecification listCategories() { + Map props = new LinkedHashMap<>(); + props.put("source", prop("string", "Filter by source id.")); + props.put("locale", prop("string", "Filter by locale, e.g. en or de.")); + Tool tool = tool("list_categories", + "List content categories with their document counts.", props, List.of()); + return support.spec(tool, this::listCategoriesLogic); + } + + // ---- tool logic (package-private for direct unit testing) ---- + + Object searchContentLogic(Map args) { + requireReady(); + ContentFilters filters = new ContentFilters( + string(args, "source"), string(args, "locale"), string(args, "category"), null); + return searchService.search( + requiredString(args, "query"), filters, + paging.resolvePage(integer(args, "page")), paging.resolveSize(integer(args, "size"))); + } + + Object listPostsLogic(Map args) { + requireReady(); + ContentFilters filters = new ContentFilters( + string(args, "source"), string(args, "locale"), string(args, "category"), string(args, "since")); + return searchService.listPosts( + filters, paging.resolvePage(integer(args, "page")), paging.resolveSize(integer(args, "size"))); + } + + Object getPostLogic(Map args) { + String url = string(args, "url"); + String id = string(args, "id"); + if (isBlank(url) && isBlank(id)) { + throw new IllegalArgumentException("Either 'url' or 'id' is required"); + } + return searchService.getByUrlOrId(url, id) + .orElseThrow(() -> new NoSuchElementException("No post found for the given url/id")); + } + + Object listCategoriesLogic(Map args) { + return searchService.categoryFacets(string(args, "source"), string(args, "locale")); + } + + private void requireReady() { + if (readinessState.isBootstrapping()) { + throw new McpUnavailableException("Content index is still bootstrapping; try again shortly"); + } + } + + private static boolean isBlank(String value) { + return value == null || value.isBlank(); + } +} diff --git a/src/test/java/com/openelements/content/ContentMcpApplicationTests.java b/src/test/java/com/openelements/content/ContentMcpApplicationTests.java index 98206f2..ef12a85 100644 --- a/src/test/java/com/openelements/content/ContentMcpApplicationTests.java +++ b/src/test/java/com/openelements/content/ContentMcpApplicationTests.java @@ -58,8 +58,8 @@ void schedulingIsEnabled() { } @Test - @DisplayName("the /mcp server is exposed, advertises server name/version, and has no content tools yet") - void mcpEndpointIsExposedWithEmptyContentToolCatalog() { + @DisplayName("the /mcp server is exposed and advertises the configured server name/version") + void mcpEndpointIsExposed() { McpSyncServer mcpServer = context.getBean(McpSyncServer.class); assertThat(mcpServer).isNotNull(); @@ -70,7 +70,9 @@ void mcpEndpointIsExposedWithEmptyContentToolCatalog() { assertThat(mcpProperties.serverName()).isEqualTo("Open Elements Content MCP"); assertThat(mcpProperties.serverVersion()).isEqualTo("0.1.0"); - // No content-specific tools are registered yet (they arrive in spec 012). - assertThat(context.getBeansOfType(McpToolProvider.class)).isEmpty(); + // Drift (spec 012): the original skeleton asserted an empty McpToolProvider catalog; the + // content tools are now registered, so a ContentMcpToolProvider is present. + assertThat(context.getBeansOfType(McpToolProvider.class)) + .containsKey("contentMcpToolProvider"); } } diff --git a/src/test/java/com/openelements/content/ContentMcpToolProviderTest.java b/src/test/java/com/openelements/content/ContentMcpToolProviderTest.java new file mode 100644 index 0000000..09e2a1f --- /dev/null +++ b/src/test/java/com/openelements/content/ContentMcpToolProviderTest.java @@ -0,0 +1,204 @@ +package com.openelements.content; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.openelements.spring.base.mcp.McpPaging; +import com.openelements.spring.base.mcp.McpProperties; +import com.openelements.spring.base.mcp.McpToolSupport; +import com.openelements.spring.base.mcp.McpUnavailableException; +import com.openelements.spring.base.services.search.MeilisearchClient; +import com.openelements.spring.base.services.search.MeilisearchProperties; +import com.openelements.spring.base.services.search.SearchReadinessState; +import java.time.Duration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link ContentMcpToolProvider}: tool registration and each tool's logic (argument + * parsing, delegation, paging clamp, and error mapping) via a capturing {@link ContentSearchService} + * subclass and the real MCP helper beans. No mocks or live Meilisearch. + */ +@DisplayName("Content MCP tool provider") +class ContentMcpToolProviderTest { + + private static final int MAX_PAGE_SIZE = 50; + + private final CapturingSearchService searchService = new CapturingSearchService(); + private final McpPaging paging = new McpPaging(new McpProperties(true, "n", "v", MAX_PAGE_SIZE, 10, null)); + private final McpToolSupport support = new McpToolSupport(paging, new ObjectMapper()); + private final SearchReadinessState readiness = new SearchReadinessState(); + private final ContentMcpToolProvider provider = + new ContentMcpToolProvider(searchService, support, paging, readiness); + + @BeforeEach + void markBootstrapComplete() { + readiness.markBootstrappingFinished(); + } + + private static Map args(Object... keyValues) { + Map map = new HashMap<>(); + for (int i = 0; i < keyValues.length; i += 2) { + map.put((String) keyValues[i], keyValues[i + 1]); + } + return map; + } + + @Test + @DisplayName("all four tools are registered with names") + void fourToolsAreRegistered() { + List names = provider.toolSpecifications().stream().map(spec -> spec.tool().name()).toList(); + + assertThat(names).containsExactlyInAnyOrder( + "search_content", "list_posts", "get_post", "list_categories"); + } + + @Test + @DisplayName("search_content passes filters through and clamps the page size") + void searchPassesFiltersAndClampsSize() { + searchContentResult(); + provider.searchContentLogic(args("query", "wallet", "locale", "de", "source", "open-elements", "size", 999)); + + assertThat(searchService.lastQuery).isEqualTo("wallet"); + assertThat(searchService.lastFilters.locale()).isEqualTo("de"); + assertThat(searchService.lastFilters.source()).isEqualTo("open-elements"); + assertThat(searchService.lastSize).isEqualTo(MAX_PAGE_SIZE); // clamped from 999 + } + + @Test + @DisplayName("search_content without a query is an invalid-argument error") + void searchWithoutQueryThrows() { + assertThatThrownBy(() -> provider.searchContentLogic(args())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + @DisplayName("search_content reports unavailable while bootstrapping") + void searchUnavailableWhileBootstrapping() { + readiness.markBootstrappingStarted(); + + assertThatThrownBy(() -> provider.searchContentLogic(args("query", "wallet"))) + .isInstanceOf(McpUnavailableException.class); + } + + @Test + @DisplayName("list_posts passes the since filter and reports unavailable while bootstrapping") + void listPostsFilterAndAvailability() { + provider.listPostsLogic(args("since", "2026-01-01", "source", "hiero")); + assertThat(searchService.lastFilters.since()).isEqualTo("2026-01-01"); + assertThat(searchService.lastFilters.source()).isEqualTo("hiero"); + + readiness.markBootstrappingStarted(); + assertThatThrownBy(() -> provider.listPostsLogic(args())) + .isInstanceOf(McpUnavailableException.class); + } + + @Test + @DisplayName("get_post returns the full document by url") + void getPostByUrl() { + ContentDocument document = document(); + searchService.document = Optional.of(document); + + Object result = provider.getPostLogic(args("url", "https://ex.com/a")); + + assertThat(result).isEqualTo(document); + assertThat(searchService.lastUrl).isEqualTo("https://ex.com/a"); + } + + @Test + @DisplayName("get_post with neither url nor id is an invalid-argument error") + void getPostWithoutArgsThrows() { + assertThatThrownBy(() -> provider.getPostLogic(args())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + @DisplayName("get_post for an unknown post is a not-found error") + void getPostUnknownIsNotFound() { + searchService.document = Optional.empty(); + + assertThatThrownBy(() -> provider.getPostLogic(args("id", "oe_missing"))) + .isInstanceOf(NoSuchElementException.class); + } + + @Test + @DisplayName("list_categories delegates with the source/locale filter") + void listCategoriesDelegates() { + searchService.categories = List.of(new CategoryCount("ai", 3)); + + Object result = provider.listCategoriesLogic(args("source", "hiero")); + + assertThat(result).isEqualTo(List.of(new CategoryCount("ai", 3))); + assertThat(searchService.lastSource).isEqualTo("hiero"); + } + + private void searchContentResult() { + searchService.hits = new SearchHits(List.of(new SearchHit("T", "u", "2026-01-01", "snip", 0.9)), 1); + } + + private static ContentDocument document() { + return new ContentDocument("oe_a", "open-elements", "en", "https://ex.com/a", + "T", "E", "body", "a", List.of("ai"), "2026-01-01", "2026-01-01", null); + } + + /** Capturing {@link ContentSearchService} test double recording arguments and returning canned data. */ + private static final class CapturingSearchService extends ContentSearchService { + private String lastQuery; + private ContentFilters lastFilters; + private int lastPage; + private int lastSize; + private String lastUrl; + private String lastId; + private String lastSource; + private String lastLocale; + private SearchHits hits = SearchHits.empty(); + private List categories = List.of(); + private Optional document = Optional.empty(); + + CapturingSearchService() { + super(new MeilisearchClient(props(), new ObjectMapper()), props()); + } + + private static MeilisearchProperties props() { + return new MeilisearchProperties("http://localhost:7700", "", "content_", Duration.ofSeconds(10)); + } + + @Override + public SearchHits search(String query, ContentFilters filters, int page, int size) { + this.lastQuery = query; + this.lastFilters = filters; + this.lastPage = page; + this.lastSize = size; + return hits; + } + + @Override + public SearchHits listPosts(ContentFilters filters, int page, int size) { + this.lastFilters = filters; + this.lastPage = page; + this.lastSize = size; + return hits; + } + + @Override + public Optional getByUrlOrId(String url, String id) { + this.lastUrl = url; + this.lastId = id; + return document; + } + + @Override + public List categoryFacets(String source, String locale) { + this.lastSource = source; + this.lastLocale = locale; + return categories; + } + } +}