-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(copilot): fix function execute tool #2222
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile OverviewGreptile SummaryThis PR fixes multiple issues with the copilot function execute tool by improving the tool call handling component. The main changes address race conditions in button interactions, unreliable backend notifications for skipped tools, inconsistent state naming, and poor UI feedback during tool execution. The fix implements race condition prevention using React refs to prevent multiple simultaneous button clicks, adds retry logic for backend notifications when tools are skipped (preventing agent hangs), standardizes state naming from 'errored' to 'error', and provides immediate UI feedback when tools start executing. Additionally, it improves auto-allowed tool management for This change integrates with the existing copilot architecture by enhancing the tool call component that manages user interactions with AI agent tools, ensuring reliable state synchronization between frontend and backend for integration tools that require server-side execution. Important Files Changed
Confidence score: 4/5
Sequence DiagramsequenceDiagram
participant User
participant ToolCall as "ToolCall Component"
participant CopilotStore as "Copilot Store"
participant Backend as "Backend API"
participant ClientTool as "Client Tool"
User->>ToolCall: "Interacts with tool call UI"
ToolCall->>CopilotStore: "Get tool call state"
CopilotStore-->>ToolCall: "Return tool call data"
alt Tool is expandable and pending
ToolCall->>ToolCall: "Set expanded state to true"
end
User->>ToolCall: "Clicks Run/Allow button"
ToolCall->>ToolCall: "Set processing state"
alt Integration tool (server-side)
ToolCall->>CopilotStore: "setToolCallState('executing')"
ToolCall->>CopilotStore: "executeIntegrationTool(toolCallId)"
CopilotStore->>Backend: "Execute integration tool"
Backend-->>CopilotStore: "Tool execution result"
alt Execution fails
ToolCall->>Backend: "POST /api/copilot/tools/mark-complete (error)"
end
else Client tool
ToolCall->>ClientTool: "handleAccept(editedParams)"
ClientTool-->>ToolCall: "Tool execution result"
end
User->>ToolCall: "Clicks Always Allow button"
ToolCall->>CopilotStore: "addAutoAllowedTool(toolName)"
ToolCall->>ToolCall: "Execute tool (same as Run flow)"
User->>ToolCall: "Clicks Skip button"
alt Integration tool
ToolCall->>CopilotStore: "setToolCallState('rejected')"
ToolCall->>Backend: "POST /api/copilot/tools/mark-complete (skipped)"
else Client tool
ToolCall->>ClientTool: "handleReject()"
ToolCall->>CopilotStore: "setToolCallState('rejected')"
end
User->>ToolCall: "Edits parameters"
ToolCall->>ToolCall: "setEditedParams(newParams)"
User->>ToolCall: "Clicks Move to Background"
ToolCall->>ClientTool: "setState('background')"
ToolCall->>ClientTool: "markToolComplete(200, message)"
|
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.
1 file reviewed, 2 comments
| }) | ||
| } catch { | ||
| // Last resort: log error if we can't notify backend | ||
| console.error('[handleRun] Failed to notify backend of tool error:', toolCall.id) |
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.
style: Using console.error violates the logging rules - should use logger.error instead
| console.error('[handleRun] Failed to notify backend of tool error:', toolCall.id) | |
| logger.error('[handleRun] Failed to notify backend of tool error:', toolCall.id) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/tool-call/tool-call.tsx
Line: 321:321
Comment:
**style:** Using `console.error` violates the logging rules - should use logger.error instead
```suggestion
logger.error('[handleRun] Failed to notify backend of tool error:', toolCall.id)
```
How can I resolve this? If you propose a fix, please make it concise.| } | ||
|
|
||
| if (!notified) { | ||
| console.error('[handleSkip] Failed to notify backend after 3 attempts:', toolCall.id) |
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.
style: Using console.error violates the logging rules - should use logger.error instead
| console.error('[handleSkip] Failed to notify backend after 3 attempts:', toolCall.id) | |
| logger.error('[handleSkip] Failed to notify backend after 3 attempts:', toolCall.id) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/tool-call/tool-call.tsx
Line: 378:378
Comment:
**style:** Using `console.error` violates the logging rules - should use logger.error instead
```suggestion
logger.error('[handleSkip] Failed to notify backend after 3 attempts:', toolCall.id)
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
Fix copilot function execute tool
Type of Change
Testing
Manual
Checklist