Skip to content

improvement(copilot): tool dependency errors show as skipped #864

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.

Sign up for GitHub

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

Merged
Sg312 merged 3 commits into from
Aug 4, 2025
Merged

improvement(copilot): tool dependency errors show as skipped #864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
133c5a7
Dependency errors show as skipped instead of errored
Sg312 Aug 4, 2025
d4e0bf3
Update apps/sim/lib/copilot/tools/client-tools/run-workflow.ts
Sg312 Aug 4, 2025
a39b04c
Lint
Sg312 Aug 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions apps/sim/lib/copilot/tools/client-tools/run-workflow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,15 @@ export class RunWorkflowTool extends BaseTool {
// Execution failed
const errorMessage = (result as any)?.error || 'Workflow execution failed'

// Notify server of error
await this.notify(toolCall.id, 'errored', `Workflow execution failed: ${errorMessage}`)
// Check if error message is exactly 'skipped' to notify 'rejected' instead of 'errored'
const targetState = errorMessage === 'skipped' ? 'rejected' : 'errored'
const message =
targetState === 'rejected'
? `Workflow execution skipped: ${errorMessage}`
: `Workflow execution failed: ${errorMessage}`
await this.notify(toolCall.id, targetState, message)

options?.onStateChange?.('errored')
options?.onStateChange?.(targetState)

return {
success: false,
Expand All @@ -184,9 +189,11 @@ export class RunWorkflowTool extends BaseTool {

const errorMessage = error?.message || 'An unknown error occurred'

await this.notify(toolCall.id, 'errored', `Workflow execution failed: ${errorMessage}`)
// Check if error message is exactly 'skipped' to notify 'rejected' instead of 'errored'
const targetState = errorMessage === 'skipped' ? 'rejected' : 'errored'
await this.notify(toolCall.id, targetState, `Workflow execution failed: ${errorMessage}`)

options?.onStateChange?.('errored')
options?.onStateChange?.(targetState)

return {
success: false,
Expand Down
7 changes: 5 additions & 2 deletions apps/sim/lib/copilot/tools/inline-tool-call.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,11 @@ async function clientAcceptTool(
}
} catch (error) {
console.error('Error executing client tool:', error)
setToolCallState(toolCall, 'errored', {
error: error instanceof Error ? error.message : 'Tool execution failed',
const errorMessage = error instanceof Error ? error.message : 'Tool execution failed'
// Check if error message is exactly 'skipped' to set 'rejected' state instead of 'errored'
const targetState = errorMessage === 'skipped' ? 'rejected' : 'errored'
setToolCallState(toolCall, targetState, {
error: errorMessage,
})
}
}
Expand Down
50 changes: 12 additions & 38 deletions apps/sim/stores/copilot/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,6 @@ function getToolDisplayNameByState(toolCall: any): string {
* don't go through createToolCall()
*/
function ensureToolCallDisplayNames(messages: CopilotMessage[]): CopilotMessage[] {
console.log('[DEBUG] ensureToolCallDisplayNames called, processing', messages.length, 'messages')

return messages.map((message: CopilotMessage) => {
if (message.role === 'assistant' && (message.toolCalls || message.contentBlocks)) {
// Check for workflow tools before recalculating
Expand All @@ -307,11 +305,6 @@ function ensureToolCallDisplayNames(messages: CopilotMessage[]): CopilotMessage[
)

if (hasWorkflowTools) {
console.log(
'[DEBUG] ensureToolCallDisplayNames found workflow tools in message:',
message.id
)

// Log current states before recalculation
const workflowToolStates = message.contentBlocks
?.filter(
Expand All @@ -325,11 +318,6 @@ function ensureToolCallDisplayNames(messages: CopilotMessage[]): CopilotMessage[
state: (b as any).toolCall.state,
displayName: (b as any).toolCall.displayName,
}))

console.log(
'[DEBUG] Current workflow tool states before recalculation:',
workflowToolStates
)
}

return {
Expand Down Expand Up @@ -408,7 +396,8 @@ function handleToolFailure(toolCall: any, error: string): void {
return
}

toolCall.state = 'errored'
// Check if error message is exactly 'skipped' to set 'rejected' state instead of 'errored'
toolCall.state = error === 'skipped' ? 'rejected' : 'errored'
toolCall.error = error

// Update displayName to match the error state
Expand Down Expand Up @@ -488,7 +477,12 @@ function setToolCallState(
break

case 'rejected':
// User rejected the tool
// User rejected the tool or tool was skipped
toolCall.endTime = Date.now()
toolCall.duration = toolCall.endTime - toolCall.startTime
if (error) {
toolCall.error = error
}
break

case 'background':
Expand Down Expand Up @@ -687,7 +681,7 @@ const sseHandlers: Record<string, SSEHandler> = {

// Handle tool result events - simplified
tool_result: (data, context, get, set) => {
const { toolCallId, result, success } = data
const { toolCallId, result, success, error } = data

if (!toolCallId) return

Expand Down Expand Up @@ -729,15 +723,11 @@ const sseHandlers: Record<string, SSEHandler> = {
if (toolSupportsReadyForReview(toolCall.name)) {
processWorkflowToolResult(toolCall, parsedResult, get)
}

// COMMENTED OUT OLD LOGIC:
// finalizeToolCall(toolCall, true, parsedResult, get)
} else {
// NEW LOGIC: Use centralized state management
setToolCallState(toolCall, 'errored', { error: result || 'Tool execution failed' })
const errorMessage = error || result || 'Tool execution failed'
const targetState = errorMessage === 'skipped' ? 'rejected' : 'errored'

// COMMENTED OUT OLD LOGIC:
// handleToolFailure(toolCall, result || 'Tool execution failed')
setToolCallState(toolCall, targetState, { error: errorMessage })
}

// Update both contentBlocks and toolCalls atomically before UI update
Expand Down Expand Up @@ -792,24 +782,13 @@ const sseHandlers: Record<string, SSEHandler> = {
const toolData = data.data
if (!toolData) return

// Log the raw tool data from LLM stream
if (toolData.name === 'run_workflow') {
console.log('🔍 LLM tool call data:', JSON.stringify(toolData, null, 2))
console.log('🔍 LLM arguments field:', JSON.stringify(toolData.arguments, null, 2))
}

// Check if tool call already exists
const existingToolCall = context.toolCalls.find((tc) => tc.id === toolData.id)

if (existingToolCall) {
// Update existing tool call with new arguments (for partial -> complete transition)
existingToolCall.input = toolData.arguments || {}

// Log the updated tool call
if (toolData.name === 'run_workflow') {
console.log('🔍 Updated existing tool call:', JSON.stringify(existingToolCall, null, 2))
}

// Update the content block as well
updateContentBlockToolCall(context.contentBlocks, toolData.id, existingToolCall)
updateStreamingMessage(set, context)
Expand All @@ -818,11 +797,6 @@ const sseHandlers: Record<string, SSEHandler> = {

const toolCall = createToolCall(toolData.id, toolData.name, toolData.arguments)

// Log the created tool call
if (toolData.name === 'run_workflow') {
console.log('🔍 Created tool call:', JSON.stringify(toolCall, null, 2))
}

context.toolCalls.push(toolCall)

context.contentBlocks.push({
Expand Down