-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add external MCP server target support and unassigned targets #406
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -189,6 +189,36 @@ export async function validateAddGatewayTargetOptions(options: AddGatewayTargetO | |
| return { valid: false, error: '--name is required' }; | ||
| } | ||
|
|
||
| if (options.type && options.type !== 'mcpServer' && options.type !== 'lambda') { | ||
| return { valid: false, error: 'Invalid type. Valid options: mcpServer, lambda' }; | ||
| } | ||
|
|
||
| if (options.source && options.source !== 'existing-endpoint' && options.source !== 'create-new') { | ||
| return { valid: false, error: 'Invalid source. Valid options: existing-endpoint, create-new' }; | ||
| } | ||
|
|
||
| if (options.source === 'existing-endpoint') { | ||
| if (!options.endpoint) { | ||
| return { valid: false, error: '--endpoint is required when source is existing-endpoint' }; | ||
| } | ||
|
Comment on lines
+200
to
+203
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When source === 'existing-endpoint', this returns { valid: true } immediately but ValidatedAddGatewayTargetOptions still requires language, exposure, etc. In non-interactive mode, downstream code that reads those fields will get undefined. We cna either return a distinct validated type for external endpoints, or populate sensible defaults for the skipped fields before returning. |
||
|
|
||
| try { | ||
| const url = new URL(options.endpoint); | ||
| if (url.protocol !== 'http:' && url.protocol !== 'https:') { | ||
| return { valid: false, error: 'Endpoint must use http:// or https:// protocol' }; | ||
| } | ||
| } catch { | ||
| return { valid: false, error: 'Endpoint must be a valid URL (e.g. https://example.com/mcp)' }; | ||
| } | ||
|
|
||
| // Populate defaults for fields skipped by external endpoint flow | ||
| options.language ??= 'Other'; | ||
| options.exposure ??= 'behind-gateway'; | ||
| options.gateway ??= undefined; | ||
|
|
||
| return { valid: true }; | ||
| } | ||
|
|
||
| if (!options.language) { | ||
| return { valid: false, error: '--language is required' }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { createExternalGatewayTarget } from '../../../operations/mcp/create-mcp'; | ||
| import { ErrorPrompt, Panel, Screen, TextInput, WizardSelect } from '../../components'; | ||
| import type { SelectableItem } from '../../components'; | ||
| import { HELP_TEXT } from '../../constants'; | ||
|
|
@@ -114,14 +115,25 @@ export function AddGatewayTargetFlow({ | |
| loading: true, | ||
| loadingMessage: 'Creating MCP tool...', | ||
| }); | ||
| void createTool(config).then(result => { | ||
| if (result.ok) { | ||
| const { toolName, projectPath } = result.result; | ||
| setFlow({ name: 'create-success', toolName, projectPath }); | ||
| return; | ||
| } | ||
| setFlow({ name: 'error', message: result.error }); | ||
| }); | ||
|
|
||
| if (config.source === 'existing-endpoint') { | ||
| void createExternalGatewayTarget(config) | ||
| .then((result: { toolName: string; projectPath: string }) => { | ||
| setFlow({ name: 'create-success', toolName: result.toolName, projectPath: result.projectPath }); | ||
| }) | ||
| .catch((err: unknown) => { | ||
| setFlow({ name: 'error', message: err instanceof Error ? err.message : 'Unknown error' }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: The parameter is typed as Error, but .catch() receives unknown at runtime. The instanceof check is actually correct, the type annotation should be unknown to be honest about it: .catch((err: unknown) => { |
||
| }); | ||
| } else { | ||
| void createTool(config).then(result => { | ||
| if (result.ok) { | ||
| const { toolName, projectPath } = result.result; | ||
| setFlow({ name: 'create-success', toolName, projectPath }); | ||
| return; | ||
| } | ||
| setFlow({ name: 'error', message: result.error }); | ||
| }); | ||
| } | ||
| }, | ||
| [createTool] | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,14 @@ import { HELP_TEXT } from '../../constants'; | |
| import { useListNavigation, useMultiSelectNavigation } from '../../hooks'; | ||
| import { generateUniqueName } from '../../utils'; | ||
| import type { AddGatewayTargetConfig, ComputeHost, ExposureMode, TargetLanguage } from './types'; | ||
| import { COMPUTE_HOST_OPTIONS, EXPOSURE_MODE_OPTIONS, MCP_TOOL_STEP_LABELS, TARGET_LANGUAGE_OPTIONS } from './types'; | ||
| import { | ||
| COMPUTE_HOST_OPTIONS, | ||
| EXPOSURE_MODE_OPTIONS, | ||
| MCP_TOOL_STEP_LABELS, | ||
| SKIP_FOR_NOW, | ||
| SOURCE_OPTIONS, | ||
| TARGET_LANGUAGE_OPTIONS, | ||
| } from './types'; | ||
| import { useAddGatewayTargetWizard } from './useAddGatewayTargetWizard'; | ||
| import { Box, Text } from 'ink'; | ||
| import React, { useMemo } from 'react'; | ||
|
|
@@ -35,6 +42,11 @@ export function AddGatewayTargetScreen({ | |
| }: AddGatewayTargetScreenProps) { | ||
| const wizard = useAddGatewayTargetWizard(existingGateways, existingAgents); | ||
|
|
||
| const sourceItems: SelectableItem[] = useMemo( | ||
| () => SOURCE_OPTIONS.map(o => ({ id: o.id, title: o.title, description: o.description })), | ||
| [] | ||
| ); | ||
|
|
||
| const languageItems: SelectableItem[] = useMemo( | ||
| () => TARGET_LANGUAGE_OPTIONS.map(o => ({ id: o.id, title: o.title, description: o.description })), | ||
| [] | ||
|
|
@@ -52,7 +64,10 @@ export function AddGatewayTargetScreen({ | |
| ); | ||
|
|
||
| const gatewayItems: SelectableItem[] = useMemo( | ||
| () => existingGateways.map(g => ({ id: g, title: g })), | ||
| () => [ | ||
| ...existingGateways.map(g => ({ id: g, title: g })), | ||
| { id: SKIP_FOR_NOW, title: 'Skip for now', description: 'Create unassigned target' }, | ||
| ], | ||
| [existingGateways] | ||
| ); | ||
|
|
||
|
|
@@ -63,16 +78,24 @@ export function AddGatewayTargetScreen({ | |
|
|
||
| const agentItems: SelectableItem[] = useMemo(() => existingAgents.map(a => ({ id: a, title: a })), [existingAgents]); | ||
|
|
||
| const isSourceStep = wizard.step === 'source'; | ||
| const isLanguageStep = wizard.step === 'language'; | ||
| const isExposureStep = wizard.step === 'exposure'; | ||
| const isAgentsStep = wizard.step === 'agents'; | ||
| const isGatewayStep = wizard.step === 'gateway'; | ||
| const isHostStep = wizard.step === 'host'; | ||
| const isTextStep = wizard.step === 'name'; | ||
| const isTextStep = wizard.step === 'name' || wizard.step === 'endpoint'; | ||
| const isConfirmStep = wizard.step === 'confirm'; | ||
| const noGatewaysAvailable = isGatewayStep && existingGateways.length === 0; | ||
| const noAgentsAvailable = isAgentsStep && existingAgents.length === 0; | ||
|
|
||
| const sourceNav = useListNavigation({ | ||
| items: sourceItems, | ||
| onSelect: item => wizard.setSource(item.id as 'existing-endpoint' | 'create-new'), | ||
| onExit: () => wizard.goBack(), | ||
| isActive: isSourceStep, | ||
| }); | ||
|
|
||
| const languageNav = useListNavigation({ | ||
| items: languageItems, | ||
| onSelect: item => wizard.setLanguage(item.id as TargetLanguage), | ||
|
|
@@ -132,6 +155,15 @@ export function AddGatewayTargetScreen({ | |
| return ( | ||
| <Screen title="Add MCP Tool" onExit={onExit} helpText={helpText} headerContent={headerContent}> | ||
| <Panel> | ||
| {isSourceStep && ( | ||
| <WizardSelect | ||
| title="Select source" | ||
| description="How would you like to create this MCP tool?" | ||
| items={sourceItems} | ||
| selectedIndex={sourceNav.selectedIndex} | ||
| /> | ||
| )} | ||
|
|
||
| {isLanguageStep && ( | ||
| <WizardSelect title="Select language" items={languageItems} selectedIndex={languageNav.selectedIndex} /> | ||
| )} | ||
|
|
@@ -180,27 +212,54 @@ export function AddGatewayTargetScreen({ | |
| {isTextStep && ( | ||
| <TextInput | ||
| key={wizard.step} | ||
| prompt={MCP_TOOL_STEP_LABELS[wizard.step]} | ||
| initialValue={generateUniqueName('mytool', existingToolNames)} | ||
| onSubmit={wizard.setName} | ||
| prompt={wizard.step === 'endpoint' ? 'MCP server endpoint URL' : MCP_TOOL_STEP_LABELS[wizard.step]} | ||
| initialValue={wizard.step === 'endpoint' ? undefined : generateUniqueName('mytool', existingToolNames)} | ||
| placeholder={wizard.step === 'endpoint' ? 'https://example.com/mcp' : undefined} | ||
| onSubmit={wizard.step === 'endpoint' ? wizard.setEndpoint : wizard.setName} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing endpoint validation in the TUI path which means users can submit any string. the CLI path validates the URL in validate.ts, but the interactive wizard does not. we can add a customValidation that mirrors the new URL() + protocol check. |
||
| onCancel={() => (wizard.currentIndex === 0 ? onExit() : wizard.goBack())} | ||
| schema={ToolNameSchema} | ||
| customValidation={value => !existingToolNames.includes(value) || 'Tool name already exists'} | ||
| schema={wizard.step === 'name' ? ToolNameSchema : undefined} | ||
| customValidation={ | ||
| wizard.step === 'name' | ||
| ? value => !existingToolNames.includes(value) || 'Tool name already exists' | ||
| : wizard.step === 'endpoint' | ||
| ? value => { | ||
| try { | ||
| const url = new URL(value); | ||
| if (url.protocol !== 'http:' && url.protocol !== 'https:') { | ||
| return 'Endpoint must use http:// or https:// protocol'; | ||
| } | ||
| return true; | ||
| } catch { | ||
| return 'Must be a valid URL (e.g. https://example.com/mcp)'; | ||
| } | ||
| } | ||
| : undefined | ||
| } | ||
| /> | ||
| )} | ||
|
|
||
| {isConfirmStep && ( | ||
| <ConfirmReview | ||
| fields={[ | ||
| { label: 'Name', value: wizard.config.name }, | ||
| { label: 'Language', value: wizard.config.language }, | ||
| { label: 'Exposure', value: isMcpRuntime ? 'MCP Runtime' : 'Behind Gateway' }, | ||
| { | ||
| label: 'Source', | ||
| value: wizard.config.source === 'existing-endpoint' ? 'Existing endpoint' : 'Create new', | ||
| }, | ||
| ...(wizard.config.endpoint ? [{ label: 'Endpoint', value: wizard.config.endpoint }] : []), | ||
| ...(wizard.config.source === 'create-new' ? [{ label: 'Language', value: wizard.config.language }] : []), | ||
| ...(wizard.config.source === 'create-new' | ||
| ? [{ label: 'Exposure', value: isMcpRuntime ? 'MCP Runtime' : 'Behind Gateway' }] | ||
| : []), | ||
| ...(isMcpRuntime && wizard.config.selectedAgents.length > 0 | ||
| ? [{ label: 'Agents', value: wizard.config.selectedAgents.join(', ') }] | ||
| : []), | ||
| ...(!isMcpRuntime && wizard.config.gateway ? [{ label: 'Gateway', value: wizard.config.gateway }] : []), | ||
| { label: 'Host', value: wizard.config.host }, | ||
| { label: 'Source', value: wizard.config.sourcePath }, | ||
| ...(!isMcpRuntime && !wizard.config.gateway | ||
| ? [{ label: 'Gateway', value: '(none - assign later)' }] | ||
| : []), | ||
| ...(wizard.config.source === 'create-new' ? [{ label: 'Host', value: wizard.config.host }] : []), | ||
| ...(wizard.config.source === 'create-new' ? [{ label: 'Source', value: wizard.config.sourcePath }] : []), | ||
| ]} | ||
| /> | ||
| )} | ||
|
|
||
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.
nit: this --type option is added but never used. validate.ts validates it, but nothing in the flow reads options.type to change behavior