Skip to content

Commit d2814c5

Browse files
waleedlatif1waleedlatif1
committed
ack PR comments
1 parent cc0f680 commit d2814c5

20 files changed

Lines changed: 473 additions & 404 deletions

File tree

‎apps/sim/app/api/mcp/servers/route.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { NextRequest } from 'next/server'
33
import { createLogger } from '@/lib/logs/console/logger'
44
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
55
import { mcpService } from '@/lib/mcp/service'
6+
import type { McpTransport } from '@/lib/mcp/types'
67
import { validateMcpServerUrl } from '@/lib/mcp/url-validator'
78
import { createMcpErrorResponse, createMcpSuccessResponse } from '@/lib/mcp/utils'
89
import { db } from '@/db'
@@ -12,6 +13,13 @@ const logger = createLogger('McpServersAPI')
1213

1314
export const dynamic = 'force-dynamic'
1415

16+
/**
17+
* Check if transport type requires a URL
18+
*/
19+
function isUrlBasedTransport(transport: McpTransport): boolean {
20+
return transport === 'http' || transport === 'sse' || transport === 'streamable-http'
21+
}
22+
1523
/**
1624
* GET - List all registered MCP servers for the workspace
1725
*/
@@ -62,12 +70,7 @@ export const POST = withMcpAuth('write')(
6270
)
6371
}
6472

65-
if (
66-
(body.transport === 'http' ||
67-
body.transport === 'sse' ||
68-
body.transport === 'streamable-http') &&
69-
body.url
70-
) {
73+
if (isUrlBasedTransport(body.transport) && body.url) {
7174
const urlValidation = validateMcpServerUrl(body.url)
7275
if (!urlValidation.isValid) {
7376
return createMcpErrorResponse(

‎apps/sim/app/api/mcp/servers/test-connection/route.ts‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,21 @@ import { getEffectiveDecryptedEnv } from '@/lib/environment/utils'
33
import { createLogger } from '@/lib/logs/console/logger'
44
import { McpClient } from '@/lib/mcp/client'
55
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
6-
import type { McpServerConfig } from '@/lib/mcp/types'
6+
import type { McpServerConfig, McpTransport } from '@/lib/mcp/types'
77
import { validateMcpServerUrl } from '@/lib/mcp/url-validator'
88
import { createMcpErrorResponse, createMcpSuccessResponse } from '@/lib/mcp/utils'
99

1010
const logger = createLogger('McpServerTestAPI')
1111

1212
export const dynamic = 'force-dynamic'
1313

14+
/**
15+
* Check if transport type requires a URL
16+
*/
17+
function isUrlBasedTransport(transport: McpTransport): boolean {
18+
return transport === 'http' || transport === 'sse' || transport === 'streamable-http'
19+
}
20+
1421
/**
1522
* Resolve environment variables in strings
1623
*/
@@ -35,7 +42,7 @@ function resolveEnvVars(value: string, envVars: Record<string, string>): string
3542

3643
interface TestConnectionRequest {
3744
name: string
38-
transport: 'http' | 'sse' | 'streamable-http'
45+
transport: McpTransport
3946
url?: string
4047
headers?: Record<string, string>
4148
timeout?: number
@@ -78,12 +85,15 @@ export const POST = withMcpAuth('write')(
7885
)
7986
}
8087

81-
if (
82-
(body.transport === 'http' ||
83-
body.transport === 'sse' ||
84-
body.transport === 'streamable-http') &&
85-
body.url
86-
) {
88+
if (isUrlBasedTransport(body.transport)) {
89+
if (!body.url) {
90+
return createMcpErrorResponse(
91+
new Error('URL is required for HTTP-based transports'),
92+
'Missing required URL',
93+
400
94+
)
95+
}
96+
8797
const urlValidation = validateMcpServerUrl(body.url)
8898
if (!urlValidation.isValid) {
8999
return createMcpErrorResponse(

‎apps/sim/app/api/mcp/tools/execute/route.ts‎

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { NextRequest } from 'next/server'
22
import { createLogger } from '@/lib/logs/console/logger'
33
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
44
import { mcpService } from '@/lib/mcp/service'
5-
import type { McpToolCall, McpToolResult } from '@/lib/mcp/types'
5+
import type { McpTool, McpToolCall, McpToolResult } from '@/lib/mcp/types'
66
import {
77
categorizeError,
88
createMcpErrorResponse,
@@ -15,6 +15,29 @@ const logger = createLogger('McpToolExecutionAPI')
1515

1616
export const dynamic = 'force-dynamic'
1717

18+
// Type definitions for improved type safety
19+
interface SchemaProperty {
20+
type: 'string' | 'number' | 'boolean' | 'object' | 'array'
21+
description?: string
22+
enum?: unknown[]
23+
format?: string
24+
items?: SchemaProperty
25+
properties?: Record<string, SchemaProperty>
26+
}
27+
28+
interface ToolExecutionResult {
29+
success: boolean
30+
output?: McpToolResult
31+
error?: string
32+
}
33+
34+
/**
35+
* Type guard to safely check if a schema property has a type field
36+
*/
37+
function hasType(prop: unknown): prop is SchemaProperty {
38+
return typeof prop === 'object' && prop !== null && 'type' in prop
39+
}
40+
1841
/**
1942
* POST - Execute a tool on an MCP server
2043
*/
@@ -66,6 +89,46 @@ export const POST = withMcpAuth('read')(
6689
404
6790
)
6891
}
92+
93+
// Parse array arguments based on tool schema
94+
if (tool.inputSchema?.properties) {
95+
for (const [paramName, paramSchema] of Object.entries(tool.inputSchema.properties)) {
96+
const schema = paramSchema as any
97+
if (
98+
schema.type === 'array' &&
99+
args[paramName] !== undefined &&
100+
typeof args[paramName] === 'string'
101+
) {
102+
const stringValue = args[paramName].trim()
103+
if (stringValue) {
104+
try {
105+
// Try to parse as JSON first (handles ["item1", "item2"])
106+
const parsed = JSON.parse(stringValue)
107+
if (Array.isArray(parsed)) {
108+
args[paramName] = parsed
109+
} else {
110+
// JSON parsed but not an array, wrap in array
111+
args[paramName] = [parsed]
112+
}
113+
} catch (error) {
114+
// JSON parsing failed - treat as comma-separated if contains commas, otherwise single item
115+
if (stringValue.includes(',')) {
116+
args[paramName] = stringValue
117+
.split(',')
118+
.map((item) => item.trim())
119+
.filter((item) => item)
120+
} else {
121+
// Single item - wrap in array since schema expects array
122+
args[paramName] = [stringValue]
123+
}
124+
}
125+
} else {
126+
// Empty string becomes empty array
127+
args[paramName] = []
128+
}
129+
}
130+
}
131+
}
69132
} catch (error) {
70133
logger.warn(
71134
`[${requestId}] Failed to discover tools for validation, proceeding anyway:`,
@@ -124,7 +187,7 @@ export const POST = withMcpAuth('read')(
124187
/**
125188
* Validate tool arguments against schema
126189
*/
127-
function validateToolArguments(tool: any, args: any): string | null {
190+
function validateToolArguments(tool: McpTool, args: Record<string, unknown>): string | null {
128191
if (!tool.inputSchema) {
129192
return null // No schema to validate against
130193
}
@@ -142,8 +205,8 @@ function validateToolArguments(tool: any, args: any): string | null {
142205
if (schema.properties && args) {
143206
for (const [propName, propSchema] of Object.entries(schema.properties)) {
144207
const propValue = args[propName]
145-
if (propValue !== undefined) {
146-
const expectedType = (propSchema as any).type
208+
if (propValue !== undefined && hasType(propSchema)) {
209+
const expectedType = propSchema.type
147210
const actualType = typeof propValue
148211

149212
if (expectedType === 'string' && actualType !== 'string') {
@@ -174,12 +237,11 @@ function validateToolArguments(tool: any, args: any): string | null {
174237
/**
175238
* Transform MCP tool result to platform format
176239
*/
177-
function transformToolResult(result: McpToolResult): any {
240+
function transformToolResult(result: McpToolResult): ToolExecutionResult {
178241
if (result.isError) {
179242
return {
180243
success: false,
181244
error: result.content?.[0]?.text || 'Tool execution failed',
182-
output: null,
183245
}
184246
}
185247

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/create-chunk-modal/create-chunk-modal.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ export function CreateChunkModal({
192192
<Button
193193
onClick={handleCreateChunk}
194194
disabled={!isFormValid || isCreating}
195-
className='bg-[var(--brand-primary-hex)] font-[480] text-muted-foreground shadow-[0_0_0_0_var(--brand-primary-hex)] transition-all duration-200 hover:bg-[var(--brand-secondary-hex)] hover:shadow-[0_0_0_4px_rgba(127,47,255,0.15)]'
195+
className='bg-[var(--brand-primary-hex)] font-[480] text-primary-foreground shadow-[0_0_0_0_var(--brand-primary-hex)] transition-all duration-200 hover:bg-[var(--brand-secondary-hex)] hover:shadow-[0_0_0_4px_rgba(127,47,255,0.15)]'
196196
>
197197
{isCreating ? (
198198
<>

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/upload-modal/upload-modal.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ export function UploadModal({
290290
<Button
291291
onClick={handleUpload}
292292
disabled={files.length === 0 || isUploading}
293-
className='bg-[var(--brand-primary-hex)] font-[480] text-muted-foreground shadow-[0_0_0_0_var(--brand-primary-hex)] transition-all duration-200 hover:bg-[var(--brand-primary-hover-hex)] hover:shadow-[0_0_0_4px_rgba(127,47,255,0.15)]'
293+
className='bg-[var(--brand-primary-hex)] font-[480] text-primary-foreground shadow-[0_0_0_0_var(--brand-primary-hex)] transition-all duration-200 hover:bg-[var(--brand-primary-hover-hex)] hover:shadow-[0_0_0_4px_rgba(127,47,255,0.15)]'
294294
>
295295
{isUploading
296296
? uploadProgress.stage === 'uploading'

‎apps/sim/app/workspace/[workspaceId]/knowledge/components/create-modal/create-modal.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,7 @@ export function CreateModal({ open, onOpenChange, onKnowledgeBaseCreated }: Crea
627627
<Button
628628
type='submit'
629629
disabled={isSubmitting || !nameValue?.trim()}
630-
className='bg-[var(--brand-primary-hex)] font-[480] text-muted-foreground shadow-[0_0_0_0_var(--brand-primary-hex)] transition-all duration-200 hover:bg-[var(--brand-primary-hover-hex)] hover:shadow-[0_0_0_4px_rgba(127,47,255,0.15)] disabled:opacity-50 disabled:hover:shadow-none'
630+
className='bg-[var(--brand-primary-hex)] font-[480] text-primary-foreground shadow-[0_0_0_0_var(--brand-primary-hex)] transition-all duration-200 hover:bg-[var(--brand-primary-hover-hex)] hover:shadow-[0_0_0_4px_rgba(127,47,255,0.15)] disabled:opacity-50 disabled:hover:shadow-none'
631631
>
632632
{isSubmitting
633633
? isUploading

0 commit comments

Comments
 (0)