Repository navigation
[MCP] refactor(samples): use configureMcpCatalog and resolve @a2ui/catalog-mcp from dist - #3187
josemontespg wants to merge 1 commit into
Conversation
…mcp from dist Move the two community MCP sample clients to the v1.0 entry point of @a2ui/catalog-mcp: register CallMcpToolImplementation (the filesystem sample next to its own data functions) and configure the host with configureMcpCatalog. Resolve @a2ui/catalog-mcp, @a2ui/lit and @a2ui/web_core from the monorepo dist/ instead of compiling the catalog from source against the npm packages; the community CI workflow builds those packages at the repository root first (TODO #1698).
There was a problem hiding this comment.
Code Review
This pull request updates the MCP client and catalog configuration across the community samples, transitioning from using createMcpCatalogFunctions to CallMcpToolImplementation and configureMcpCatalog. It also updates Catalog constructors to include a version parameter, modifies getClientCapabilities calls, and refactors monorepo aliases in tsconfig.json and vite.config.ts to use built packages. Feedback on these changes highlights two main issues: first, removing @a2ui/lit leaves the filesystem client without an explicit dependency on lit (which is still directly used); second, getMcpClientForTool should include a defensive check to throw an error if this.mcpClient is not yet initialized, preventing potential runtime errors.
| "lint:fix": "eslint . --fix" | ||
| }, | ||
| "dependencies": { | ||
| "@a2ui/lit": "^0.10.0", |
There was a problem hiding this comment.
With the removal of @a2ui/lit from the dependencies, this project no longer has a direct dependency on lit, even though A2uiFilesystemApp extends LitElement and uses other Lit features. To prevent potential resolution issues (especially in strict package managers), lit should be explicitly declared as a dependency, similar to how it is done in the recipe client.
| ); | ||
| ]); | ||
| configureMcpCatalog({ | ||
| getMcpClientForTool: () => this.mcpClient, |
There was a problem hiding this comment.
If this.mcpClient is not yet initialized or connection fails, getMcpClientForTool will return undefined. This can lead to runtime TypeErrors when a tool is invoked. Consider adding a defensive check to throw a descriptive error if the client is not initialized.
getMcpClientForTool: () => {
if (!this.mcpClient) {
throw new Error('MCP client is not initialized');
}
return this.mcpClient;
},|
Folded into #3186: the community samples are the in-repo consumers of the API change there, and the stack was red in between (Community Code CI compiled the samples against an entry point that no longer exported |
Summary
Moves the two community MCP sample clients to the v1.0 entry point of
@a2ui/catalog-mcpintroduced in the parent PR, and resolves@a2ui/catalog-mcp,@a2ui/litand@a2ui/web_corefrom the monorepodist/instead of compiling the catalog from source against the npm packages.Changes
samples/community/mcp/a2ui-over-mcp-{filesystem,recipe}/client/app.ts: registerCallMcpToolImplementation(the filesystem sample next to its own data functions) and configure the host withconfigureMcpCatalog.vite.config.ts,tsconfig.json,package.jsonof both clients: resolve the three packages from the repositorydist/(TODO Clean up folder samples/community. #1698); the sample tests follow the monorepoCatalog/DataContextconstructors..github/workflows/community_code.yml: build@a2ui/catalog-mcpand@a2ui/litat the repository root before the community checks.Verification
yarn testin both sample clients against a fresh@a2ui/catalog-mcpbuild (64 and 11 tests).python3 scripts/fix_licenses.py --check.Related to #3157