From 2c031ac4dd3ffab3ba8e6b56a525c272620022ef Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan Date: Wed, 24 Jun 2026 10:29:43 -0700 Subject: [PATCH 1/3] improvement(workflow-linter): added custom tool validation to workflow linter --- .../server/workflow/edit-workflow/index.ts | 26 ++ .../server/workflow/edit-workflow/lint.ts | 22 +- .../workflow/edit-workflow/validation.test.ts | 319 +++++++++++++++++- .../workflow/edit-workflow/validation.ts | 258 +++++++++++++- 4 files changed, 615 insertions(+), 10 deletions(-) diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts index 118060afd55..23aa7e4fe41 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts @@ -44,6 +44,7 @@ import { } from './lint' import { type EditWorkflowParams, isDeferredSkippedItem, type ValidationError } from './types' import { + collectUnresolvedAgentToolReferences, collectUnresolvedReferences, preValidateCredentialInputs, UNRESOLVABLE_AT_LINT_NOTE, @@ -190,6 +191,31 @@ export const editWorkflowServerTool: BaseServerTool error: toError(error).message, }) } + + // Resolve agent-block tool/skill references (custom tools, MCP servers, + // skills). A well-shaped entry whose id does not resolve is dropped at + // runtime, so the agent silently loses the tool/skill - surface it through + // the same lint + input-validation channels as credential/resource refs. + try { + const toolReferences = await collectUnresolvedAgentToolReferences(modifiedWorkflowState, { + userId: context.userId, + workspaceId, + }) + unresolvedReferences.push(...toolReferences) + validationErrors.push( + ...toolReferences.map((ref) => ({ + blockId: ref.blockId, + blockType: ref.blockType ?? 'agent', + field: ref.field, + value: ref.value, + error: ref.reason, + })) + ) + } catch (error) { + logger.warn('Agent tool/skill reference validation failed', { + error: toError(error).message, + }) + } } // Validate the workflow state diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/lint.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/lint.ts index 8b9789fa276..a4907006b53 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/lint.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/lint.ts @@ -65,11 +65,11 @@ export interface WorkflowLintFieldIssue extends WorkflowLintBlockRef { inactiveModeValues: InactiveModeValue[] } -/** Tier-2 (async, DB) credential/resource reference that does not resolve to an accessible entity. */ +/** Tier-2 (async, DB) reference that does not resolve to an accessible entity. */ export interface WorkflowLintUnresolvedReference extends WorkflowLintBlockRef { field: string value: string | string[] - kind: 'credential' | 'resource' + kind: 'credential' | 'resource' | 'custom-tool' | 'mcp-tool' | 'skill' reason: string } @@ -351,9 +351,23 @@ export function formatWorkflowLintMessage(lint: WorkflowLintIssueView) { } const unresolved = lint.unresolvedReferences ?? [] - if (unresolved.length > 0) { + const credResourceRefs = unresolved.filter( + (ref) => ref.kind === 'credential' || ref.kind === 'resource' + ) + if (credResourceRefs.length > 0) { + parts.push( + `Credential/resource references that do not resolve: ${credResourceRefs + .map((ref) => `"${ref.blockName || ref.blockId}".${ref.field} (${ref.reason})`) + .join(', ')}` + ) + } + + const toolSkillRefs = unresolved.filter( + (ref) => ref.kind === 'custom-tool' || ref.kind === 'mcp-tool' || ref.kind === 'skill' + ) + if (toolSkillRefs.length > 0) { parts.push( - `Credential/resource references that do not resolve: ${unresolved + `Agent tool/skill references that do not resolve (they will not attach at runtime): ${toolSkillRefs .map((ref) => `"${ref.blockName || ref.blockId}".${ref.field} (${ref.reason})`) .join(', ')}` ) diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts index 9c929988030..c4f60119ea9 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts @@ -5,10 +5,13 @@ import { envFlagsMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' import { normalizeConditionRouterIds } from './builders' -const { mockValidateSelectorIds, mockGetModelOptions } = vi.hoisted(() => ({ - mockValidateSelectorIds: vi.fn(), - mockGetModelOptions: vi.fn(() => []), -})) +const { mockValidateSelectorIds, mockGetModelOptions, mockGetCustomToolById, mockGetSkillById } = + vi.hoisted(() => ({ + mockValidateSelectorIds: vi.fn(), + mockGetModelOptions: vi.fn(() => []), + mockGetCustomToolById: vi.fn(), + mockGetSkillById: vi.fn(), + })) const conditionBlockConfig = { type: 'condition', @@ -38,7 +41,11 @@ const agentBlockConfig = { type: 'agent', name: 'Agent', outputs: {}, - subBlocks: [{ id: 'model', type: 'combobox', options: mockGetModelOptions }], + subBlocks: [ + { id: 'model', type: 'combobox', options: mockGetModelOptions }, + { id: 'tools', type: 'tool-input' }, + { id: 'skills', type: 'skill-input' }, + ], } const huggingfaceBlockConfig = { @@ -92,6 +99,14 @@ vi.mock('@/lib/copilot/validation/selector-validator', () => ({ validateSelectorIds: mockValidateSelectorIds, })) +vi.mock('@/lib/workflows/custom-tools/operations', () => ({ + getCustomToolById: mockGetCustomToolById, +})) + +vi.mock('@/lib/workflows/skills/operations', () => ({ + getSkillById: mockGetSkillById, +})) + vi.mock('@/lib/core/config/env-flags', () => envFlagsMock) vi.mock('@/providers/utils', () => ({ @@ -99,6 +114,7 @@ vi.mock('@/providers/utils', () => ({ })) import { + collectUnresolvedAgentToolReferences, collectUnresolvedReferences, preValidateCredentialInputs, validateInputsForBlock, @@ -415,3 +431,296 @@ describe('collectUnresolvedReferences', () => { expect(refs[0]).toMatchObject({ field: 'credential', kind: 'credential' }) }) }) + +describe('validateInputsForBlock - agent tools (tool-input)', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('accepts a reference-format custom tool', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ type: 'custom-tool', customToolId: 'ct_123', usageControl: 'auto' }] }, + 'agent-1' + ) + expect(result.errors).toHaveLength(0) + expect(result.validInputs.tools).toBeDefined() + }) + + it('accepts an inline custom tool with schema.function', () => { + const result = validateInputsForBlock( + 'agent', + { + tools: [ + { + type: 'custom-tool', + schema: { type: 'function', function: { name: 'foo', parameters: { type: 'object' } } }, + code: 'return 1', + }, + ], + }, + 'agent-1' + ) + expect(result.errors).toHaveLength(0) + expect(result.validInputs.tools).toBeDefined() + }) + + it('rejects a custom tool missing "type": "custom-tool" (the no-icon case)', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ customToolId: 'ct_123', usageControl: 'auto' }] }, + 'agent-1' + ) + expect(result.validInputs.tools).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0]?.error).toContain('custom-tool') + }) + + it('rejects a raw OpenAI function schema pasted into the array', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ type: 'function', function: { name: 'foo', parameters: {} } }] }, + 'agent-1' + ) + expect(result.validInputs.tools).toBeUndefined() + expect(result.errors[0]?.error).toContain('raw function schema') + }) + + it('rejects a custom tool with neither customToolId nor inline schema', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ type: 'custom-tool', usageControl: 'auto' }] }, + 'agent-1' + ) + expect(result.validInputs.tools).toBeUndefined() + expect(result.errors[0]?.error).toContain('customToolId') + }) + + it('rejects an MCP tool missing params.serverId/toolName', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ type: 'mcp', title: 'x', usageControl: 'auto' }] }, + 'agent-1' + ) + expect(result.validInputs.tools).toBeUndefined() + expect(result.errors[0]?.error).toContain('params.serverId') + }) + + it('accepts an MCP tool with params.serverId and params.toolName', () => { + const result = validateInputsForBlock( + 'agent', + { + tools: [ + { + type: 'mcp', + params: { serverId: 'srv_1', toolName: 'web_search' }, + usageControl: 'auto', + }, + ], + }, + 'agent-1' + ) + expect(result.errors).toHaveLength(0) + expect(result.validInputs.tools).toBeDefined() + }) + + it('accepts an integration tool whose type is a known block', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ type: 'slack', operation: 'send', usageControl: 'auto' }] }, + 'agent-1' + ) + expect(result.errors).toHaveLength(0) + expect(result.validInputs.tools).toBeDefined() + }) + + it('rejects an unrecognized tool type', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ type: 'nonexistent-block', operation: 'x' }] }, + 'agent-1' + ) + expect(result.validInputs.tools).toBeUndefined() + expect(result.errors[0]?.error).toContain('unrecognized tool type') + }) + + it('reports every bad entry in a single error', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ customToolId: 'x' }, { type: 'mcp' }] }, + 'agent-1' + ) + expect(result.validInputs.tools).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0]?.error).toContain('tools[0]') + expect(result.errors[0]?.error).toContain('tools[1]') + }) + + it('rejects a non-array tools value', () => { + const result = validateInputsForBlock('agent', { tools: 'not-an-array' }, 'agent-1') + expect(result.validInputs.tools).toBeUndefined() + expect(result.errors[0]?.error).toContain('expected an array') + }) +}) + +describe('validateInputsForBlock - agent skills (skill-input)', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('accepts a well-formed skill entry', () => { + const result = validateInputsForBlock( + 'agent', + { skills: [{ skillId: 'builtin-deploy-workflow', name: 'deploy-workflow' }] }, + 'agent-1' + ) + expect(result.errors).toHaveLength(0) + expect(result.validInputs.skills).toBeDefined() + }) + + it('rejects a skill entry that uses "id" instead of "skillId"', () => { + const result = validateInputsForBlock('agent', { skills: [{ id: 'x', name: 'y' }] }, 'agent-1') + expect(result.validInputs.skills).toBeUndefined() + expect(result.errors[0]?.error).toContain('skillId') + }) + + it('rejects a skill entry missing skillId', () => { + const result = validateInputsForBlock('agent', { skills: [{ name: 'y' }] }, 'agent-1') + expect(result.validInputs.skills).toBeUndefined() + expect(result.errors[0]?.error).toContain('skillId') + }) + + it('rejects a tool-shaped entry placed in the skills array', () => { + const result = validateInputsForBlock( + 'agent', + { skills: [{ type: 'custom-tool', customToolId: 'ct_1' }] }, + 'agent-1' + ) + expect(result.validInputs.skills).toBeUndefined() + expect(result.errors[0]?.error).toContain('skills') + }) + + it('rejects a non-array skills value', () => { + const result = validateInputsForBlock('agent', { skills: {} }, 'agent-1') + expect(result.validInputs.skills).toBeUndefined() + expect(result.errors[0]?.error).toContain('expected an array') + }) +}) + +describe('collectUnresolvedAgentToolReferences', () => { + beforeEach(() => { + vi.clearAllMocks() + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [] }) + mockGetCustomToolById.mockResolvedValue(null) + mockGetSkillById.mockResolvedValue(null) + }) + + it('flags a custom tool whose customToolId does not resolve', async () => { + mockGetCustomToolById.mockResolvedValue(null) + const state = { + blocks: { + a1: { + type: 'agent', + name: 'Agent 1', + subBlocks: { tools: { value: [{ type: 'custom-tool', customToolId: 'missing_ct' }] } }, + }, + }, + } + const refs = await collectUnresolvedAgentToolReferences(state, CTX) + expect(refs).toHaveLength(1) + expect(refs[0]).toMatchObject({ blockId: 'a1', field: 'tools', kind: 'custom-tool' }) + expect(refs[0]?.reason).toContain('missing_ct') + }) + + it('does not DB-check an inline custom tool (it carries its own schema)', async () => { + const state = { + blocks: { + a1: { + type: 'agent', + subBlocks: { + tools: { + value: [ + { + type: 'custom-tool', + customToolId: 'x', + schema: { type: 'function', function: { name: 'f', parameters: {} } }, + }, + ], + }, + }, + }, + }, + } + const refs = await collectUnresolvedAgentToolReferences(state, CTX) + expect(refs).toHaveLength(0) + expect(mockGetCustomToolById).not.toHaveBeenCalled() + }) + + it('does not flag a custom tool that resolves', async () => { + mockGetCustomToolById.mockResolvedValue({ id: 'ct_ok' }) + const state = { + blocks: { + a1: { + type: 'agent', + subBlocks: { tools: { value: [{ type: 'custom-tool', customToolId: 'ct_ok' }] } }, + }, + }, + } + const refs = await collectUnresolvedAgentToolReferences(state, CTX) + expect(refs).toHaveLength(0) + }) + + it('flags an MCP tool whose server does not resolve', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['srv_missing'] }) + const state = { + blocks: { + a1: { + type: 'agent', + subBlocks: { + tools: { value: [{ type: 'mcp', params: { serverId: 'srv_missing', toolName: 'x' } }] }, + }, + }, + }, + } + const refs = await collectUnresolvedAgentToolReferences(state, CTX) + expect(refs).toHaveLength(1) + expect(refs[0]).toMatchObject({ field: 'tools', kind: 'mcp-tool' }) + expect(mockValidateSelectorIds).toHaveBeenCalledWith('mcp-server-selector', 'srv_missing', CTX) + }) + + it('flags a skill whose skillId does not resolve', async () => { + mockGetSkillById.mockResolvedValue(null) + const state = { + blocks: { + a1: { type: 'agent', subBlocks: { skills: { value: [{ skillId: 'bogus-skill' }] } } }, + }, + } + const refs = await collectUnresolvedAgentToolReferences(state, CTX) + expect(refs).toHaveLength(1) + expect(refs[0]).toMatchObject({ field: 'skills', kind: 'skill' }) + expect(refs[0]?.reason).toContain('bogus-skill') + }) + + it('does not flag a skill that resolves (builtin or workspace)', async () => { + mockGetSkillById.mockResolvedValue({ id: 'builtin-deploy-workflow', name: 'deploy-workflow' }) + const state = { + blocks: { + a1: { + type: 'agent', + subBlocks: { skills: { value: [{ skillId: 'builtin-deploy-workflow' }] } }, + }, + }, + } + const refs = await collectUnresolvedAgentToolReferences(state, CTX) + expect(refs).toHaveLength(0) + }) + + it('ignores non-agent blocks', async () => { + const state = { + blocks: { s1: { type: 'slack', subBlocks: { tools: { value: [{ type: 'custom-tool' }] } } } }, + } + const refs = await collectUnresolvedAgentToolReferences(state, CTX) + expect(refs).toHaveLength(0) + expect(mockGetCustomToolById).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts index 58d04785611..bc6e22ad94d 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts @@ -3,6 +3,8 @@ import { toError } from '@sim/utils/errors' import { validateSelectorIds } from '@/lib/copilot/validation/selector-validator' import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access' import type { PermissionGroupConfig } from '@/lib/permission-groups/types' +import { getCustomToolById } from '@/lib/workflows/custom-tools/operations' +import { getSkillById } from '@/lib/workflows/skills/operations' import { buildCanonicalIndex, buildSubBlockValues, @@ -116,6 +118,101 @@ export function validateInputsForBlock( return { validInputs: validatedInputs, errors } } +/** Tool-entry `type` values that are valid but are not registry block types. */ +const KNOWN_NON_BLOCK_TOOL_TYPES = new Set(['custom-tool', 'mcp', 'workflow']) + +/** + * Validates a single entry in an agent block's `tools` (tool-input) array and + * returns a human-readable error string for LLM feedback, or null when valid. + * + * Targets the shapes that silently fail to attach (the entry is stored but the + * wrench icon never renders and/or the runtime drops the tool so the agent never + * sees it): a custom tool missing `type: 'custom-tool'`, a custom tool with + * neither `customToolId` nor an inline `schema.function`, an MCP tool missing + * its `params.serverId`/`params.toolName`, a raw OpenAI function schema pasted + * into the array, and unrecognized tool types. + */ +function validateAgentToolEntry(item: any, index: number): string | null { + const where = `tools[${index}]` + if (item === null || typeof item !== 'object' || Array.isArray(item)) { + return `${where} must be a tool object` + } + + const type = item.type + + // Raw OpenAI function schema pasted directly into the array (common mistake). + if ( + type !== 'custom-tool' && + item.function && + typeof item.function === 'object' && + typeof item.function.name === 'string' + ) { + return `${where} looks like a raw function schema. A custom tool must be {"type":"custom-tool","customToolId":""} (preferred) or {"type":"custom-tool","schema":{"type":"function","function":{...}},"code":"..."}` + } + + if (typeof type !== 'string' || type.trim() === '') { + return `${where} is missing a string "type". Custom tools require "type":"custom-tool" (without it the tool will not attach or show its icon); use "mcp" for MCP tools or an integration block type (e.g. "exa") otherwise` + } + + if (type === 'custom-tool') { + const hasReference = typeof item.customToolId === 'string' && item.customToolId.trim() !== '' + const fn = item.schema?.function + const hasInlineSchema = + !!fn && + typeof fn.name === 'string' && + fn.name.trim() !== '' && + !!fn.parameters && + typeof fn.parameters === 'object' + if (!hasReference && !hasInlineSchema) { + return `${where} (custom-tool) must include "customToolId" (the "id" from agent/custom-tools/{name}.json - not the filename, not schema.function.name) or an inline "schema.function" with "name" and "parameters"` + } + return null + } + + if (type === 'mcp') { + const serverId = item.params?.serverId + const toolName = item.params?.toolName + const ok = + typeof serverId === 'string' && + serverId.trim() !== '' && + typeof toolName === 'string' && + toolName.trim() !== '' + if (!ok) { + return `${where} (mcp) must include params.serverId and params.toolName` + } + return null + } + + // Integration/block-based tool: the type must be a real registry block type. + if (!KNOWN_NON_BLOCK_TOOL_TYPES.has(type) && !getBlock(type)) { + return `${where} has unrecognized tool type "${type}". Use "custom-tool" for custom tools, "mcp" for MCP tools, or a valid integration block type` + } + + return null +} + +/** + * Validates a single entry in an agent block's `skills` (skill-input) array. + * Skills are a SEPARATE array from tools; each entry references a workspace or + * builtin skill by `skillId`. Returns an error string or null when valid. + */ +function validateAgentSkillEntry(item: any, index: number): string | null { + const where = `skills[${index}]` + if (item === null || typeof item !== 'object' || Array.isArray(item)) { + return `${where} must be a skill object like {"skillId":"","name":""}` + } + if (typeof item.skillId !== 'string' || item.skillId.trim() === '') { + if (typeof item.id === 'string') { + return `${where} uses "id" but skills require "skillId" (the "id" from agent/skills/{name}.json)` + } + if (typeof item.type === 'string' || item.schema || item.customToolId) { + return `${where} looks like a tool entry. Skills go in the SEPARATE "skills" array and need only {"skillId":""} - no "type"/"schema"/"customToolId"` + } + return `${where} must include "skillId" (the "id" from agent/skills/{name}.json)` + } + return null +} + /** * Validates a value against its expected subBlock type * Returns validation result with the value or an error @@ -297,6 +394,53 @@ export function validateValueForSubBlockType( }, } } + const toolErrors = value + .map((item, index) => validateAgentToolEntry(item, index)) + .filter((err): err is string => err !== null) + if (toolErrors.length > 0) { + return { + valid: false, + error: { + blockId, + blockType, + field: fieldName, + value, + error: `Invalid tool ${toolErrors.length === 1 ? 'entry' : 'entries'} in "${fieldName}": ${toolErrors.join('; ')}`, + }, + } + } + return { valid: true, value } + } + + case 'skill-input': { + // Should be an array of skill reference objects ({ skillId, name? }) + if (!Array.isArray(value)) { + return { + valid: false, + error: { + blockId, + blockType, + field: fieldName, + value, + error: `Invalid skill-input value for field "${fieldName}" - expected an array of skill objects`, + }, + } + } + const skillErrors = value + .map((item, index) => validateAgentSkillEntry(item, index)) + .filter((err): err is string => err !== null) + if (skillErrors.length > 0) { + return { + valid: false, + error: { + blockId, + blockType, + field: fieldName, + value, + error: `Invalid skill ${skillErrors.length === 1 ? 'entry' : 'entries'} in "${fieldName}": ${skillErrors.join('; ')}`, + }, + } + } return { valid: true, value } } @@ -735,7 +879,7 @@ export interface UnresolvedSelectorReference { blockType?: string field: string value: string | string[] - kind: 'credential' | 'resource' + kind: 'credential' | 'resource' | 'custom-tool' | 'mcp-tool' | 'skill' reason: string } @@ -937,6 +1081,118 @@ export async function collectUnresolvedReferences( return references } +/** + * Lint-facing existence check for agent-block tool/skill references. Walks every + * agent block and verifies that reference-format custom tools (`customToolId`), + * MCP tools (`params.serverId`), and skills (`skillId`) resolve to real + * workspace/builtin entities. A well-shaped entry whose id does not resolve + * passes shape validation but is silently dropped at runtime (the agent never + * sees the tool/skill), so surface it through the lint channel. Best-effort: + * per-entry resolution failures are skipped rather than failing the edit. + */ +export async function collectUnresolvedAgentToolReferences( + workflowState: any, + context: { userId: string; workspaceId?: string } +): Promise { + const logger = createLogger('EditWorkflowAgentToolLint') + const references: UnresolvedSelectorReference[] = [] + const { userId, workspaceId } = context + + for (const [blockId, block] of Object.entries(workflowState.blocks || {})) { + const blockData = block as any + if (blockData?.type !== 'agent') continue + const blockName = blockData.name as string | undefined + + const tools = blockData.subBlocks?.tools?.value + if (Array.isArray(tools)) { + for (const tool of tools) { + if (!tool || typeof tool !== 'object') continue + + // Reference-format custom tools must resolve to a DB row. Inline tools + // (those carrying their own schema) are self-contained, so skip them. + if (tool.type === 'custom-tool' && !tool.schema) { + const toolId = tool.customToolId + if (typeof toolId !== 'string' || toolId.trim() === '') continue + try { + const found = await getCustomToolById({ toolId, userId, workspaceId }) + if (!found) { + references.push({ + blockId, + blockName, + blockType: 'agent', + field: 'tools', + value: toolId, + kind: 'custom-tool', + reason: `custom tool id "${toolId}" does not resolve to a custom tool in this workspace - create it with manage_custom_tool and use the returned id, otherwise the agent will not see the tool`, + }) + } + } catch (error) { + logger.warn('Custom tool resolution failed; skipping', { + blockId, + toolId, + error: toError(error).message, + }) + } + } else if (tool.type === 'mcp' && workspaceId) { + const serverId = tool.params?.serverId + if (typeof serverId !== 'string' || serverId.trim() === '') continue + try { + const result = await validateSelectorIds('mcp-server-selector', serverId, context) + if (result.invalid.length > 0) { + references.push({ + blockId, + blockName, + blockType: 'agent', + field: 'tools', + value: serverId, + kind: 'mcp-tool', + reason: `MCP server "${serverId}" does not resolve to an enabled MCP server in this workspace`, + }) + } + } catch (error) { + logger.warn('MCP server resolution failed; skipping', { + blockId, + serverId, + error: toError(error).message, + }) + } + } + } + } + + const skills = blockData.subBlocks?.skills?.value + if (Array.isArray(skills) && workspaceId) { + for (const skillEntry of skills) { + if (!skillEntry || typeof skillEntry !== 'object') continue + const skillId = skillEntry.skillId + if (typeof skillId !== 'string' || skillId.trim() === '') continue + try { + const found = await getSkillById({ skillId, workspaceId }) + if (!found) { + references.push({ + blockId, + blockName, + blockType: 'agent', + field: 'skills', + value: skillId, + kind: 'skill', + reason: `skill id "${skillId}" does not resolve to a builtin or workspace skill - use manage_skill (operation "list") to get valid ids`, + }) + } + } catch (error) { + logger.warn('Skill resolution failed; skipping', { + blockId, + skillId, + error: toError(error).message, + }) + } + } + } + } + + return references +} + /** * Pre-validates credential and apiKey inputs in operations before they are applied. * - Validates oauth-input (credential) IDs are accessible to the user in the workflow workspace From ccd737bca154b3b7ed37254cc6f401b87e26ec34 Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan Date: Wed, 24 Jun 2026 11:16:01 -0700 Subject: [PATCH 2/3] fix(comments): address pr comments --- .../workflow/edit-workflow/validation.test.ts | 14 ++++++++++++++ .../server/workflow/edit-workflow/validation.ts | 13 ++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts index c4f60119ea9..bd0fc79d77b 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts @@ -670,6 +670,20 @@ describe('collectUnresolvedAgentToolReferences', () => { expect(refs).toHaveLength(0) }) + it('does not DB-check a custom tool when workspaceId is absent (avoids false positives)', async () => { + const state = { + blocks: { + a1: { + type: 'agent', + subBlocks: { tools: { value: [{ type: 'custom-tool', customToolId: 'ct_x' }] } }, + }, + }, + } + const refs = await collectUnresolvedAgentToolReferences(state, { userId: 'user-1' }) + expect(refs).toHaveLength(0) + expect(mockGetCustomToolById).not.toHaveBeenCalled() + }) + it('flags an MCP tool whose server does not resolve', async () => { mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['srv_missing'] }) const state = { diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts index bc6e22ad94d..8f79c22dbf8 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts @@ -27,6 +27,7 @@ import type { import { SELECTOR_TYPES } from './types' const validationLogger = createLogger('EditWorkflowValidation') +const agentToolLintLogger = createLogger('EditWorkflowAgentToolLint') /** * Finds an existing block with the same normalized name. @@ -141,8 +142,11 @@ function validateAgentToolEntry(item: any, index: number): string | null { const type = item.type // Raw OpenAI function schema pasted directly into the array (common mistake). + // Keyed on type === 'function' (OpenAI's exact discriminator) so a real + // integration tool that happens to carry a `function` property is not + // misreported here - it falls through to the block-type check below. if ( - type !== 'custom-tool' && + type === 'function' && item.function && typeof item.function === 'object' && typeof item.function.name === 'string' @@ -1094,7 +1098,7 @@ export async function collectUnresolvedAgentToolReferences( workflowState: any, context: { userId: string; workspaceId?: string } ): Promise { - const logger = createLogger('EditWorkflowAgentToolLint') + const logger = agentToolLintLogger const references: UnresolvedSelectorReference[] = [] const { userId, workspaceId } = context @@ -1110,7 +1114,10 @@ export async function collectUnresolvedAgentToolReferences( // Reference-format custom tools must resolve to a DB row. Inline tools // (those carrying their own schema) are self-contained, so skip them. - if (tool.type === 'custom-tool' && !tool.schema) { + // Gated on workspaceId (like the MCP/skill paths below): without a + // workspace, getCustomToolById only sees legacy tools and would + // false-positive on every workspace-scoped tool. + if (tool.type === 'custom-tool' && !tool.schema && workspaceId) { const toolId = tool.customToolId if (typeof toolId !== 'string' || toolId.trim() === '') continue try { From fa5e12d89943ad62d54602686c857f60f0db8278 Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan Date: Wed, 24 Jun 2026 11:19:50 -0700 Subject: [PATCH 3/3] improvement(validation): ensure type is known --- .../workflow/edit-workflow/validation.test.ts | 11 +++++++++++ .../server/workflow/edit-workflow/validation.ts | 16 +++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts index bd0fc79d77b..bee6a3186d5 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts @@ -25,6 +25,7 @@ const oauthBlockConfig = { name: 'Slack', outputs: {}, subBlocks: [{ id: 'credential', type: 'oauth-input' }], + tools: { access: ['slack_message'] }, } const routerBlockConfig = { @@ -544,6 +545,16 @@ describe('validateInputsForBlock - agent tools (tool-input)', () => { expect(result.errors[0]?.error).toContain('unrecognized tool type') }) + it('rejects a known block that exposes no callable tools (not tool-capable)', () => { + const result = validateInputsForBlock( + 'agent', + { tools: [{ type: 'condition', operation: 'x' }] }, + 'agent-1' + ) + expect(result.validInputs.tools).toBeUndefined() + expect(result.errors[0]?.error).toContain('cannot be attached as an agent tool') + }) + it('reports every bad entry in a single error', () => { const result = validateInputsForBlock( 'agent', diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts index 8f79c22dbf8..2fb0a6de35b 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts @@ -187,9 +187,19 @@ function validateAgentToolEntry(item: any, index: number): string | null { return null } - // Integration/block-based tool: the type must be a real registry block type. - if (!KNOWN_NON_BLOCK_TOOL_TYPES.has(type) && !getBlock(type)) { - return `${where} has unrecognized tool type "${type}". Use "custom-tool" for custom tools, "mcp" for MCP tools, or a valid integration block type` + // Integration/block-based tool: the type must be a real registry block that + // actually exposes callable tools. A known block with an empty tools.access + // (control-flow blocks like condition/loop/parallel/router, or the agent block + // itself) can't attach as an agent tool, so the addition would not apply + // correctly even though the type "exists". + if (!KNOWN_NON_BLOCK_TOOL_TYPES.has(type)) { + const block = getBlock(type) + if (!block) { + return `${where} has unrecognized tool type "${type}". Use "custom-tool" for custom tools, "mcp" for MCP tools, or a valid integration block type` + } + if (!Array.isArray(block.tools?.access) || block.tools.access.length === 0) { + return `${where} block type "${type}" cannot be attached as an agent tool (it exposes no callable tools)` + } } return null