Skip to content

Conversation

@betterclever
Copy link
Contributor

Summary

Consolidated MCP tool‑mode work end‑to‑end (component metadata → workflow/tool registration → MCP gateway → agent execution → UI + AWS MCP servers) into a single reviewable branch.

Scope (Linear)

  • ENG-95: Component SDK tool‑mode metadata + schema helpers
  • ENG-96: Tool Registry (Redis‑backed credentials + tool readiness)
  • ENG-97: Workflow tool‑mode handling + MCP server node wiring
  • ENG-98: MCP Gateway JSON‑RPC endpoint + tool listing/execution
  • ENG-132: Tool→agent binding + scoped discovery via connected node IDs
  • ENG-100: OpenCode agent component + MCP client integration
  • ENG-101/102: Tool‑mode UI + agent orchestration + component updates
  • ENG-103: MCP stdio proxy + AWS MCP servers + runtime + UI polish

What changed (high level)

  • Added tool‑mode metadata and tool schema helpers to component‑sdk, used across worker/backend.
  • Implemented Redis‑backed Tool Registry and MCP Gateway (auth, tool discovery, execution via Temporal).
  • Added workflow/compiler changes to register tools instead of executing tool‑mode nodes.
  • Enabled tool scoping via graph connections (connectedToolNodeIds) for multi‑agent isolation.
  • Built OpenCode agent component and MCP tool integration.
  • Delivered UI for tool mode + MCP server nodes + port/contract updates.
  • Added MCP stdio proxy + AWS MCP server components/images.

Testing

  • E2E tests added for MCP gateway, tool mode, and OpenCode; not re‑run in this consolidation.

Stacked‑diff consolidation

This PR consolidates work previously stacked across:
#208 (ENG‑96), #212 (ENG‑97), #213 (ENG‑98), #230 (ENG‑132), #236 (ENG‑100), #237 (ENG‑101/102), #239 (ENG‑103).

Refs: ENG-95, ENG-96, ENG-97, ENG-98, ENG-100, ENG-101, ENG-102, ENG-103, ENG-132

…ents

ENG-95

- Add PortBindingType and bindingType to ComponentPortMetadata
- Add AgentToolConfig and agentTool to ComponentUiMetadata
- Create tool-helpers.ts with isAgentCallable, getToolSchema, getCredentialInputIds, getActionInputIds, getToolName, getToolDescription, getToolMetadata
- Add comprehensive tests for all helper functions (14 tests)

Signed-off-by: betterclever <[email protected]>
Previously, json/any ports were incorrectly forced to type: 'object' in
MCP tool schemas. This fix uses Zod's built-in toJSONSchema() method
which correctly handles all types:

- z.any() → {} (empty schema = any JSON value)
- z.union([...]) → { anyOf: [...] }
- z.enum([...]) → { type: 'string', enum: [...] }
- z.literal('X') → { type: 'string', const: 'X' }
- z.record(...) → { type: 'object', additionalProperties: {...} }

Changes:
- Use @modelcontextprotocol/sdk for official Tool types
- ToolInputSchema now derives from Tool['inputSchema']
- Simplified code by reusing existing getActionInputIds()
- Removed redundant helper functions

Fixes P2 bug in PR #207

Signed-off-by: betterclever <[email protected]>
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 <[email protected]>
- 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 <[email protected]>
- 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 <[email protected]>
… MCP proxying

- Replace deprecated SSEClientTransport with StreamableHTTPClientTransport
- Fix lint errors (trailing whitespace in constructor and emitProgress)
- Gateway currently executes components inline (to be refactored to Temporal)

Signed-off-by: betterclever <[email protected]>
- Add executeToolCallSignal and toolCallCompletedSignal for MCP tool calls
- Add getToolCallResult query for polling tool execution results
- Refactor callComponentTool to signal workflow instead of inline execution
- Add queryWorkflow method to TemporalService
- Tool calls now execute on worker with full Docker/secrets/storage support

Signed-off-by: betterclever <[email protected]>
…ation

- Refactor component tool execution to run on Temporal workers via signals/queries
- Implement validation for workflow run access and organization ownership
- Add comprehensive telemetry: log tool execution (STARTED, COMPLETED, FAILED) to trace repository
- implement robust external MCP proxying with 30s timeouts and exponential backoff retries
- Add support for tool filtering via allowedTools header
- Add E2E test for MCP gateway tool discovery and execution

Signed-off-by: Antigravity <[email protected]>
Signed-off-by: betterclever <[email protected]>
- Extract X-Run-Id and X-Allowed-Tools headers in McpGatewayController
- Pass organizationId and allowedTools to McpGatewayService
- Add basic protocol version validation
- Fix type casting for MCP transport request handling

Signed-off-by: betterclever <[email protected]>
…eway

- Add McpAuthService to manage short-lived, run-bounded session tokens
- Implement McpAuthGuard for RFC 6750 (Bearer) compliance and AuthInfo injection
- Refactor McpGatewayController to use native MCP AuthInfo instead of internal AuthContext
- Add internal endpoint /internal/mcp/generate-token for session token issuance
- Update E2E tests to validate the complete secure handshake and tool execution flow
- Fix type safety issues in MCP transport integration

Signed-off-by: Antigravity <[email protected]>
Signed-off-by: betterclever <[email protected]>
…script harness

- Ensure component 'parameters' are passed through tool registration and execution signals
- Correctly map agent 'arguments' to component 'inputs' in runComponentActivity
- Fix race condition in logic-script harness by ensuring output directory exists before write
- Update E2E gateway test to reflect correct registration and execution pattern
- Clean up debug logs and resolve linting errors across gateway and worker

Signed-off-by: betterclever <[email protected]>
Detailed plan for enabling graph-based tool→agent binding:
- Phase 1: Compiler tracks tool→agent edges
- Phase 2: Runtime passes connectedToolNodeIds to agent
- Phase 3: Agent queries MCP Gateway for tools
- Phase 4: Gateway filters tools by nodeIds
- Phase 5: E2E tests

Linear-issue: ENG-132
Signed-off-by: betterclever <[email protected]>
Phase 1 of ENG-132:
- Added connectedToolNodeIds to WorkflowNodeMetadata (backend/worker)
- Added tools input port to AI agent component
- Included connectedToolNodeIds in RunComponentActivityInput metadata

Linear-issue: ENG-132
Signed-off-by: betterclever <[email protected]>
Phase 2 of ENG-132:
- Modified compiler to collect connectedToolNodeIds from graph edges
- Updated validator to allow multiple edges to 'tools' port
- Virtualized 'tools' output port for nodes in tool mode
- Updated DTO schemas to support tool mode metadata
- Added unit test to verify tool->agent binding

Linear-issue: ENG-132
Signed-off-by: betterclever <[email protected]>
Phase 3 of ENG-132:
- Updated shipsecWorkflowRun to extract connectedToolNodeIds from node metadata
- Included connectedToolNodeIds in activity metadata for agent discovery
- Synchronized worker types for workflow execution

Linear-issue: ENG-132
Signed-off-by: betterclever <[email protected]>
Phase 4 of ENG-132:
- Added nodeIds filter to ToolRegistryService.getToolsForRun
- Updated McpGatewayService to support tool scoping by nodeId
- Included allowedNodeIds in session token and metadata
- Updated InternalMcpController to allow scoped token generation

Linear-issue: ENG-132
Signed-off-by: betterclever <[email protected]>
Phase 3 of ENG-132:
- Updated ExecutionContextMetadata to include connectedToolNodeIds and organizationId
- Modified runComponentActivity to inject metadata into the execution context
- Ensured organizationId is available for agent tool discovery

Linear-issue: ENG-132
Signed-off-by: betterclever <[email protected]>
Phase 5 of ENG-132:
- Added McpGatewayClient support to AI agent component
- Implemented runtime tool discovery via MCP Gateway
- Scoped tool discovery using connectedToolNodeIds
- Integrated discovered tools with agent reasoning loop
- Added session token generation helper

Linear-issue: ENG-132
Signed-off-by: betterclever <[email protected]>
- Add ApiKeysModule import to McpModule to fix DI issues
- Cache gateway servers/transports by runId + allowedNodeIds for agent isolation
- Implement buildMcpToolSchema to convert JSON Schema to Zod format
- Fix MCP tool content handling (extract text from array results)
- Add proper imports for StreamableHTTPClientTransport and Client

Tests now pass:
- Agent can run with no tools
- Multiple agents have isolated tool sets based on graph connections

This enables workflow graphs to connect tool-mode nodes directly to agents,
with the agent automatically discovering and using only its connected tools.

Signed-off-by: betterclever <[email protected]>
- Explicitly type tool execution to return Promise<string>
- Add comprehensive fallback conversions for tool result content
- Handle all formats: arrays, strings, objects, primitives
- Update test to use more reliable httpbin.org endpoint

Tool discovery is working correctly (tests 1 & 3 pass).
Test 2 fails due to tool execution issues, not discovery.

Signed-off-by: betterclever <[email protected]>
- Create DebugLogger utility for structured JSON logging to /tmp/shipsec-debug/
- Separate heartbeat logs to dedicated file (not printed to console)
- Add view-debug-logs.ts script for easy log viewing/filtering
- Convert agent gateway functions to use DebugLogger
- Remove verbose console heartbeat logs (15-second polling logs)
- Add getRecentLogs, getLogsByContext, getLogsByLevel utilities

Benefits:
- Cleaner console output (no heartbeat spam)
- All debug context centralized in one place
- Easy filtering by context, level, or search terms
- Structured JSON format for programmatic access

Debug logs now available at: /tmp/shipsec-debug/worker.log
Usage: bun scripts/view-debug-logs.ts [filter] [line-count]

Signed-off-by: betterclever <[email protected]>
- Log tool invocation arguments to debug-logger
- Log MCP result type and content before conversion
- Log final tool execution result with preview
- Log errors with stack traces for debugging

Root cause identified in Test 2: MCP protocol timeout on external endpoint
The tool is called successfully, gateway discovery works,
but the MCP callTool() times out waiting for the HTTP endpoint response.
This is NOT a tool discovery or integration issue - it's endpoint latency.

Signed-off-by: betterclever <[email protected]>
Signed-off-by: betterclever <[email protected]>
betterclever and others added 23 commits January 30, 2026 14:13
- Add zai-coding-plan to LLMProviderSchema with apiKey and modelId support
- Fix OpenCode component to use proper model format (provider/modelId)
- Configure Z.AI provider with apiKey in provider.options
- Fix MCP server config to use type: "remote" instead of transport: "http"
- Remove unused env var API key handling in favor of provider config

Co-Authored-By: Claude <[email protected]>
Signed-off-by: betterclever <[email protected]>
- Use sh -c with properly quoted prompt string to handle multi-word prompts
- Escape single quotes in prompt to prevent shell injection
- Add current-state.md documenting investigation and findings
- Add opencode E2E test

Co-Authored-By: Claude <[email protected]>
Signed-off-by: betterclever <[email protected]>
…er script

- Remove --quiet flag (doesn't exist in opencode 1.1.34), use --log-level ERROR
- Use wrapper script approach to handle prompt file reading inside container
- Set entrypoint to /bin/sh to override default opencode entrypoint
- Fix test assertions to check outputSummary.report instead of output.report
- Update current-state.md with resolution details

E2E tests now passing: 2 pass, 0 fail

Co-Authored-By: Claude <[email protected]>
Signed-off-by: betterclever <[email protected]>
…ided

When a custom systemPrompt is provided, the task was not being included
because the {{TASK}} placeholder only exists in the default template.

Now the task is always appended to ensure OpenCode receives the full prompt.

Co-Authored-By: Claude <[email protected]>
Signed-off-by: betterclever <[email protected]>
- Add optional baseUrl and headers properties to zai-coding-plan provider
- Add zai-coding-plan to ModelProvider type in ai-agent.ts
- This fixes TypeScript build errors when using the new provider

Co-Authored-By: Claude <[email protected]>
Signed-off-by: betterclever <[email protected]>
… d.ts files

- Remove emitDeclarationOnly: true from worker/tsconfig.json
- This allows TypeScript to emit both .js and .d.ts files
- Fixes backend typecheck errors when importing from @shipsec/studio-worker/workflows
- The worker still uses source files directly via bun, so .js files don't interfere

Co-Authored-By: Claude <[email protected]>
Signed-off-by: betterclever <[email protected]>
- Rename unused 'error' to '_error' in catch block to satisfy ESLint
- Apply code formatting from linter to opencode.ts

Co-Authored-By: Claude <[email protected]>
Signed-off-by: betterclever <[email protected]>
Signed-off-by: betterclever <[email protected]>
feat: MCP AWS servers and proxy (ENG-103)

Consolidating the work in 1 PR from the stack
…nt-ui

feat: tool mode UI + agent orchestration (ENG-101/102)

Consolidating the work in one PR
feat: OpenCode agent component (ENG-100)

Consolidating the work in final PR
feat: implement tool-mode orchestration (ENG-132)

Consolidating
feat(mcp): implement MCP Gateway for internal and external tools

Consolidating
feat(dsl): implement workflow tool mode handling and MCP server node
feat(mcp): add Tool Registry Service for agent tool credentials
@betterclever betterclever changed the title feat: MCP tool mode end-to-end (ENG-95/96/97/98/100/101/102/103/132) feat: tool mode + OpenCode agent + MCP gateway Jan 30, 2026
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: 7d7836bb57

ℹ️ 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 +196 to +200
// Merge credentials (pre-bound) with agent-provided arguments
inputs: {
...(request.credentials ?? {}),
...request.arguments,
},

Choose a reason for hiding this comment

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

P1 Badge Prevent tool calls from overriding bound credentials

The tool-call handler merges request.credentials and request.arguments with arguments taking precedence, so a caller can override pre‑bound credential inputs (e.g. send apiKey in arguments) even though those ports are meant to be fixed by the workflow. Because callComponentTool does not filter incoming args to action inputs, a malicious or buggy MCP client can inject credential fields and bypass the intended binding, leading to unauthorized or unexpected access. Consider filtering arguments to getActionInputIds() and/or giving credentials priority over arguments before executing the component.

Useful? React with 👍 / 👎.

Comment on lines +558 to +562
async cleanupRun(runId: string) {
const server = this.servers.get(runId);
if (server) {
await server.close();
this.servers.delete(runId);

Choose a reason for hiding this comment

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

P2 Badge Close scoped MCP servers on cleanup

getServerForRun caches servers under a key that includes allowedNodeIds (e.g. runId:node1,node2), but cleanupRun only looks up this.servers.get(runId). As soon as you mint scoped tokens (which is the default for tool‑mode agents), those servers are never closed and remain in the map, causing resource leaks and stale tool registrations across long‑running runs. Cleanup should iterate cache keys with the runId prefix (like refreshServersForRun) or accept the cacheKey explicitly.

Useful? React with 👍 / 👎.

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