Skip to content

Commit db06a4b

Browse files
authored
feat(connectors): generic MCP connector + schema-typed action outputs (#263)
* feat(connectors): generic MCP connector + schema-typed action outputs Add a generic MCP connector that spawns any stdio MCP server, discovers its tools via tools/list, and exposes them as first-class connector actions. Tool inputSchema/outputSchema flow through to the workflow editor so arg fields and {{steps.x.field}} autocomplete stay schema-driven — same shape as static connectors. Also: - GitHub actions now declare outputSchemas; createIssue / closeIssue / commentOnIssue return their API payloads as structuredOutput so downstream steps can reference html_url, number, state, etc. - Template resolver walks arbitrary dotted paths; object/array leaves JSON-stringify. - Deleting a condition node cascades the if/else subtree up to the join point instead of leaving two dangling parallel paths. - MCP icon reuses the ModelContextProtocol mark in both the connector badge and the workflow icon picker. * fix: export json-schema-utils subpath, update connector-form-panels mocks Two CI regressions from the previous commit: - Vite respects the package.json `exports` field for subpath resolution, so `@vornrun/shared/json-schema-utils` needed an entry next to the other subpaths. Without it, every server test that imported the scheduler (transitively: mcp.ts) failed module resolution. - connector-form-panels.test.tsx mocked `window.api.listConnectors`/ `listConnections` but not the new `listConnectionActions`; the form also now uses a textarea-based VariableAutocomplete for arg fields, so the element query needed to target `textarea`. * test: cover MCP connector, schema utils, walk-path resolver, condition cascade CI's diff-cover gate requires 80% on changed lines. New tests: - json-schema-utils: 100% - mcp-connector: tool→action mapping, structuredContent unwrap, MCP error surfacing, type coercion (number/bool/object), empty-string drop - mcp-clients: spawn config from filters, decrypted secretEnv merge, cache reuse, transport-onclose drops the entry, stopClient/stopAllClients - template-vars: walk nested paths, JSON-stringify object/array leaves, schema-derived buildStepGroups keys - workflow-helpers removeNode: condition cascade-delete + terminal case - ConnectorIcon: MCP brand-mark renders with both paths Patch coverage now 85% (above the 80% gate). * fix: address Copilot review comments - McpToolsPanel: guard discoveredTools cast with Array.isArray so a malformed filters payload can't crash .map at render. - InvokeToolDialog: key on tool.name so switching tools remounts with a fresh args stub + cleared result instead of leaking previous state. - WorkflowEditor prefetch: catch per-connection rejections so a single dead connection doesn't blank the autocomplete for healthy ones. - mcp.ts describe(): comment now points at connection:listActions, the endpoint the workflow editor actually consumes. * fix: address second batch of Copilot review comments - mcp-clients: build the child-process env from getSafeEnv() instead of process.env so the same secret allowlist that protects gh / agent detection / tailscale child invocations also covers MCP servers. GH / NPM / API tokens never reach an arbitrary MCP server now. - workflow-execution: arrays were sneaking into structuredOutput via `typeof === 'object'`; that breaks buildStepOutputsMap which spreads the value into a string-keyed map. Restrict to plain objects only. - CallConnectorActionNodeForm: render select fields with SelectPicker so enum-backed args (declared via JSON-Schema enum) keep their dropdown. text / textarea still flow through VariableAutocomplete. * chore: fix CardHeader formatting (broken on main, surfaced after rebase) * test: align card-context-menu test with #264 menu pruning #264 dropped Expand / Rename / Close from the ⋯ context menu since those moved to first-class header buttons. The test still asserted their presence; updating it to check for the actually-present entries. * fix: avoid stale action list on connection switch Connection-change effect now clears `actions` immediately and ignores a late response via a cancellation flag, so switching connections never flashes the previous connection's actions or fires onChange against a stale set. * fix: serialize concurrent MCP startups + preserve empty strings on string fields Two valid Copilot review concerns: - mcp-clients getOrStartClient: two concurrent callers hitting a cache miss could both spawn separate child processes before the first call ran clients.set(). Cache the in-flight Promise per connectionId so parallel calls share the same startup. - mcp coerceMcpArgs: dropping every empty string is too aggressive. Empty for a string field is a legitimate value the user may want to send; empty for a required non-string is something the MCP server's own validator should reject explicitly. Only drop empty for optional non-string types now.
1 parent 153ba67 commit db06a4b

31 files changed

Lines changed: 1939 additions & 135 deletions

packages/server/src/connectors/github.ts

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,40 @@ import { resolveGhPath, GhNotFoundError, getGhEnv } from './gh-cli'
1313

1414
const execFileAsync = promisify(execFile)
1515

16+
// JSON Schema subsets of the GitHub REST responses we pass through as
17+
// `ActionResult.output`. Only keys that are useful to reference from
18+
// downstream workflow steps are listed — the raw body still contains
19+
// everything, the schema just drives the variable-autocomplete UI and
20+
// documents the stable fields.
21+
const GITHUB_ISSUE_SCHEMA: Record<string, unknown> = {
22+
type: 'object',
23+
properties: {
24+
id: { type: 'number', description: 'Numeric id of the issue' },
25+
number: { type: 'number', description: 'Issue number within the repo' },
26+
html_url: { type: 'string', description: 'Issue URL to show in UIs' },
27+
url: { type: 'string', description: 'GitHub REST API URL for the issue' },
28+
title: { type: 'string' },
29+
body: { type: 'string' },
30+
state: { type: 'string', description: 'open or closed' },
31+
labels: { type: 'array' },
32+
assignees: { type: 'array' },
33+
created_at: { type: 'string' },
34+
updated_at: { type: 'string' },
35+
closed_at: { type: 'string' }
36+
}
37+
}
38+
39+
const GITHUB_COMMENT_SCHEMA: Record<string, unknown> = {
40+
type: 'object',
41+
properties: {
42+
id: { type: 'number' },
43+
html_url: { type: 'string', description: 'Link to the comment' },
44+
body: { type: 'string' },
45+
created_at: { type: 'string' },
46+
updated_at: { type: 'string' }
47+
}
48+
}
49+
1650
const TRANSIENT_CODES = new Set(['ETIMEDOUT', 'ENETDOWN', 'ENETUNREACH', 'ECONNRESET'])
1751

1852
function isTransientErr(err: unknown): boolean {
@@ -317,16 +351,18 @@ export const githubConnector: VornConnector = {
317351
case 'closeIssue': {
318352
const { number: issueNumber } = args
319353
if (!issueNumber) return { success: false, error: 'number is required' }
320-
await ghApi(`repos/${owner}/${repo}/issues/${issueNumber}`, 'PATCH', { state: 'closed' })
321-
return { success: true }
354+
const result = await ghApi(`repos/${owner}/${repo}/issues/${issueNumber}`, 'PATCH', {
355+
state: 'closed'
356+
})
357+
return { success: true, output: result as Record<string, unknown> }
322358
}
323359
case 'commentOnIssue': {
324360
const { number: num, body: comment } = args
325361
if (!num || !comment) return { success: false, error: 'number and body are required' }
326-
await ghApi(`repos/${owner}/${repo}/issues/${num}/comments`, 'POST', {
362+
const result = await ghApi(`repos/${owner}/${repo}/issues/${num}/comments`, 'POST', {
327363
body: String(comment)
328364
})
329-
return { success: true }
365+
return { success: true, output: result as Record<string, unknown> }
330366
}
331367
case 'syncTasks': {
332368
// This is handled by the sync engine at a higher level.
@@ -401,7 +437,8 @@ export const githubConnector: VornConnector = {
401437
},
402438
{ key: 'body', label: 'Body', type: 'textarea', supportsTemplates: true },
403439
{ key: 'labels', label: 'Labels', type: 'text', placeholder: 'bug,enhancement' }
404-
]
440+
],
441+
outputSchema: GITHUB_ISSUE_SCHEMA
405442
},
406443
{
407444
type: 'closeIssue',
@@ -416,7 +453,8 @@ export const githubConnector: VornConnector = {
416453
placeholder: '{{connectorItem.externalId}}',
417454
supportsTemplates: true
418455
}
419-
]
456+
],
457+
outputSchema: GITHUB_ISSUE_SCHEMA
420458
},
421459
{
422460
type: 'commentOnIssue',
@@ -438,7 +476,8 @@ export const githubConnector: VornConnector = {
438476
required: true,
439477
supportsTemplates: true
440478
}
441-
]
479+
],
480+
outputSchema: GITHUB_COMMENT_SCHEMA
442481
}
443482
],
444483
defaultWorkflows: [

packages/server/src/connectors/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
export { connectorRegistry } from './registry'
22
export { githubConnector, detectRepoSlug } from './github'
33
export { linearConnector } from './linear'
4+
export { mcpConnector, invokeMcpTool, discoverTools, mcpConnectionActions } from './mcp'
5+
export type { McpDiscoveredTool } from './mcp'
6+
export { stopClient as stopMcpClient, stopAllClients as stopAllMcpClients } from './mcp-clients'
47
export {
58
setDecryptedCreds,
69
clearDecryptedCreds,
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/**
2+
* In-memory cache of live MCP stdio clients, one per connection.
3+
*
4+
* Each MCP connection points at an external MCP server (npx …, node …, etc.).
5+
* We spawn the child process lazily on first use and keep it alive for the
6+
* lifetime of the server process, so tool invocations don't pay a startup
7+
* cost per call. The map is keyed by `connectionId` so `connection:delete`
8+
* can terminate the right process.
9+
*
10+
* Secret env values flow in via the usual decrypted-creds path: the server
11+
* never sees the encrypted ciphertext, only the plaintext the main process
12+
* pushes via `credentials:setDecrypted`.
13+
*/
14+
import { Client } from '@modelcontextprotocol/sdk/client/index.js'
15+
import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'
16+
import type { SourceConnection } from '@vornrun/shared/types'
17+
import { getDecryptedCreds } from './decrypted-creds'
18+
import { getSafeEnv } from '../process-utils'
19+
import log from '../logger'
20+
21+
interface LiveClient {
22+
client: Client
23+
transport: StdioClientTransport
24+
}
25+
26+
const clients = new Map<string, LiveClient>()
27+
// In-flight startups, so two concurrent `getOrStartClient` calls for the
28+
// same connection share one spawn instead of racing two children.
29+
const pending = new Map<string, Promise<Client>>()
30+
31+
function tryParseJson<T>(raw: unknown, guard: (v: unknown) => v is T, fallback: T): T {
32+
if (typeof raw !== 'string' || raw === '') return fallback
33+
try {
34+
const parsed: unknown = JSON.parse(raw)
35+
return guard(parsed) ? parsed : fallback
36+
} catch {
37+
return fallback
38+
}
39+
}
40+
41+
const isStringMap = (v: unknown): v is Record<string, unknown> =>
42+
!!v && typeof v === 'object' && !Array.isArray(v)
43+
44+
function parseJsonObject(raw: unknown): Record<string, string> {
45+
const obj = tryParseJson<Record<string, unknown>>(raw, isStringMap, {})
46+
const out: Record<string, string> = {}
47+
for (const [k, v] of Object.entries(obj)) out[k] = String(v)
48+
return out
49+
}
50+
51+
function parseJsonArray(raw: unknown): string[] {
52+
const arr = tryParseJson<unknown[]>(raw, (v): v is unknown[] => Array.isArray(v), [])
53+
return arr.map((v) => String(v))
54+
}
55+
56+
function buildSpawnConfig(conn: SourceConnection): {
57+
command: string
58+
args: string[]
59+
env: Record<string, string>
60+
} {
61+
const command = String(conn.filters.command ?? '').trim()
62+
if (!command) throw new Error('MCP connection is missing a command')
63+
const args = parseJsonArray(conn.filters.args)
64+
const env = parseJsonObject(conn.filters.env)
65+
// Decrypted secret env (pushed from main via safeStorage) overrides plain env.
66+
const decrypted = getDecryptedCreds(conn.id) ?? {}
67+
const secretEnv = parseJsonObject(decrypted.secretEnv)
68+
return { command, args, env: { ...env, ...secretEnv } }
69+
}
70+
71+
export async function getOrStartClient(conn: SourceConnection): Promise<Client> {
72+
const existing = clients.get(conn.id)
73+
if (existing) return existing.client
74+
const inFlight = pending.get(conn.id)
75+
if (inFlight) return inFlight
76+
77+
const startup = startClient(conn).finally(() => {
78+
pending.delete(conn.id)
79+
})
80+
pending.set(conn.id, startup)
81+
return startup
82+
}
83+
84+
async function startClient(conn: SourceConnection): Promise<Client> {
85+
const { command, args, env } = buildSpawnConfig(conn)
86+
// Inherit PATH and friends from the parent via getSafeEnv() — same
87+
// sanitization the rest of the server uses for child processes — so
88+
// GH/Linear/NPM tokens etc. don't leak into arbitrary MCP servers
89+
// the user adds. Explicit per-connection env still wins over the base.
90+
const transport = new StdioClientTransport({
91+
command,
92+
args,
93+
env: { ...getSafeEnv(), ...env }
94+
})
95+
96+
const client = new Client({ name: 'vorn', version: '0.1.0' }, { capabilities: {} })
97+
98+
try {
99+
await client.connect(transport)
100+
} catch (err) {
101+
// Transport already spawned the child; close it so we don't leak.
102+
try {
103+
await transport.close()
104+
} catch {
105+
/* ignore */
106+
}
107+
throw err
108+
}
109+
110+
const live: LiveClient = { client, transport }
111+
clients.set(conn.id, live)
112+
113+
// If the child exits, drop it from the cache so the next call respawns.
114+
transport.onclose = () => {
115+
const current = clients.get(conn.id)
116+
if (current === live) {
117+
clients.delete(conn.id)
118+
log.info(`[mcp-clients] ${conn.id} transport closed, will respawn on next call`)
119+
}
120+
}
121+
transport.onerror = (err) => {
122+
log.warn(`[mcp-clients] ${conn.id} transport error: ${err}`)
123+
}
124+
125+
return client
126+
}
127+
128+
export async function stopClient(connectionId: string): Promise<void> {
129+
const live = clients.get(connectionId)
130+
if (!live) return
131+
clients.delete(connectionId)
132+
try {
133+
await live.client.close()
134+
} catch (err) {
135+
log.warn(`[mcp-clients] ${connectionId} close failed: ${err}`)
136+
}
137+
}
138+
139+
export async function stopAllClients(): Promise<void> {
140+
const ids = [...clients.keys()]
141+
await Promise.allSettled(ids.map(stopClient))
142+
}
143+
144+
export function hasClient(connectionId: string): boolean {
145+
return clients.has(connectionId)
146+
}

0 commit comments

Comments
 (0)