Skip to content

Commit 76dd4a0

Browse files
committed
Fix always allow, credential validation
1 parent 66dfe2c commit 76dd4a0

File tree

4 files changed

+169
-25
lines changed

4 files changed

+169
-25
lines changed

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/tool-call/tool-call.tsx

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,10 +1412,13 @@ function RunSkipButtons({
14121412
setIsProcessing(true)
14131413
setButtonsHidden(true)
14141414
try {
1415-
// Add to auto-allowed list first
1415+
// Add to auto-allowed list - this also executes all pending integration tools of this type
14161416
await addAutoAllowedTool(toolCall.name)
1417-
// Then execute
1418-
await handleRun(toolCall, setToolCallState, onStateChange, editedParams)
1417+
// For client tools with interrupts (not integration tools), we still need to call handleRun
1418+
// since executeIntegrationTool only works for server-side tools
1419+
if (!isIntegrationTool(toolCall.name)) {
1420+
await handleRun(toolCall, setToolCallState, onStateChange, editedParams)
1421+
}
14191422
} finally {
14201423
setIsProcessing(false)
14211424
actionInProgressRef.current = false
@@ -1438,10 +1441,10 @@ function RunSkipButtons({
14381441

14391442
if (buttonsHidden) return null
14401443

1441-
// Hide "Always Allow" for integration tools (only show for client tools with interrupts)
1442-
const showAlwaysAllow = !isIntegrationTool(toolCall.name)
1444+
// Show "Always Allow" for all tools that require confirmation
1445+
const showAlwaysAllow = true
14431446

1444-
// Standardized buttons for all interrupt tools: Allow, (Always Allow for client tools only), Skip
1447+
// Standardized buttons for all interrupt tools: Allow, Always Allow, Skip
14451448
return (
14461449
<div className='mt-[10px] flex gap-[6px]'>
14471450
<Button onClick={onRun} disabled={isProcessing} variant='tertiary'>

apps/sim/lib/copilot/tools/server/workflow/edit-workflow.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2468,16 +2468,17 @@ async function validateWorkflowSelectorIds(
24682468
const result = await validateSelectorIds(selector.selectorType, selector.value, context)
24692469

24702470
if (result.invalid.length > 0) {
2471+
// Include warning info (like available credentials) in the error message for better LLM feedback
2472+
const warningInfo = result.warning ? `. ${result.warning}` : ''
24712473
errors.push({
24722474
blockId: selector.blockId,
24732475
blockType: selector.blockType,
24742476
field: selector.fieldName,
24752477
value: selector.value,
2476-
error: `Invalid ${selector.selectorType} ID(s): ${result.invalid.join(', ')} - ID(s) do not exist`,
2478+
error: `Invalid ${selector.selectorType} ID(s): ${result.invalid.join(', ')} - ID(s) do not exist or user doesn't have access${warningInfo}`,
24772479
})
2478-
}
2479-
2480-
if (result.warning) {
2480+
} else if (result.warning) {
2481+
// Log warnings that don't have errors (shouldn't happen for credentials but may for other selectors)
24812482
logger.warn(result.warning, {
24822483
blockId: selector.blockId,
24832484
fieldName: selector.fieldName,

apps/sim/lib/copilot/validation/selector-validator.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,31 @@ export async function validateSelectorIds(
3939
.from(account)
4040
.where(and(inArray(account.id, idsArray), eq(account.userId, context.userId)))
4141
existingIds = results.map((r) => r.id)
42+
43+
// If any IDs are invalid, fetch user's available credentials to include in error message
44+
const existingSet = new Set(existingIds)
45+
const invalidIds = idsArray.filter((id) => !existingSet.has(id))
46+
if (invalidIds.length > 0) {
47+
// Fetch all of the user's credentials to provide helpful feedback
48+
const allUserCredentials = await db
49+
.select({ id: account.id, providerId: account.providerId })
50+
.from(account)
51+
.where(eq(account.userId, context.userId))
52+
53+
const availableCredentials = allUserCredentials
54+
.map((c) => `${c.id} (${c.providerId})`)
55+
.join(', ')
56+
const noCredentialsMessage = 'User has no credentials configured.'
57+
58+
return {
59+
valid: existingIds,
60+
invalid: invalidIds,
61+
warning:
62+
allUserCredentials.length > 0
63+
? `Available credentials for this user: ${availableCredentials}`
64+
: noCredentialsMessage,
65+
}
66+
}
4267
break
4368
}
4469

apps/sim/stores/panel/copilot/store.ts

Lines changed: 130 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,7 +1320,16 @@ const sseHandlers: Record<string, SSEHandler> = {
13201320
typeof def.hasInterrupt === 'function'
13211321
? !!def.hasInterrupt(args || {})
13221322
: !!def.hasInterrupt
1323-
if (!hasInterrupt && typeof def.execute === 'function') {
1323+
// Check if tool is auto-allowed - if so, execute even if it has an interrupt
1324+
const { autoAllowedTools } = get()
1325+
const isAutoAllowed = name ? autoAllowedTools.includes(name) : false
1326+
if ((!hasInterrupt || isAutoAllowed) && typeof def.execute === 'function') {
1327+
if (isAutoAllowed && hasInterrupt) {
1328+
logger.info('[toolCallsById] Auto-executing tool with interrupt (auto-allowed)', {
1329+
id,
1330+
name,
1331+
})
1332+
}
13241333
const ctx = createExecutionContext({ toolCallId: id, toolName: name || 'unknown_tool' })
13251334
// Defer executing transition by a tick to let pending render
13261335
setTimeout(() => {
@@ -1426,11 +1435,20 @@ const sseHandlers: Record<string, SSEHandler> = {
14261435
logger.warn('tool_call registry auto-exec check failed', { id, name, error: e })
14271436
}
14281437

1429-
// Class-based auto-exec for non-interrupt tools
1438+
// Class-based auto-exec for non-interrupt tools or auto-allowed tools
14301439
try {
14311440
const inst = getClientTool(id) as any
14321441
const hasInterrupt = !!inst?.getInterruptDisplays?.()
1433-
if (!hasInterrupt && typeof inst?.execute === 'function') {
1442+
// Check if tool is auto-allowed - if so, execute even if it has an interrupt
1443+
const { autoAllowedTools: classAutoAllowed } = get()
1444+
const isClassAutoAllowed = name ? classAutoAllowed.includes(name) : false
1445+
if ((!hasInterrupt || isClassAutoAllowed) && (typeof inst?.execute === 'function' || typeof inst?.handleAccept === 'function')) {
1446+
if (isClassAutoAllowed && hasInterrupt) {
1447+
logger.info('[toolCallsById] Auto-executing class tool with interrupt (auto-allowed)', {
1448+
id,
1449+
name,
1450+
})
1451+
}
14341452
setTimeout(() => {
14351453
// Guard against duplicate execution - check if already executing or terminal
14361454
const currentState = get().toolCallsById[id]?.state
@@ -1449,7 +1467,12 @@ const sseHandlers: Record<string, SSEHandler> = {
14491467

14501468
Promise.resolve()
14511469
.then(async () => {
1452-
await inst.execute(args || {})
1470+
// Use handleAccept for tools with interrupts, execute for others
1471+
if (hasInterrupt && typeof inst?.handleAccept === 'function') {
1472+
await inst.handleAccept(args || {})
1473+
} else {
1474+
await inst.execute(args || {})
1475+
}
14531476
// Success/error will be synced via registerToolStateSync
14541477
})
14551478
.catch(() => {
@@ -1474,20 +1497,35 @@ const sseHandlers: Record<string, SSEHandler> = {
14741497
}
14751498
} catch {}
14761499

1477-
// Integration tools: Stay in pending state until user confirms via buttons
1500+
// Integration tools: Check auto-allowed or stay in pending state until user confirms
14781501
// This handles tools like google_calendar_*, exa_*, gmail_read, etc. that aren't in the client registry
14791502
// Only relevant if mode is 'build' (agent)
1480-
const { mode, workflowId } = get()
1503+
const { mode, workflowId, autoAllowedTools, executeIntegrationTool } = get()
14811504
if (mode === 'build' && workflowId) {
14821505
// Check if tool was NOT found in client registry
14831506
const def = name ? getTool(name) : undefined
14841507
const inst = getClientTool(id) as any
14851508
if (!def && !inst && name) {
1486-
// Integration tools stay in pending state until user confirms
1487-
logger.info('[build mode] Integration tool awaiting user confirmation', {
1488-
id,
1489-
name,
1490-
})
1509+
// Check if this integration tool is auto-allowed - if so, execute it immediately
1510+
if (autoAllowedTools.includes(name)) {
1511+
logger.info('[build mode] Auto-executing integration tool (auto-allowed)', { id, name })
1512+
// Defer to allow pending state to render briefly
1513+
setTimeout(() => {
1514+
executeIntegrationTool(id).catch((err) => {
1515+
logger.error('[build mode] Auto-execute integration tool failed', {
1516+
id,
1517+
name,
1518+
error: err,
1519+
})
1520+
})
1521+
}, 0)
1522+
} else {
1523+
// Integration tools stay in pending state until user confirms
1524+
logger.info('[build mode] Integration tool awaiting user confirmation', {
1525+
id,
1526+
name,
1527+
})
1528+
}
14911529
}
14921530
}
14931531
},
@@ -1976,15 +2014,26 @@ const subAgentSSEHandlers: Record<string, SSEHandler> = {
19762014
}
19772015

19782016
// Execute client tools in parallel (non-blocking) - same pattern as main tool_call handler
2017+
// Check if tool is auto-allowed
2018+
const { autoAllowedTools: subAgentAutoAllowed } = get()
2019+
const isSubAgentAutoAllowed = name ? subAgentAutoAllowed.includes(name) : false
2020+
19792021
try {
19802022
const def = getTool(name)
19812023
if (def) {
19822024
const hasInterrupt =
19832025
typeof def.hasInterrupt === 'function'
19842026
? !!def.hasInterrupt(args || {})
19852027
: !!def.hasInterrupt
1986-
if (!hasInterrupt) {
1987-
// Auto-execute tools without interrupts - non-blocking
2028+
// Auto-execute if no interrupt OR if auto-allowed
2029+
if (!hasInterrupt || isSubAgentAutoAllowed) {
2030+
if (isSubAgentAutoAllowed && hasInterrupt) {
2031+
logger.info('[SubAgent] Auto-executing tool with interrupt (auto-allowed)', {
2032+
id,
2033+
name,
2034+
})
2035+
}
2036+
// Auto-execute tools - non-blocking
19882037
const ctx = createExecutionContext({ toolCallId: id, toolName: name })
19892038
Promise.resolve()
19902039
.then(() => def.execute(ctx, args || {}))
@@ -2001,9 +2050,22 @@ const subAgentSSEHandlers: Record<string, SSEHandler> = {
20012050
const instance = getClientTool(id)
20022051
if (instance) {
20032052
const hasInterruptDisplays = !!instance.getInterruptDisplays?.()
2004-
if (!hasInterruptDisplays) {
2053+
// Auto-execute if no interrupt OR if auto-allowed
2054+
if (!hasInterruptDisplays || isSubAgentAutoAllowed) {
2055+
if (isSubAgentAutoAllowed && hasInterruptDisplays) {
2056+
logger.info('[SubAgent] Auto-executing class tool with interrupt (auto-allowed)', {
2057+
id,
2058+
name,
2059+
})
2060+
}
20052061
Promise.resolve()
2006-
.then(() => instance.execute(args || {}))
2062+
.then(() => {
2063+
// Use handleAccept for tools with interrupts, execute for others
2064+
if (hasInterruptDisplays && typeof instance.handleAccept === 'function') {
2065+
return instance.handleAccept(args || {})
2066+
}
2067+
return instance.execute(args || {})
2068+
})
20072069
.catch((execErr: any) => {
20082070
logger.error('[SubAgent] Class tool execution failed', {
20092071
id,
@@ -3676,6 +3738,19 @@ export const useCopilotStore = create<CopilotStore>()(
36763738

36773739
const { id, name, params } = toolCall
36783740

3741+
// Guard against double execution - skip if already executing or in terminal state
3742+
if (
3743+
toolCall.state === ClientToolCallState.executing ||
3744+
isTerminalState(toolCall.state)
3745+
) {
3746+
logger.info('[executeIntegrationTool] Skipping - already executing or terminal', {
3747+
id,
3748+
name,
3749+
state: toolCall.state,
3750+
})
3751+
return
3752+
}
3753+
36793754
// Set to executing state
36803755
const executingMap = { ...get().toolCallsById }
36813756
executingMap[id] = {
@@ -3824,6 +3899,46 @@ export const useCopilotStore = create<CopilotStore>()(
38243899
const data = await res.json()
38253900
set({ autoAllowedTools: data.autoAllowedTools || [] })
38263901
logger.info('[AutoAllowedTools] Added tool', { toolId })
3902+
3903+
// Auto-execute all pending tools of the same type
3904+
const { toolCallsById, executeIntegrationTool } = get()
3905+
const pendingToolCalls = Object.values(toolCallsById).filter(
3906+
(tc) => tc.name === toolId && tc.state === ClientToolCallState.pending
3907+
)
3908+
if (pendingToolCalls.length > 0) {
3909+
const isIntegrationTool = !CLASS_TOOL_METADATA[toolId]
3910+
logger.info('[AutoAllowedTools] Auto-executing pending tools', {
3911+
toolId,
3912+
count: pendingToolCalls.length,
3913+
isIntegrationTool,
3914+
})
3915+
for (const tc of pendingToolCalls) {
3916+
if (isIntegrationTool) {
3917+
// Integration tools use executeIntegrationTool
3918+
executeIntegrationTool(tc.id).catch((err) => {
3919+
logger.error('[AutoAllowedTools] Auto-execute pending integration tool failed', {
3920+
toolCallId: tc.id,
3921+
toolId,
3922+
error: err,
3923+
})
3924+
})
3925+
} else {
3926+
// Client tools with interrupts use handleAccept
3927+
const inst = getClientTool(tc.id) as any
3928+
if (inst && typeof inst.handleAccept === 'function') {
3929+
Promise.resolve()
3930+
.then(() => inst.handleAccept(tc.params || {}))
3931+
.catch((err: any) => {
3932+
logger.error('[AutoAllowedTools] Auto-execute pending client tool failed', {
3933+
toolCallId: tc.id,
3934+
toolId,
3935+
error: err,
3936+
})
3937+
})
3938+
}
3939+
}
3940+
}
3941+
}
38273942
}
38283943
} catch (err) {
38293944
logger.error('[AutoAllowedTools] Failed to add tool', { toolId, error: err })

0 commit comments

Comments
 (0)