feat(mcp-chat): added shared libraires and extension points#8370
feat(mcp-chat): added shared libraires and extension points#8370fjudith wants to merge 34 commits into
Conversation
2ca5d06 to
5620679
Compare
Unexpected ChangesetsThe following changeset(s) reference packages that have not been changed in this PR:
Note that only changes that affect the published package require changesets, for example changes to tests and storybook stories do not require changesets. Changed Packages
|
f6bd506 to
23e0387
Compare
fdf8074 to
783ad0e
Compare
|
@Lucifergene the PR is ready for review |
|
This is a great work @fjudith |
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
…ckage Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
… package Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
see: comments backstage#8196 Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 56 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
workspaces/mcp-chat/plugins/mcp-chat-common/src/types.ts:654
ConversationRowis described as an internal DB row shape, but it’s tagged@publicwhich exposes it as part of the common package public API (and locks its schema for consumers). Either revert the tag to@internal(preferred for DB row types), or update the documentation to reflect that it’s intended for external consumption and stability guarantees apply.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Florian JUDITH <florian.judith.b@gmail.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
doc: https://backstage.io/docs/backend-system/architecture/extension-points/#registering-an-extension-point Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
Signed-off-by: Florian JUDITH <florian.judith@alithya.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 56 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
workspaces/mcp-chat/plugins/mcp-chat-backend/src/services/MCPClientServiceImpl.ts:95
MCPClientServiceImplinitializes the LLM provider in the constructor (this.llmProvider = this.initializeLLMProvider();). Because provider modules register via the extension point during backend initialization, there’s no guarantee the extension-point providers have been registered before this constructor runs, which can cause the built-in factory path to be taken (or even throw) and the extension provider to be ignored for the lifetime of the service. Consider deferring LLM provider resolution until after initialization (e.g., lazily on first use), or wiring initialization so the plugin waits for provider registration before constructingMCPClientServiceImpl.
constructor(options: Options) {
this.logger = options.logger;
this.config = options.config;
this.extensionProviders = options.extensionProviders ?? new Map();
this.toolCallTimeout =
this.config.getOptionalNumber('mcpChat.toolCallTimeout') ??
DEFAULT_MCP_TOOL_CALL_TIMEOUT_MS;
this.llmProvider = this.initializeLLMProvider();
this.mcpServers = this.initializeMCPServers();
this.systemPrompt =
this.config.getOptionalString('mcpChat.systemPrompt') ||
"You are a helpful assistant. When using tools, provide a clear, readable summary of the results rather than showing raw data. Focus on answering the user's question with the information gathered.";
}
workspaces/mcp-chat/plugins/mcp-chat-common/src/types.ts:654
- The doc comment says
ConversationRowis “Used internally for database operations”, but the TSDoc tag was changed to@public. If this type is meant to remain internal implementation detail, it should stay@internal(or be excluded from the public barrel exports). If it’s intentionally public now, please update the comment to reflect that it’s part of the supported API surface.
| private initializeLLMProvider(): LLMProvider { | ||
| try { | ||
| const providerConfig = getConfig(this.config); | ||
|
|
||
| // Check if an extension-point provider was registered for this type | ||
| const extensionProvider = this.extensionProviders.get( | ||
| providerConfig.type, | ||
| ); | ||
| if (extensionProvider) { | ||
| this.logger.info( | ||
| `Using extension-point LLM Provider: ${providerConfig.type}, Model: ${providerConfig.model}`, | ||
| ); | ||
| return extensionProvider; | ||
| } | ||
|
|
| // Check if an extension-point provider was registered for this type | ||
| const extensionProvider = this.extensionProviders.get( | ||
| providerConfig.type, | ||
| ); | ||
| if (extensionProvider) { | ||
| this.logger.info( | ||
| `Using extension-point LLM Provider: ${providerConfig.type}, Model: ${providerConfig.model}`, | ||
| ); | ||
| return extensionProvider; | ||
| } |
| const providers = new Map<string, LLMProvider>(); | ||
|
|
||
| env.registerExtensionPoint(llmProviderExtensionPoint, { | ||
| registerProvider(type, provider) { | ||
| if (providers.has(type)) { | ||
| // As documented in the interface: replace and warn | ||
| console.warn( | ||
| `LLM provider with type '${type}' is already registered and will be replaced`, | ||
| ); | ||
| } | ||
| providers.set(type, provider); | ||
| }, |
| /** Maximum number of tokens to generate (default: 1000 for OpenAI-compatible, 4096 for Claude, 8192 for Gemini) */ | ||
| maxTokens?: number; | ||
| /** Temperature for response randomness, between 0 and 1 (default: 0.7) */ | ||
| temperature?: number; |
Hey, I just made a Pull Request!
Starts the Implementation of #7976 following the adr011 and (superseeds #8196).
✔️ Checklist
Signed-off-byline in the message. (more info)