Skip to content

Conversation

@betterclever
Copy link
Contributor

Summary

  • DSL Evolution: Extended WorkflowNodeMetadata to support mode ('normal' | 'tool') and toolConfig.
  • Compiler Upgrades: Updated the workflow compiler to be type-safe and correctly populate tool metadata from node configurations.
  • MCP Server Support: Added a new core.mcp.server component (Docker-based) to allow workflows to start and manage external tool providers.
  • Architecture Alignment: Fully integrated with the Zod-first SDK and the new Tool Registry infrastructure.

Stacked on: #208 (ENG-96)

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 7074d5305c

ℹ️ 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".

Comment on lines 69 to 73
async execute() {
// The Docker runner handles execution.
// At runtime, the workflow engine will resolve the dynamic endpoint.
return {
endpoint: 'http://localhost:8080', // Placeholder

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Invoke the docker runner for MCP server node

In the workflow runner, only component.execute is called; Docker execution is not triggered automatically (see run-component.activity.ts, which expects components to call runComponentWithRunner themselves). This execute implementation just returns a placeholder endpoint and never starts the container, so workflows that include this node will not actually launch an MCP server and will point to http://localhost:8080 even if nothing is running. Unless an external server is already up, the node will silently succeed but downstream tool calls will fail.

Useful? React with 👍 / 👎.

Comment on lines 175 to 145
let encryptedCredentials: string | undefined;
if (authToken) {
const encryptionMaterial = await this.encryption.encrypt(authToken);
encryptedCredentials = JSON.stringify(encryptionMaterial);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Store auth token as JSON or handle raw string

registerRemoteMcp encrypts a raw authToken string, but getToolCredentials later does JSON.parse on the decrypted payload. For non-JSON tokens, this throws and credentials are dropped, so remote MCP auth tokens become unretrievable. Either JSON.stringify the token before encryption or make getToolCredentials accept plain strings for remote-mcp tools.

Useful? React with 👍 / 👎.

@betterclever betterclever force-pushed the eng-97/workflow-tool-mode branch 2 times, most recently from f79081a to 190a3d9 Compare January 20, 2026 15:02
@betterclever betterclever changed the base branch from main to eng-96/tool-registry January 20, 2026 15:03
Signed-off-by: betterclever <paliwal.pranjal83@gmail.com>
- Add port mapping support to DockerRunner
- Refactor mcp-server component to use dynamic runner config and exposed ports
- Fix trace event mapping in E2E tests
- Add container cleanup to mcp-tool-mode E2E test
- Ensure local MCP registration uses actual containerId and endpoint
- Fix workflow definition validation by adding node positions in test

Signed-off-by: betterclever <paliwal.pranjal83@gmail.com>
@betterclever betterclever force-pushed the eng-97/workflow-tool-mode branch from 190a3d9 to ca18b5e Compare January 20, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants