Skip to content

Commit 1f5e8a4

Browse files
authored
fix(tools): fixed workflow tool for agent to respect user provided params, inject at runtime like all other tools (#2750)
* fix(tools): fixed wrokflow tool for agent to respect user provided params, inject at runtime like all other tools * ack comments * remove redunant if-else * added tests
1 parent 796f73e commit 1f5e8a4

File tree

7 files changed

+630
-42
lines changed

7 files changed

+630
-42
lines changed

apps/sim/app/api/help/route.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ export async function POST(req: NextRequest) {
2121
const requestId = generateRequestId()
2222

2323
try {
24-
// Get user session
2524
const session = await getSession()
2625
if (!session?.user?.email) {
2726
logger.warn(`[${requestId}] Unauthorized help request attempt`)
@@ -30,20 +29,20 @@ export async function POST(req: NextRequest) {
3029

3130
const email = session.user.email
3231

33-
// Handle multipart form data
3432
const formData = await req.formData()
3533

36-
// Extract form fields
3734
const subject = formData.get('subject') as string
3835
const message = formData.get('message') as string
3936
const type = formData.get('type') as string
37+
const workflowId = formData.get('workflowId') as string | null
38+
const workspaceId = formData.get('workspaceId') as string
39+
const userAgent = formData.get('userAgent') as string | null
4040

4141
logger.info(`[${requestId}] Processing help request`, {
4242
type,
4343
email: `${email.substring(0, 3)}***`, // Log partial email for privacy
4444
})
4545

46-
// Validate the form data
4746
const validationResult = helpFormSchema.safeParse({
4847
subject,
4948
message,
@@ -60,7 +59,6 @@ export async function POST(req: NextRequest) {
6059
)
6160
}
6261

63-
// Extract images
6462
const images: { filename: string; content: Buffer; contentType: string }[] = []
6563

6664
for (const [key, value] of formData.entries()) {
@@ -81,10 +79,14 @@ export async function POST(req: NextRequest) {
8179

8280
logger.debug(`[${requestId}] Help request includes ${images.length} images`)
8381

84-
// Prepare email content
82+
const userId = session.user.id
8583
let emailText = `
8684
Type: ${type}
8785
From: ${email}
86+
User ID: ${userId}
87+
Workspace ID: ${workspaceId ?? 'N/A'}
88+
Workflow ID: ${workflowId ?? 'N/A'}
89+
Browser: ${userAgent ?? 'N/A'}
8890
8991
${message}
9092
`
@@ -115,7 +117,6 @@ ${message}
115117

116118
logger.info(`[${requestId}] Help request email sent successfully`)
117119

118-
// Send confirmation email to the user
119120
try {
120121
const confirmationHtml = await renderHelpConfirmationEmail(
121122
type as 'bug' | 'feedback' | 'feature_request' | 'other',

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/help-modal/help-modal.tsx

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,11 @@ interface ImageWithPreview extends File {
5757
interface HelpModalProps {
5858
open: boolean
5959
onOpenChange: (open: boolean) => void
60+
workflowId?: string
61+
workspaceId: string
6062
}
6163

62-
export function HelpModal({ open, onOpenChange }: HelpModalProps) {
64+
export function HelpModal({ open, onOpenChange, workflowId, workspaceId }: HelpModalProps) {
6365
const fileInputRef = useRef<HTMLInputElement>(null)
6466
const scrollContainerRef = useRef<HTMLDivElement>(null)
6567

@@ -370,18 +372,20 @@ export function HelpModal({ open, onOpenChange }: HelpModalProps) {
370372
setSubmitStatus(null)
371373

372374
try {
373-
// Prepare form data with images
374375
const formData = new FormData()
375376
formData.append('subject', data.subject)
376377
formData.append('message', data.message)
377378
formData.append('type', data.type)
379+
formData.append('workspaceId', workspaceId)
380+
formData.append('userAgent', navigator.userAgent)
381+
if (workflowId) {
382+
formData.append('workflowId', workflowId)
383+
}
378384

379-
// Attach all images to form data
380385
images.forEach((image, index) => {
381386
formData.append(`image_${index}`, image)
382387
})
383388

384-
// Submit to API
385389
const response = await fetch('/api/help', {
386390
method: 'POST',
387391
body: formData,
@@ -392,11 +396,9 @@ export function HelpModal({ open, onOpenChange }: HelpModalProps) {
392396
throw new Error(errorData.error || 'Failed to submit help request')
393397
}
394398

395-
// Handle success
396399
setSubmitStatus('success')
397400
reset()
398401

399-
// Clean up resources
400402
images.forEach((image) => URL.revokeObjectURL(image.preview))
401403
setImages([])
402404
} catch (error) {
@@ -406,7 +408,7 @@ export function HelpModal({ open, onOpenChange }: HelpModalProps) {
406408
setIsSubmitting(false)
407409
}
408410
},
409-
[images, reset]
411+
[images, reset, workflowId, workspaceId]
410412
)
411413

412414
/**

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,12 @@ export function Sidebar() {
661661
/>
662662

663663
{/* Footer Navigation Modals */}
664-
<HelpModal open={isHelpModalOpen} onOpenChange={setIsHelpModalOpen} />
664+
<HelpModal
665+
open={isHelpModalOpen}
666+
onOpenChange={setIsHelpModalOpen}
667+
workflowId={workflowId}
668+
workspaceId={workspaceId}
669+
/>
665670
<SettingsModal
666671
open={isSettingsModalOpen}
667672
onOpenChange={(open) => (open ? openSettingsModal() : closeSettingsModal())}

apps/sim/providers/utils.test.ts

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
MODELS_WITH_TEMPERATURE_SUPPORT,
2626
MODELS_WITH_VERBOSITY,
2727
PROVIDERS_WITH_TOOL_USAGE_CONTROL,
28+
prepareToolExecution,
2829
prepareToolsWithUsageControl,
2930
shouldBillModelUsage,
3031
supportsTemperature,
@@ -979,6 +980,245 @@ describe('Tool Management', () => {
979980
})
980981
})
981982

983+
describe('prepareToolExecution', () => {
984+
describe('basic parameter merging', () => {
985+
it.concurrent('should merge LLM args with user params', () => {
986+
const tool = {
987+
params: { apiKey: 'user-key', channel: '#general' },
988+
}
989+
const llmArgs = { message: 'Hello world', channel: '#random' }
990+
const request = { workflowId: 'wf-123' }
991+
992+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
993+
994+
expect(toolParams.apiKey).toBe('user-key')
995+
expect(toolParams.channel).toBe('#general') // User value wins
996+
expect(toolParams.message).toBe('Hello world')
997+
})
998+
999+
it.concurrent('should filter out empty string user params', () => {
1000+
const tool = {
1001+
params: { apiKey: 'user-key', channel: '' }, // Empty channel
1002+
}
1003+
const llmArgs = { message: 'Hello', channel: '#llm-channel' }
1004+
const request = {}
1005+
1006+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1007+
1008+
expect(toolParams.apiKey).toBe('user-key')
1009+
expect(toolParams.channel).toBe('#llm-channel') // LLM value used since user is empty
1010+
expect(toolParams.message).toBe('Hello')
1011+
})
1012+
})
1013+
1014+
describe('inputMapping deep merge for workflow tools', () => {
1015+
it.concurrent('should deep merge inputMapping when user provides empty object', () => {
1016+
const tool = {
1017+
params: {
1018+
workflowId: 'child-workflow-123',
1019+
inputMapping: '{}', // Empty JSON string from UI
1020+
},
1021+
}
1022+
const llmArgs = {
1023+
inputMapping: { query: 'search term', limit: 10 },
1024+
}
1025+
const request = { workflowId: 'parent-workflow' }
1026+
1027+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1028+
1029+
// LLM values should be used since user object is empty
1030+
expect(toolParams.inputMapping).toEqual({ query: 'search term', limit: 10 })
1031+
expect(toolParams.workflowId).toBe('child-workflow-123')
1032+
})
1033+
1034+
it.concurrent('should deep merge inputMapping with partial user values', () => {
1035+
const tool = {
1036+
params: {
1037+
workflowId: 'child-workflow',
1038+
inputMapping: '{"query": "", "customField": "user-value"}', // Partial values
1039+
},
1040+
}
1041+
const llmArgs = {
1042+
inputMapping: { query: 'llm-search', limit: 10 },
1043+
}
1044+
const request = {}
1045+
1046+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1047+
1048+
// LLM fills empty query, user's customField preserved, LLM's limit included
1049+
expect(toolParams.inputMapping).toEqual({
1050+
query: 'llm-search',
1051+
limit: 10,
1052+
customField: 'user-value',
1053+
})
1054+
})
1055+
1056+
it.concurrent('should preserve non-empty user inputMapping values', () => {
1057+
const tool = {
1058+
params: {
1059+
workflowId: 'child-workflow',
1060+
inputMapping: '{"query": "user-search", "limit": 5}',
1061+
},
1062+
}
1063+
const llmArgs = {
1064+
inputMapping: { query: 'llm-search', limit: 10, extra: 'field' },
1065+
}
1066+
const request = {}
1067+
1068+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1069+
1070+
// User values win, but LLM's extra field is included
1071+
expect(toolParams.inputMapping).toEqual({
1072+
query: 'user-search',
1073+
limit: 5,
1074+
extra: 'field',
1075+
})
1076+
})
1077+
1078+
it.concurrent('should handle inputMapping as object (not JSON string)', () => {
1079+
const tool = {
1080+
params: {
1081+
workflowId: 'child-workflow',
1082+
inputMapping: { query: '', customField: 'user-value' }, // Object, not string
1083+
},
1084+
}
1085+
const llmArgs = {
1086+
inputMapping: { query: 'llm-search', limit: 10 },
1087+
}
1088+
const request = {}
1089+
1090+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1091+
1092+
expect(toolParams.inputMapping).toEqual({
1093+
query: 'llm-search',
1094+
limit: 10,
1095+
customField: 'user-value',
1096+
})
1097+
})
1098+
1099+
it.concurrent('should use LLM inputMapping when user does not provide it', () => {
1100+
const tool = {
1101+
params: { workflowId: 'child-workflow' }, // No inputMapping
1102+
}
1103+
const llmArgs = {
1104+
inputMapping: { query: 'llm-search', limit: 10 },
1105+
}
1106+
const request = {}
1107+
1108+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1109+
1110+
expect(toolParams.inputMapping).toEqual({ query: 'llm-search', limit: 10 })
1111+
})
1112+
1113+
it.concurrent('should use user inputMapping when LLM does not provide it', () => {
1114+
const tool = {
1115+
params: {
1116+
workflowId: 'child-workflow',
1117+
inputMapping: '{"query": "user-search"}',
1118+
},
1119+
}
1120+
const llmArgs = {} // No inputMapping from LLM
1121+
const request = {}
1122+
1123+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1124+
1125+
expect(toolParams.inputMapping).toEqual({ query: 'user-search' })
1126+
})
1127+
1128+
it.concurrent('should handle invalid JSON in user inputMapping gracefully', () => {
1129+
const tool = {
1130+
params: {
1131+
workflowId: 'child-workflow',
1132+
inputMapping: 'not valid json {',
1133+
},
1134+
}
1135+
const llmArgs = {
1136+
inputMapping: { query: 'llm-search' },
1137+
}
1138+
const request = {}
1139+
1140+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1141+
1142+
// Should use LLM values since user JSON is invalid
1143+
expect(toolParams.inputMapping).toEqual({ query: 'llm-search' })
1144+
})
1145+
1146+
it.concurrent('should not affect other parameters - normal override behavior', () => {
1147+
const tool = {
1148+
params: { apiKey: 'user-key', channel: '#general' },
1149+
}
1150+
const llmArgs = { message: 'Hello', channel: '#random' }
1151+
const request = {}
1152+
1153+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1154+
1155+
// Normal behavior: user values override LLM values
1156+
expect(toolParams.apiKey).toBe('user-key')
1157+
expect(toolParams.channel).toBe('#general') // User value wins
1158+
expect(toolParams.message).toBe('Hello')
1159+
})
1160+
1161+
it.concurrent('should preserve 0 and false as valid user values in inputMapping', () => {
1162+
const tool = {
1163+
params: {
1164+
workflowId: 'child-workflow',
1165+
inputMapping: '{"limit": 0, "enabled": false, "query": ""}',
1166+
},
1167+
}
1168+
const llmArgs = {
1169+
inputMapping: { limit: 10, enabled: true, query: 'llm-search' },
1170+
}
1171+
const request = {}
1172+
1173+
const { toolParams } = prepareToolExecution(tool, llmArgs, request)
1174+
1175+
// 0 and false should be preserved (they're valid values)
1176+
// empty string should be filled by LLM
1177+
expect(toolParams.inputMapping).toEqual({
1178+
limit: 0,
1179+
enabled: false,
1180+
query: 'llm-search',
1181+
})
1182+
})
1183+
})
1184+
1185+
describe('execution params context', () => {
1186+
it.concurrent('should include workflow context in executionParams', () => {
1187+
const tool = { params: { message: 'test' } }
1188+
const llmArgs = {}
1189+
const request = {
1190+
workflowId: 'wf-123',
1191+
workspaceId: 'ws-456',
1192+
chatId: 'chat-789',
1193+
userId: 'user-abc',
1194+
}
1195+
1196+
const { executionParams } = prepareToolExecution(tool, llmArgs, request)
1197+
1198+
expect(executionParams._context).toEqual({
1199+
workflowId: 'wf-123',
1200+
workspaceId: 'ws-456',
1201+
chatId: 'chat-789',
1202+
userId: 'user-abc',
1203+
})
1204+
})
1205+
1206+
it.concurrent('should include environment and workflow variables', () => {
1207+
const tool = { params: {} }
1208+
const llmArgs = {}
1209+
const request = {
1210+
environmentVariables: { API_KEY: 'secret' },
1211+
workflowVariables: { counter: 42 },
1212+
}
1213+
1214+
const { executionParams } = prepareToolExecution(tool, llmArgs, request)
1215+
1216+
expect(executionParams.envVars).toEqual({ API_KEY: 'secret' })
1217+
expect(executionParams.workflowVariables).toEqual({ counter: 42 })
1218+
})
1219+
})
1220+
})
1221+
9821222
describe('Provider/Model Blacklist', () => {
9831223
describe('isProviderBlacklisted', () => {
9841224
it.concurrent('should return false when no providers are blacklisted', () => {

0 commit comments

Comments
 (0)