-
Notifications
You must be signed in to change notification settings - Fork 13
feat(mcp): add Tool Registry Service for agent tool credentials #208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: eng-95/component-sdk-tool-mode
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8251468d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| const encryptionMaterial = JSON.parse(tool.encryptedCredentials); | ||
| const decrypted = await this.encryption.decrypt(encryptionMaterial); | ||
| return JSON.parse(decrypted); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle non-JSON auth tokens in getToolCredentials
registerRemoteMcp encrypts a raw authToken string, but getToolCredentials always JSON.parses the decrypted payload. When the tool is a remote MCP (auth token is a plain string, not JSON), JSON.parse(decrypted) throws and the method returns null, so any consumer trying to fetch credentials for remote MCPs will never receive the token. This breaks authenticated remote MCP calls whenever authToken is not JSON-encoded.
Useful? React with 👍 / 👎.
7e4d294 to
e49f0c8
Compare
b69e898 to
15d7d28
Compare
4f4cee6 to
f09bb4d
Compare
15d7d28 to
3670de1
Compare
f3da236 to
0660362
Compare
d7263bb to
908d508
Compare
ENG-96
- Create ToolRegistryService with Redis-backed storage
- Implement registerComponentTool, registerRemoteMcp, registerLocalMcp
- Implement getToolsForRun, getTool, getToolByName, getToolCredentials
- Implement areAllToolsReady for agent readiness check
- Implement cleanupRun for workflow completion cleanup
- Encrypt credentials using existing SecretsEncryptionService
- Redis key pattern: mcp:run:{runId}:tools (Hash, TTL 1hr)
- Add McpModule to app imports
- Add comprehensive tests (8 tests passing)
Note: Temporal activities (registerToolActivity, waitForToolsActivity, etc.)
will be added in a follow-up as they reside in the worker package.
Signed-off-by: betterclever <paliwal.pranjal83@gmail.com>
Signed-off-by: betterclever <paliwal.pranjal83@gmail.com>
- Update registerRemoteMcp to store authToken as JSON object for consistency - Add fallback in getToolCredentials to handle legacy raw string tokens - Add test case for remote MCP credentials Signed-off-by: betterclever <paliwal.pranjal83@gmail.com>
Signed-off-by: betterclever <paliwal.pranjal83@gmail.com>
908d508 to
f82a05d
Compare
Summary
Redis-backed registry for storing tool metadata and credentials during workflow runs. This bridges the gap between Temporal workflows (where credentials are resolved) and the MCP gateway (where agents call tools).
Stacked PR
Changes
New: `backend/src/mcp/` module
ToolRegistryService:
Redis Key Pattern
```
mcp:run:{runId}:tools (Hash, TTL 1hr)
```
Security
Tests
Note
Temporal activities (`registerToolActivity`, `waitForToolsActivity`, etc.) will be added in a follow-up as they reside in the worker package.
Linear Issue
Closes ENG-96