Skip to content

Commit c5b3fcb

Browse files
authored
fix(copilot): fix custom tools (#2278)
* Fix title custom tool * Checkpoitn (broken) * Fix custom tool flash * Edit workflow returns null fix * Works * Fix lint
1 parent dd7db6e commit c5b3fcb

File tree

8 files changed

+532
-111
lines changed

8 files changed

+532
-111
lines changed

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
} from '@/lib/workflows/triggers/triggers'
1717
import { useCurrentWorkflow } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-current-workflow'
1818
import type { BlockLog, ExecutionResult, StreamingExecution } from '@/executor/types'
19+
import { coerceValue } from '@/executor/utils/start-block'
1920
import { subscriptionKeys } from '@/hooks/queries/subscription'
2021
import { useExecutionStream } from '@/hooks/use-execution-stream'
2122
import { WorkflowValidationError } from '@/serializer'
@@ -757,7 +758,7 @@ export function useWorkflowExecution() {
757758
if (Array.isArray(inputFormatValue)) {
758759
inputFormatValue.forEach((field: any) => {
759760
if (field && typeof field === 'object' && field.name && field.value !== undefined) {
760-
testInput[field.name] = field.value
761+
testInput[field.name] = coerceValue(field.type, field.value)
761762
}
762763
})
763764
}

apps/sim/executor/utils/start-block.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ function extractInputFormat(block: SerializedBlock): InputFormatField[] {
132132
.map((field) => field)
133133
}
134134

135-
function coerceValue(type: string | null | undefined, value: unknown): unknown {
135+
export function coerceValue(type: string | null | undefined, value: unknown): unknown {
136136
if (value === undefined || value === null) {
137137
return value
138138
}

apps/sim/lib/copilot/registry.ts

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export const ToolIds = z.enum([
3232
'navigate_ui',
3333
'knowledge_base',
3434
'manage_custom_tool',
35+
'manage_mcp_tool',
3536
])
3637
export type ToolId = z.infer<typeof ToolIds>
3738

@@ -199,12 +200,6 @@ export const ToolArgSchemas = {
199200
.describe(
200201
'Required for edit and delete operations. The database ID of the custom tool (e.g., "0robnW7_JUVwZrDkq1mqj"). Use get_workflow_data with data_type "custom_tools" to get the list of tools and their IDs. Do NOT use the function name - use the actual "id" field from the tool.'
201202
),
202-
title: z
203-
.string()
204-
.optional()
205-
.describe(
206-
'The display title of the custom tool. Required for add. Should always be provided for edit/delete so the user knows which tool is being modified.'
207-
),
208203
schema: z
209204
.object({
210205
type: z.literal('function'),
@@ -227,6 +222,36 @@ export const ToolArgSchemas = {
227222
'Required for add. The JavaScript function body code. Use {{ENV_VAR}} for environment variables and reference parameters directly by name.'
228223
),
229224
}),
225+
226+
manage_mcp_tool: z.object({
227+
operation: z
228+
.enum(['add', 'edit', 'delete'])
229+
.describe('The operation to perform: add (create new), edit (update existing), or delete'),
230+
serverId: z
231+
.string()
232+
.optional()
233+
.describe(
234+
'Required for edit and delete operations. The database ID of the MCP server. Use the MCP settings panel or API to get server IDs.'
235+
),
236+
config: z
237+
.object({
238+
name: z.string().describe('The display name for the MCP server'),
239+
transport: z
240+
.enum(['streamable-http'])
241+
.optional()
242+
.default('streamable-http')
243+
.describe('Transport protocol (currently only streamable-http is supported)'),
244+
url: z.string().optional().describe('The MCP server endpoint URL (required for add)'),
245+
headers: z
246+
.record(z.string())
247+
.optional()
248+
.describe('Optional HTTP headers to send with requests'),
249+
timeout: z.number().optional().describe('Request timeout in milliseconds (default: 30000)'),
250+
enabled: z.boolean().optional().describe('Whether the server is enabled (default: true)'),
251+
})
252+
.optional()
253+
.describe('Required for add and edit operations. The MCP server configuration.'),
254+
}),
230255
} as const
231256
export type ToolArgSchemaMap = typeof ToolArgSchemas
232257

@@ -292,6 +317,7 @@ export const ToolSSESchemas = {
292317
navigate_ui: toolCallSSEFor('navigate_ui', ToolArgSchemas.navigate_ui),
293318
knowledge_base: toolCallSSEFor('knowledge_base', ToolArgSchemas.knowledge_base),
294319
manage_custom_tool: toolCallSSEFor('manage_custom_tool', ToolArgSchemas.manage_custom_tool),
320+
manage_mcp_tool: toolCallSSEFor('manage_mcp_tool', ToolArgSchemas.manage_mcp_tool),
295321
} as const
296322
export type ToolSSESchemaMap = typeof ToolSSESchemas
297323

@@ -519,6 +545,13 @@ export const ToolResultSchemas = {
519545
title: z.string().optional(),
520546
message: z.string().optional(),
521547
}),
548+
manage_mcp_tool: z.object({
549+
success: z.boolean(),
550+
operation: z.enum(['add', 'edit', 'delete']),
551+
serverId: z.string().optional(),
552+
serverName: z.string().optional(),
553+
message: z.string().optional(),
554+
}),
522555
} as const
523556
export type ToolResultSchemaMap = typeof ToolResultSchemas
524557

apps/sim/lib/copilot/tools/client/workflow/edit-workflow.ts

Lines changed: 95 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,26 @@ export class EditWorkflowClientTool extends BaseClientTool {
9393
}
9494
}
9595

96+
/**
97+
* Safely get the current workflow JSON sanitized for copilot without throwing.
98+
* Used to ensure we always include workflow state in markComplete.
99+
*/
100+
private getCurrentWorkflowJsonSafe(logger: ReturnType<typeof createLogger>): string | undefined {
101+
try {
102+
const currentState = useWorkflowStore.getState().getWorkflowState()
103+
if (!currentState) {
104+
logger.warn('No current workflow state available')
105+
return undefined
106+
}
107+
return this.getSanitizedWorkflowJson(currentState)
108+
} catch (error) {
109+
logger.warn('Failed to get current workflow JSON safely', {
110+
error: error instanceof Error ? error.message : String(error),
111+
})
112+
return undefined
113+
}
114+
}
115+
96116
static readonly metadata: BaseClientToolMetadata = {
97117
displayNames: {
98118
[ClientToolCallState.generating]: { text: 'Editing your workflow', icon: Loader2 },
@@ -133,66 +153,16 @@ export class EditWorkflowClientTool extends BaseClientTool {
133153

134154
async handleAccept(): Promise<void> {
135155
const logger = createLogger('EditWorkflowClientTool')
136-
logger.info('handleAccept called', {
137-
toolCallId: this.toolCallId,
138-
state: this.getState(),
139-
hasResult: this.lastResult !== undefined,
140-
})
141-
this.setState(ClientToolCallState.success)
142-
143-
// Read from the workflow store to get the actual state with diff applied
144-
const workflowStore = useWorkflowStore.getState()
145-
const currentState = workflowStore.getWorkflowState()
146-
147-
// Get the workflow state that was applied, merge subblocks, and sanitize
148-
// This matches what get_user_workflow would return
149-
const workflowJson = this.getSanitizedWorkflowJson(currentState)
150-
151-
// Build sanitized data including workflow JSON and any skipped/validation info from the result
152-
const sanitizedData: Record<string, any> = {}
153-
if (workflowJson) {
154-
sanitizedData.userWorkflow = workflowJson
155-
}
156-
157-
// Include skipped items and validation errors in the accept response for LLM feedback
158-
if (this.lastResult?.skippedItems?.length > 0) {
159-
sanitizedData.skippedItems = this.lastResult.skippedItems
160-
sanitizedData.skippedItemsMessage = this.lastResult.skippedItemsMessage
161-
}
162-
if (this.lastResult?.inputValidationErrors?.length > 0) {
163-
sanitizedData.inputValidationErrors = this.lastResult.inputValidationErrors
164-
sanitizedData.inputValidationMessage = this.lastResult.inputValidationMessage
165-
}
166-
167-
// Build a message that includes info about skipped items
168-
let acceptMessage = 'Workflow edits accepted'
169-
if (
170-
this.lastResult?.skippedItems?.length > 0 ||
171-
this.lastResult?.inputValidationErrors?.length > 0
172-
) {
173-
const parts: string[] = []
174-
if (this.lastResult?.skippedItems?.length > 0) {
175-
parts.push(`${this.lastResult.skippedItems.length} operation(s) were skipped`)
176-
}
177-
if (this.lastResult?.inputValidationErrors?.length > 0) {
178-
parts.push(`${this.lastResult.inputValidationErrors.length} input(s) were rejected`)
179-
}
180-
acceptMessage = `Workflow edits accepted. Note: ${parts.join(', ')}.`
181-
}
182-
183-
await this.markToolComplete(
184-
200,
185-
acceptMessage,
186-
Object.keys(sanitizedData).length > 0 ? sanitizedData : undefined
187-
)
156+
logger.info('handleAccept called', { toolCallId: this.toolCallId, state: this.getState() })
157+
// Tool was already marked complete in execute() - this is just for UI state
188158
this.setState(ClientToolCallState.success)
189159
}
190160

191161
async handleReject(): Promise<void> {
192162
const logger = createLogger('EditWorkflowClientTool')
193163
logger.info('handleReject called', { toolCallId: this.toolCallId, state: this.getState() })
164+
// Tool was already marked complete in execute() - this is just for UI state
194165
this.setState(ClientToolCallState.rejected)
195-
await this.markToolComplete(200, 'Workflow changes rejected')
196166
}
197167

198168
async execute(args?: EditWorkflowArgs): Promise<void> {
@@ -202,9 +172,14 @@ export class EditWorkflowClientTool extends BaseClientTool {
202172
await this.executeWithTimeout(async () => {
203173
if (this.hasExecuted) {
204174
logger.info('execute skipped (already executed)', { toolCallId: this.toolCallId })
205-
// Even if skipped, ensure we mark complete
175+
// Even if skipped, ensure we mark complete with current workflow state
206176
if (!this.hasBeenMarkedComplete()) {
207-
await this.markToolComplete(200, 'Tool already executed')
177+
const currentWorkflowJson = this.getCurrentWorkflowJsonSafe(logger)
178+
await this.markToolComplete(
179+
200,
180+
'Tool already executed',
181+
currentWorkflowJson ? { userWorkflow: currentWorkflowJson } : undefined
182+
)
208183
}
209184
return
210185
}
@@ -231,7 +206,12 @@ export class EditWorkflowClientTool extends BaseClientTool {
231206
const operations = args?.operations || []
232207
if (!operations.length) {
233208
this.setState(ClientToolCallState.error)
234-
await this.markToolComplete(400, 'No operations provided for edit_workflow')
209+
const currentWorkflowJson = this.getCurrentWorkflowJsonSafe(logger)
210+
await this.markToolComplete(
211+
400,
212+
'No operations provided for edit_workflow',
213+
currentWorkflowJson ? { userWorkflow: currentWorkflowJson } : undefined
214+
)
235215
return
236216
}
237217

@@ -281,12 +261,22 @@ export class EditWorkflowClientTool extends BaseClientTool {
281261

282262
if (!res.ok) {
283263
const errorText = await res.text().catch(() => '')
264+
let errorMessage: string
284265
try {
285266
const errorJson = JSON.parse(errorText)
286-
throw new Error(errorJson.error || errorText || `Server error (${res.status})`)
267+
errorMessage = errorJson.error || errorText || `Server error (${res.status})`
287268
} catch {
288-
throw new Error(errorText || `Server error (${res.status})`)
269+
errorMessage = errorText || `Server error (${res.status})`
289270
}
271+
// Mark complete with error but include current workflow state
272+
this.setState(ClientToolCallState.error)
273+
const currentWorkflowJson = this.getCurrentWorkflowJsonSafe(logger)
274+
await this.markToolComplete(
275+
res.status,
276+
errorMessage,
277+
currentWorkflowJson ? { userWorkflow: currentWorkflowJson } : undefined
278+
)
279+
return
290280
}
291281

292282
const json = await res.json()
@@ -318,7 +308,14 @@ export class EditWorkflowClientTool extends BaseClientTool {
318308

319309
// Update diff directly with workflow state - no YAML conversion needed!
320310
if (!result.workflowState) {
321-
throw new Error('No workflow state returned from server')
311+
this.setState(ClientToolCallState.error)
312+
const currentWorkflowJson = this.getCurrentWorkflowJsonSafe(logger)
313+
await this.markToolComplete(
314+
500,
315+
'No workflow state returned from server',
316+
currentWorkflowJson ? { userWorkflow: currentWorkflowJson } : undefined
317+
)
318+
return
322319
}
323320

324321
let actualDiffWorkflow: WorkflowState | null = null
@@ -336,17 +333,37 @@ export class EditWorkflowClientTool extends BaseClientTool {
336333
actualDiffWorkflow = workflowStore.getWorkflowState()
337334

338335
if (!actualDiffWorkflow) {
339-
throw new Error('Failed to retrieve workflow state after applying changes')
336+
this.setState(ClientToolCallState.error)
337+
const currentWorkflowJson = this.getCurrentWorkflowJsonSafe(logger)
338+
await this.markToolComplete(
339+
500,
340+
'Failed to retrieve workflow state after applying changes',
341+
currentWorkflowJson ? { userWorkflow: currentWorkflowJson } : undefined
342+
)
343+
return
340344
}
341345

342346
// Get the workflow state that was just applied, merge subblocks, and sanitize
343347
// This matches what get_user_workflow would return (the true state after edits were applied)
344-
const workflowJson = this.getSanitizedWorkflowJson(actualDiffWorkflow)
348+
let workflowJson = this.getSanitizedWorkflowJson(actualDiffWorkflow)
349+
350+
// Fallback: try to get current workflow state if sanitization failed
351+
if (!workflowJson) {
352+
workflowJson = this.getCurrentWorkflowJsonSafe(logger)
353+
}
354+
355+
// userWorkflow must always be present on success - log error if missing
356+
if (!workflowJson) {
357+
logger.error('Failed to get workflow JSON on success path - this should not happen', {
358+
toolCallId: this.toolCallId,
359+
workflowId: this.workflowId,
360+
})
361+
}
345362

346363
// Build sanitized data including workflow JSON and any skipped/validation info
347-
const sanitizedData: Record<string, any> = {}
348-
if (workflowJson) {
349-
sanitizedData.userWorkflow = workflowJson
364+
// Always include userWorkflow on success paths
365+
const sanitizedData: Record<string, any> = {
366+
userWorkflow: workflowJson ?? '{}', // Fallback to empty object JSON if all else fails
350367
}
351368

352369
// Include skipped items and validation errors in the response for LLM feedback
@@ -372,21 +389,25 @@ export class EditWorkflowClientTool extends BaseClientTool {
372389
completeMessage = `Workflow diff ready for review. Note: ${parts.join(', ')}.`
373390
}
374391

375-
// Mark complete early to unblock LLM stream
376-
await this.markToolComplete(
377-
200,
378-
completeMessage,
379-
Object.keys(sanitizedData).length > 0 ? sanitizedData : undefined
380-
)
392+
// Mark complete early to unblock LLM stream - sanitizedData always has userWorkflow
393+
await this.markToolComplete(200, completeMessage, sanitizedData)
381394

382395
// Move into review state
383396
this.setState(ClientToolCallState.review, { result })
384397
} catch (fetchError: any) {
385398
clearTimeout(fetchTimeout)
386-
if (fetchError.name === 'AbortError') {
387-
throw new Error('Server request timed out')
388-
}
389-
throw fetchError
399+
// Handle error with current workflow state
400+
this.setState(ClientToolCallState.error)
401+
const currentWorkflowJson = this.getCurrentWorkflowJsonSafe(logger)
402+
const errorMessage =
403+
fetchError.name === 'AbortError'
404+
? 'Server request timed out'
405+
: fetchError.message || String(fetchError)
406+
await this.markToolComplete(
407+
500,
408+
errorMessage,
409+
currentWorkflowJson ? { userWorkflow: currentWorkflowJson } : undefined
410+
)
390411
}
391412
})
392413
}

0 commit comments

Comments
 (0)