-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(tools): fixed workflow tool for agent to respect user provided params, inject at runtime like all other tools #2750
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
…rams, inject at runtime like all other tools
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
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.
Greptile Overview
Greptile Summary
Changes Overview
This PR addresses two main issues:
- Help Ticket Enhancement: Adds contextual information (workflowId, workspaceId, userAgent) to help ticket emails to improve support debugging
- Workflow Tool Parameter Fix: Fixes the workflow_executor tool to properly respect user-provided parameters by implementing deep merge logic for inputMapping
Key Implementation Details
Help Ticket Context (3 files changed)
- Modified
help-modal.tsxto accept and passworkflowIdandworkspaceIdprops - Updated
sidebar.tsxto provide these context values from URL params - Enhanced
route.tsAPI to extract and include these fields in support emails
Workflow Tool Parameter Handling (4 files changed)
- Created
deepMergeInputMapping()function intools/params.tsfor intelligent parameter merging - Updated
prepareToolExecution()inproviders/utils.tsto use deep merge for inputMapping - Added
createLLMToolSchema()special handling to always include inputMapping for workflow_executor - Extensive test coverage added for all new functionality
Problem Solved
Previously, the workflow_executor tool treated inputMapping as a regular parameter, so if a user provided an empty object {} or partial values in the UI, the LLM wouldn't be able to fill in dynamic values at runtime. The fix implements a deep merge strategy where:
- User-provided non-empty values take precedence
- LLM fills in missing or empty fields
- This allows hybrid user/LLM parameter configuration
Issues Found
- Critical:
workspaceIdin email template doesn't handle null case (missing?? 'N/A') - Logic Bug:
deepMergeInputMapping()incorrectly treats0andfalseas empty values, which would prevent users from explicitly setting these valid values
Confidence Score: 3/5
- This PR is generally safe to merge but has two logic bugs that need fixing
- Score of 3 reflects two important issues: (1) workspaceId can display "null" in emails due to missing null handling, and (2) deepMergeInputMapping incorrectly treats 0/false as empty, preventing users from setting these valid values. The core functionality is well-tested and sound, but these edge cases need addressing before merge.
- apps/sim/app/api/help/route.ts needs null handling for workspaceId; apps/sim/tools/params.ts needs to properly handle 0 and false values in deepMergeInputMapping
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/app/api/help/route.ts | 3/5 | Adds workflowId, workspaceId, and userAgent to help ticket emails; workspaceId missing null handling |
| apps/sim/providers/utils.ts | 4/5 | Implements special deep merge handling for inputMapping in prepareToolExecution |
| apps/sim/tools/params.ts | 3/5 | Adds deepMergeInputMapping function and updates mergeToolParameters; edge case with 0/false values |
Sequence Diagram
sequenceDiagram
participant User
participant HelpModal
participant HelpAPI
participant Email
participant Agent
participant WorkflowTool
participant PrepareToolExec
participant DeepMerge
Note over User,Email: Help Ticket Flow (New)
User->>HelpModal: Opens help modal
HelpModal->>HelpModal: Captures workflowId, workspaceId from context
User->>HelpModal: Fills form and submits
HelpModal->>HelpAPI: POST /api/help with formData
Note right of HelpModal: Includes: subject, message, type<br/>workflowId, workspaceId, userAgent
HelpAPI->>HelpAPI: Extract and validate fields
HelpAPI->>Email: Send email with context info
Note right of Email: Email now includes:<br/>workflowId, workspaceId, userId, userAgent
Email-->>User: Confirmation email sent
Note over Agent,DeepMerge: Workflow Tool Parameter Flow (Fixed)
Agent->>WorkflowTool: Uses workflow_executor tool
Agent->>WorkflowTool: Provides llmArgs (inputMapping)
Note right of Agent: User may have provided partial<br/>or empty inputMapping in UI
WorkflowTool->>PrepareToolExec: prepareToolExecution(tool, llmArgs, request)
PrepareToolExec->>PrepareToolExec: Filter empty user params
PrepareToolExec->>PrepareToolExec: Check if param is inputMapping
PrepareToolExec->>DeepMerge: deepMergeInputMapping(llm, user)
DeepMerge->>DeepMerge: Parse user JSON string if needed
DeepMerge->>DeepMerge: Merge: LLM values as base
DeepMerge->>DeepMerge: Override with non-empty user values
DeepMerge-->>PrepareToolExec: Merged inputMapping
PrepareToolExec-->>WorkflowTool: toolParams with merged inputMapping
WorkflowTool->>WorkflowTool: Execute workflow with merged params
Greptile OverviewGreptile SummaryFixes workflow_executor tool to properly respect user-provided parameters by always including inputMapping in the LLM schema and implementing deep merge logic. Previously, any user interaction with inputMapping fields would prevent the LLM from filling empty values, but now the LLM can dynamically populate fields the user left blank while preserving explicitly set values. Additionally enhances help ticket emails by including workflowId, workspaceId, and userAgent for better support context. Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant UI as Workflow UI
participant Schema as createLLMToolSchema
participant LLM as Language Model
participant Merge as mergeToolParameters
participant Exec as prepareToolExecution
participant Tool as Workflow Executor
Note over UI,Tool: User configures workflow with empty inputMapping fields
UI->>Schema: Generate schema for LLM<br/>userParams: {workflowId, inputMapping: {query: ""}}
Note over Schema: Special handling for workflow_executor
Schema-->>Schema: Always include inputMapping in schema<br/>(even if user touched it)
Schema->>LLM: Schema with inputMapping included
LLM->>Merge: Returns params<br/>{workflowId, inputMapping: {query: "llm-value", limit: 10}}
Note over Merge: Deep merge inputMapping
Merge-->>Merge: User empty values + LLM values<br/>→ {query: "llm-value", limit: 10}
Merge->>Exec: Merged params
Note over Exec: Apply same deep merge logic
Exec-->>Exec: Filter empty user params<br/>Deep merge inputMapping again
Exec->>Tool: Final params with complete inputMapping
|
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.
5 files reviewed, 5 comments
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.
4 files reviewed, 4 comments
Summary
Type of Change
Testing
Tested manually.
Checklist