Add Create Domain MCP Tool #26609#27281
Add Create Domain MCP Tool #26609#27281aniruddhaadak80 wants to merge 7 commits intoopen-metadata:mainfrom
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR primarily adds a new MCP tool (create_domain) to the OpenMetadata MCP server so agents can create Domain entities via Model Context Protocol. It also includes several unrelated fixes across UI lineage popover styling and backend tag/query handling.
Changes:
- Add
create_domainto MCP tool definitions, dispatch, and implementCreateDomainTool. - Update user listing DAO queries to pass
ListFilterquery params into SQL bindings. - Minor UI lineage popover button styling/formatting and table data model tag merge adjustments.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityLineage/CanvasButtonPopover.component.tsx | Minor TSX formatting + button style tweaks for the lineage popover wrapper. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java | Adjust tag merging behavior when adding a DataModel (table + column tags). |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java | Pass filter.getQueryParams() into listCount/listBefore/listAfter query methods via @BindMap. |
| openmetadata-mcp/src/main/resources/json/data/mcp/tools.json | Add the create_domain tool schema (and reformat some existing JSON sections). |
| openmetadata-mcp/src/main/java/org/openmetadata/mcp/tools/DefaultToolContext.java | Route create_domain tool calls to CreateDomainTool. |
| openmetadata-mcp/src/main/java/org/openmetadata/mcp/tools/CreateDomainTool.java | New MCP tool implementation for Domain creation (currently contains blocking compile/runtime issues). |
| @" | ||
| package org.openmetadata.mcp.tools; | ||
|
|
There was a problem hiding this comment.
This new Java file includes stray @" at the top and "@ at the end (looks like a copied PowerShell here-string delimiter), which will make the class fail to compile. Remove these markers so the file starts with the package ... declaration and ends after the closing brace.
| CreateDomain createDomain = new CreateDomain(); | ||
| createDomain.setName(name); | ||
| createDomain.setDescription(description); | ||
|
|
||
| try { | ||
| createDomain.setDomainType(org.openmetadata.schema.type.DomainType.fromValue(domainType)); | ||
| } catch (Exception e) { | ||
| throw new IllegalArgumentException( | ||
| "Parameter 'domainType' has invalid value '" + domainType + "'. Valid values are: Aggregate, Source, Aligned, Consumer, other"); | ||
| } |
There was a problem hiding this comment.
createDomain.setDomainType(org.openmetadata.schema.type.DomainType.fromValue(domainType)) appears to be using the wrong enum type for CreateDomain. Elsewhere the API uses org.openmetadata.schema.api.domains.CreateDomain.DomainType (e.g., integration tests), so this is likely a compilation error or runtime mismatch. Also, the error message lists values that don’t match the actual enum names (e.g., CONSUMER_ALIGNED). Please use the CreateDomain DomainType enum and derive the "valid values" list from the enum to avoid drift.
| }, | ||
| "domainType": { | ||
| "type": "string", | ||
| "description": "Type" |
There was a problem hiding this comment.
The create_domain tool schema leaves domainType as a free-form string. Since the server code validates against an enum, the MCP schema should publish the allowed values (an enum) so agents can reliably provide valid inputs and avoid trial-and-error failures.
| "description": "Type" | |
| "description": "Type", | |
| "enum": [ | |
| "Source-aligned", | |
| "Consumer-aligned", | |
| "Aggregate", | |
| "Functional" | |
| ] |
| return EntityDAO.super.listCount(filter); | ||
| } | ||
| return listCount( | ||
| getTableName(), mySqlCondition, postgresCondition, team, Relationship.HAS.ordinal()); | ||
| getTableName(), mySqlCondition, postgresCondition, team, Relationship.HAS.ordinal(), filter.getQueryParams()); | ||
| } |
There was a problem hiding this comment.
Passing filter.getQueryParams() into @BindMap params will also bind entries like team (and potentially relation/cursor keys if ever present). Since this query already binds :team via @BindFQN("team"), the map binding can override the hashed/quoted value and break team filtering (or create hard-to-debug precedence issues). Consider passing a copy of the params map with overlapping keys removed (at least team) or using a prefixed bind map to avoid collisions.
| case "create_metric": | ||
| result = new CreateMetricTool().execute(authorizer, limits, securityContext, params); | ||
| break; | ||
| case "create_domain": | ||
| result = new CreateDomainTool().execute(authorizer, limits, securityContext, params); | ||
| break; |
There was a problem hiding this comment.
A new MCP tool is being added to the dispatch switch, but there is no corresponding unit test like the existing CreateMetricToolTest / GlossaryToolTest coverage. Please add a CreateDomainToolTest to validate mapping + repository calls (and prevent regressions in parameter validation).
| case "create_metric": | ||
| result = new CreateMetricTool().execute(authorizer, limits, securityContext, params); | ||
| break; | ||
| case "create_domain": | ||
| result = new CreateDomainTool().execute(authorizer, limits, securityContext, params); | ||
| break; |
There was a problem hiding this comment.
The PR description focuses on adding the create_domain MCP tool, but this diff also includes unrelated changes in the UI lineage popover and in table tag merging / user listing SQL bindings. If these aren’t required for the MCP tool, consider splitting them into separate PRs to keep the change isolated and easier to review/revert.
aniruddhaadak80
force-pushed
the
feat-mcp-create-domain
branch
from
3aab4d9 to
e407b26
Compare
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
aniruddhaadak80
force-pushed
the
feat-mcp-create-domain
branch
from
e407b26 to
856f0e9
Compare
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
| "experts": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "owners": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "tags": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| } |
There was a problem hiding this comment.
The new create_domain tool schema is missing description fields for experts, owners, and tags. Other MCP tool definitions (e.g. create_metric) document these array parameters, and the lack of descriptions makes the tool harder for agents to use correctly. Add clear descriptions for each of these properties (what values are expected, e.g. usernames/team names vs tag FQNs).
| @Slf4j | ||
| public class CreateDomainTool implements McpTool { | ||
|
|
||
| @Override | ||
| public Map<String, Object> execute( | ||
| Authorizer authorizer, CatalogSecurityContext securityContext, Map<String, Object> params) { | ||
| throw new UnsupportedOperationException("CreateDomainTool requires limit validation."); | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> execute( | ||
| Authorizer authorizer, | ||
| Limits limits, | ||
| CatalogSecurityContext securityContext, | ||
| Map<String, Object> params) { |
There was a problem hiding this comment.
There is no unit test for the newly added CreateDomainTool. Other MCP create tools (e.g. CreateMetricToolTest) verify that the tool calls Entity.getEntityRepository(...).prepareInternal(...) and createOrUpdate(...) with the mapped entity. Add a CreateDomainToolTest with similar mocking to cover the happy path and at least one validation failure (e.g. missing/invalid domainType).
|
Would it be possible to add the |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
mohityadav766
added
the
safe to test
| Limits limits, | ||
| CatalogSecurityContext securityContext, | ||
| Map<String, Object> params) { | ||
| Object nameRaw = params.get("name"); |
There was a problem hiding this comment.
you can rely on CreateDomain schema for this , instead of check individual fields @aniruddhaadak80
| "properties": { | ||
| "name": { | ||
| "type": "string", | ||
| "description": "Unique name" |
There was a problem hiding this comment.
Let's add proper descriptions here
| "type": "string", | ||
| "description": "Domain description" | ||
| }, | ||
| "domainType": { |
There was a problem hiding this comment.
for MCP it should be a clear description that LLM can use to undertand and send the created request
|
The Java checkstyle failed. Please run You can install the pre-commit hooks with |
|
The Java checkstyle failed. Please run You can install the pre-commit hooks with |
| "tags": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| } |
There was a problem hiding this comment.
The tags parameter is missing a description. Add one clarifying that values should be tag FQNs (e.g., Tier.Tier1) which will be applied as MANUAL classification tags.
| try { | ||
| createDomain.setDomainType(org.openmetadata.schema.type.DomainType.fromValue(domainType)); | ||
| } catch (Exception e) { | ||
| throw new IllegalArgumentException( | ||
| "Parameter 'domainType' has invalid value '" + domainType + "'. Valid values are: Aggregate, Source-aligned, Consumer-aligned"); |
There was a problem hiding this comment.
domainType parsing catches Exception and hard-codes the list of valid values. This can hide unexpected failures and will drift if the DomainType enum changes. Catch IllegalArgumentException from DomainType.fromValue(...) and build the valid-values list from DomainType.values() (or reuse a shared helper, as done in CreateMetricTool).
| if (!(parentRaw instanceof String)) { | ||
| throw new IllegalArgumentException( | ||
| "Parameter 'parent' must be a string. Received: " + parentRaw); | ||
| } | ||
| createDomain.setParent((String) parentRaw); |
There was a problem hiding this comment.
When parent is provided as an empty/blank string, it passes the instanceof String check and is set on CreateDomain, which later becomes an EntityReference with a blank FQN and is likely to fail resolution. Treat blank strings the same as missing: either reject with a clear IllegalArgumentException or ignore the field when blank.
| if (!(parentRaw instanceof String)) { | |
| throw new IllegalArgumentException( | |
| "Parameter 'parent' must be a string. Received: " + parentRaw); | |
| } | |
| createDomain.setParent((String) parentRaw); | |
| if (!(parentRaw instanceof String parent) || parent.isBlank()) { | |
| throw new IllegalArgumentException( | |
| "Parameter 'parent' must be a non-blank string when provided. Received: " | |
| + parentRaw); | |
| } | |
| createDomain.setParent(parent); |
|
|
||
| when(repo.createOrUpdate(isNull(), any(Domain.class), anyString(), isNull())) | ||
| .thenReturn(putResponse); |
There was a problem hiding this comment.
This test stubs repo.createOrUpdate(..., impersonatedBy) with isNull() for the 4th argument, but the tool reads it from ImpersonationContext (a thread-local). If another test sets impersonation and doesn’t clear it, this test can become flaky. Clear ImpersonationContext in @BeforeEach (and/or relax the stub to accept any value for impersonatedBy).
| "experts": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| }, |
There was a problem hiding this comment.
The experts parameter is missing a description. Other tools in this file provide per-parameter descriptions, and agents rely on them to supply correct inputs. Add a short description (e.g., list of OpenMetadata usernames/login names).
| "domainType": { | ||
| "type": "string", | ||
| "description": "Type", | ||
| "enum": ["Aggregate", "Source-aligned", "Consumer-aligned"] |
There was a problem hiding this comment.
In create_domain, the domainType enum is the only enum in this file still formatted inline. Consider formatting it as a multi-line array like the other enum definitions to keep the JSON consistently formatted (and reduce future diff noise).
| "enum": ["Aggregate", "Source-aligned", "Consumer-aligned"] | |
| "enum": [ | |
| "Aggregate", | |
| "Source-aligned", | |
| "Consumer-aligned" | |
| ] |
| "owners": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| }, |
There was a problem hiding this comment.
The owners parameter is missing a description. Add one clarifying the expected inputs (e.g., OpenMetadata usernames or team names, which the backend will resolve to entity references).
aniruddhaadak80
requested review from
akash-jain-10,
harshach and
tutte
as code owners
Code Review ✅ Approved 7 resolved / 7 findingsImplements the Create Domain MCP tool, resolving compilation issues from PowerShell syntax, correcting enum values, and adding comprehensive validation tests. All identified structural and configuration findings have been addressed. ✅ 7 resolved✅ Bug: File contains PowerShell here-string delimiters that break compilation
✅ Bug: Error message lists incorrect DomainType enum values
✅ Quality: tools.json missing domainType enum constraint
✅ Quality: Missing newline at end of tools.json
✅ Quality: Test only asserts non-null; doesn't verify result content
...and 2 more resolved from earlier reviews OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
| if (!(parentRaw instanceof String)) { | ||
| throw new IllegalArgumentException( | ||
| "Parameter 'parent' must be a string. Received: " + parentRaw); | ||
| } | ||
| createDomain.setParent((String) parentRaw); |
There was a problem hiding this comment.
parent accepts an empty string (""), which will be treated as a non-null FQN by DomainMapper and routed into Entity.getEntityReferenceByName(...) with an empty name. That path is expected to throw (entity lookup by blank FQN), turning a user input issue into a 500. Treat blank parent as missing (ignore it) or validate that when provided it must be a non-blank string.
| if (!(parentRaw instanceof String)) { | |
| throw new IllegalArgumentException( | |
| "Parameter 'parent' must be a string. Received: " + parentRaw); | |
| } | |
| createDomain.setParent((String) parentRaw); | |
| if (!(parentRaw instanceof String parent)) { | |
| throw new IllegalArgumentException( | |
| "Parameter 'parent' must be a string. Received: " + parentRaw); | |
| } | |
| if (!parent.isBlank()) { | |
| createDomain.setParent(parent); | |
| } |
| @BeforeEach | ||
| void setUp() { | ||
| authorizer = mock(Authorizer.class); | ||
| limits = mock(Limits.class); | ||
| securityContext = mock(CatalogSecurityContext.class); | ||
|
|
||
| Principal mockPrincipal = mock(Principal.class); | ||
| when(mockPrincipal.getName()).thenReturn("test-user"); | ||
| when(securityContext.getUserPrincipal()).thenReturn(mockPrincipal); | ||
| } |
There was a problem hiding this comment.
This test stubs repo.createOrUpdate(..., impersonatedBy) assuming ImpersonationContext.getImpersonatedBy() is null, but it never clears the ThreadLocal. If another test leaves ImpersonationContext set, this test can become order-dependent/flaky. Add an @AfterEach that calls ImpersonationContext.clear() (and/or set it explicitly within the test).
Fixes #26609. Added the CreateDomainTool to OpenMetadata's MCP server. This allows AI agents to create explicit domain boundaries and data product assignments via Model Context Protocol, checking off the 'Domain & Data Product management' epic.