Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Greptile Summary
This PR fixes inconsistent UI behavior in the chat interface's tool management system by adding defensive logic to coordinate between two separate toggle states: disabled/enabled and forced/unforced. The change addresses a scenario where tools could exist in a contradictory state of being both "disabled" and "forced" simultaneously.
The implementation adds identical logic in two components (MCPToolsList and ActionToggle) within the toggleToolForCurrentAssistant function. When a user disables a tool that is currently in the "forced" state, the system now automatically removes that tool from the forcedToolIds array. This prevents the logical inconsistency where a forced tool (which should always be available) could also be disabled.
The fix integrates with the existing tool management system in the chat interface, which allows users to both enable/disable individual tools and separately designate certain tools as "forced" (always active). By adding this coordination logic, the UI maintains consistent state relationships and provides a more intuitive user experience when managing tool availability in chat sessions.
Confidence score: 4/5
- This PR is safe to merge with minimal risk of breaking existing functionality
- Score reflects straightforward defensive logic that prevents UI inconsistencies without modifying core business logic
- Pay close attention to the ActionManagement.tsx file to ensure the logic correctly handles edge cases in tool state management
1 file reviewed, no comments
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="web/src/app/chat/components/input/ActionManagement.tsx">
<violation number="1" location="web/src/app/chat/components/input/ActionManagement.tsx:274">
Use functional state update to avoid stale state when deriving next value from previous (setForcedToolIds should use prev => prev.filter(...)).</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
|
|
||
| // If we're disabling a tool that is currently forced, remove it from forced tools | ||
| if (!disabled && forcedToolIds.includes(toolId)) { | ||
| setForcedToolIds(forcedToolIds.filter((id) => id !== toolId)); |
There was a problem hiding this comment.
Use functional state update to avoid stale state when deriving next value from previous (setForcedToolIds should use prev => prev.filter(...)).
Prompt for AI agents
Address the following comment on web/src/app/chat/components/input/ActionManagement.tsx at line 274:
<comment>Use functional state update to avoid stale state when deriving next value from previous (setForcedToolIds should use prev => prev.filter(...)).</comment>
<file context>
@@ -268,6 +268,11 @@ function MCPToolsList({
+
+ // If we're disabling a tool that is currently forced, remove it from forced tools
+ if (!disabled && forcedToolIds.includes(toolId)) {
+ setForcedToolIds(forcedToolIds.filter((id) => id !== toolId));
+ }
};
</file context>
| setForcedToolIds(forcedToolIds.filter((id) => id !== toolId)); | |
| setForcedToolIds((prev) => prev.filter((id) => id !== toolId)); |
Description
Unintuitive toggle behavior now fixed
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Backporting (check the box to trigger backport action)
Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.
Summary by cubic
Fix toggle behavior so disabling a tool automatically clears it from forced tools, preventing contradictory states in MCPToolsList and ActionToggle. Users can no longer have a tool both disabled and forced; toggles now stay in sync.