fix(salesforce): address Greptile/Cursor review

waleedlatif1 · waleedlatif1 · commit a0c593f61c2e · 2026-06-13T19:53:09.000-07:00
- Add shared requireId() guard so whitespace-only IDs fail fast instead of producing malformed /sobjects/Object/ empty-path requests (all update/delete/single-get tools)
- URL-encode the fields query value in get_opportunities and get_tasks single-record GETs (matching the other get_* tools)
- Reflect the Salesforce API success flag consistently across all create tools (success/created use data.success === true)
diff --git a/apps/sim/tools/salesforce/create_account.ts b/apps/sim/tools/salesforce/create_account.ts
@@ -169,8 +169,8 @@ export const salesforceCreateAccountTool: ToolConfig<
       success: true,
       output: {
         id: data.id,
-        success: true,
-        created: true,
+        success: data.success === true,
+        created: data.success === true,
       },
     }
   },
diff --git a/apps/sim/tools/salesforce/create_case.ts b/apps/sim/tools/salesforce/create_case.ts
@@ -108,8 +108,8 @@ export const salesforceCreateCaseTool: ToolConfig<
       success: true,
       output: {
         id: data.id,
-        success: data.success,
-        created: true,
+        success: data.success === true,
+        created: data.success === true,
       },
     }
   },
diff --git a/apps/sim/tools/salesforce/create_contact.ts b/apps/sim/tools/salesforce/create_contact.ts
@@ -143,8 +143,8 @@ export const salesforceCreateContactTool: ToolConfig<
       success: true,
       output: {
         id: data.id,
-        success: data.success,
-        created: true,
+        success: data.success === true,
+        created: data.success === true,
       },
     }
   },
diff --git a/apps/sim/tools/salesforce/create_opportunity.ts b/apps/sim/tools/salesforce/create_opportunity.ts
@@ -103,8 +103,8 @@ export const salesforceCreateOpportunityTool: ToolConfig<
       success: true,
       output: {
         id: data.id,
-        success: data.success,
-        created: true,
+        success: data.success === true,
+        created: data.success === true,
       },
     }
   },
diff --git a/apps/sim/tools/salesforce/create_task.ts b/apps/sim/tools/salesforce/create_task.ts
@@ -108,8 +108,8 @@ export const salesforceCreateTaskTool: ToolConfig<
       success: true,
       output: {
         id: data.id,
-        success: data.success,
-        created: true,
+        success: data.success === true,
+        created: data.success === true,
       },
     }
   },
diff --git a/apps/sim/tools/salesforce/delete_account.ts b/apps/sim/tools/salesforce/delete_account.ts
@@ -3,7 +3,7 @@ import type {
   SalesforceDeleteAccountResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_DELETE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceDeleteAccountTool: ToolConfig<
@@ -47,7 +47,7 @@ export const salesforceDeleteAccountTool: ToolConfig<
   request: {
     url: (params) => {
       const instanceUrl = getInstanceUrl(params.idToken, params.instanceUrl)
-      const accountId = params.accountId.trim()
+      const accountId = requireId(params.accountId, 'Account ID')
 
       return `${instanceUrl}/services/data/v59.0/sobjects/Account/${accountId}`
     },
diff --git a/apps/sim/tools/salesforce/delete_case.ts b/apps/sim/tools/salesforce/delete_case.ts
@@ -3,7 +3,7 @@ import type {
   SalesforceDeleteCaseResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_DELETE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceDeleteCaseTool: ToolConfig<
@@ -46,7 +46,7 @@ export const salesforceDeleteCaseTool: ToolConfig<
 
   request: {
     url: (params) => {
-      const caseId = params.caseId.trim()
+      const caseId = requireId(params.caseId, 'Case ID')
       return `${getInstanceUrl(params.idToken, params.instanceUrl)}/services/data/v59.0/sobjects/Case/${caseId}`
     },
     method: 'DELETE',
diff --git a/apps/sim/tools/salesforce/delete_contact.ts b/apps/sim/tools/salesforce/delete_contact.ts
@@ -4,7 +4,7 @@ import type {
   SalesforceDeleteContactResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_DELETE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 const logger = createLogger('SalesforceContacts')
@@ -35,7 +35,7 @@ export const salesforceDeleteContactTool: ToolConfig<
   request: {
     url: (params) => {
       const instanceUrl = getInstanceUrl(params.idToken, params.instanceUrl)
-      const contactId = params.contactId.trim()
+      const contactId = requireId(params.contactId, 'Contact ID')
       return `${instanceUrl}/services/data/v59.0/sobjects/Contact/${contactId}`
     },
     method: 'DELETE',
diff --git a/apps/sim/tools/salesforce/delete_lead.ts b/apps/sim/tools/salesforce/delete_lead.ts
@@ -3,7 +3,7 @@ import type {
   SalesforceDeleteLeadResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_DELETE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceDeleteLeadTool: ToolConfig<
@@ -34,7 +34,7 @@ export const salesforceDeleteLeadTool: ToolConfig<
 
   request: {
     url: (params) => {
-      const leadId = params.leadId.trim()
+      const leadId = requireId(params.leadId, 'Lead ID')
       return `${getInstanceUrl(params.idToken, params.instanceUrl)}/services/data/v59.0/sobjects/Lead/${leadId}`
     },
     method: 'DELETE',
diff --git a/apps/sim/tools/salesforce/delete_opportunity.ts b/apps/sim/tools/salesforce/delete_opportunity.ts
@@ -4,7 +4,7 @@ import type {
   SalesforceDeleteOpportunityResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_DELETE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 const logger = createLogger('SalesforceDeleteOpportunity')
@@ -37,7 +37,7 @@ export const salesforceDeleteOpportunityTool: ToolConfig<
 
   request: {
     url: (params) => {
-      const opportunityId = params.opportunityId.trim()
+      const opportunityId = requireId(params.opportunityId, 'Opportunity ID')
       return `${getInstanceUrl(params.idToken, params.instanceUrl)}/services/data/v59.0/sobjects/Opportunity/${opportunityId}`
     },
     method: 'DELETE',
diff --git a/apps/sim/tools/salesforce/delete_task.ts b/apps/sim/tools/salesforce/delete_task.ts
@@ -3,7 +3,7 @@ import type {
   SalesforceDeleteTaskResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_DELETE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceDeleteTaskTool: ToolConfig<
@@ -46,7 +46,7 @@ export const salesforceDeleteTaskTool: ToolConfig<
 
   request: {
     url: (params) => {
-      const taskId = params.taskId.trim()
+      const taskId = requireId(params.taskId, 'Task ID')
       return `${getInstanceUrl(params.idToken, params.instanceUrl)}/services/data/v59.0/sobjects/Task/${taskId}`
     },
     method: 'DELETE',
diff --git a/apps/sim/tools/salesforce/get_cases.ts b/apps/sim/tools/salesforce/get_cases.ts
@@ -1,6 +1,6 @@
 import type { SalesforceGetCasesParams, SalesforceGetCasesResponse } from '@/tools/salesforce/types'
 import { QUERY_PAGING_OUTPUT, RESPONSE_METADATA_OUTPUT } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceGetCasesTool: ToolConfig<
@@ -52,7 +52,7 @@ export const salesforceGetCasesTool: ToolConfig<
     url: (params) => {
       const instanceUrl = getInstanceUrl(params.idToken, params.instanceUrl)
       if (params.caseId) {
-        const caseId = params.caseId.trim()
+        const caseId = requireId(params.caseId, 'Case ID')
         const fields =
           params.fields || 'Id,CaseNumber,Subject,Status,Priority,Origin,ContactId,AccountId'
         return `${instanceUrl}/services/data/v59.0/sobjects/Case/${caseId}?fields=${encodeURIComponent(fields)}`
diff --git a/apps/sim/tools/salesforce/get_contacts.ts b/apps/sim/tools/salesforce/get_contacts.ts
@@ -4,7 +4,7 @@ import type {
   SalesforceGetContactsResponse,
 } from '@/tools/salesforce/types'
 import { QUERY_PAGING_OUTPUT, RESPONSE_METADATA_OUTPUT } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 const logger = createLogger('SalesforceContacts')
@@ -60,7 +60,7 @@ export const salesforceGetContactsTool: ToolConfig<
 
       // Single contact by ID
       if (params.contactId) {
-        const contactId = params.contactId.trim()
+        const contactId = requireId(params.contactId, 'Contact ID')
         const fields =
           params.fields || 'Id,FirstName,LastName,Email,Phone,AccountId,Title,Department'
         return `${instanceUrl}/services/data/v59.0/sobjects/Contact/${contactId}?fields=${encodeURIComponent(fields)}`
diff --git a/apps/sim/tools/salesforce/get_leads.ts b/apps/sim/tools/salesforce/get_leads.ts
@@ -1,6 +1,6 @@
 import type { SalesforceGetLeadsParams, SalesforceGetLeadsResponse } from '@/tools/salesforce/types'
 import { QUERY_PAGING_OUTPUT, RESPONSE_METADATA_OUTPUT } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceGetLeadsTool: ToolConfig<
@@ -52,7 +52,7 @@ export const salesforceGetLeadsTool: ToolConfig<
     url: (params) => {
       const instanceUrl = getInstanceUrl(params.idToken, params.instanceUrl)
       if (params.leadId) {
-        const leadId = params.leadId.trim()
+        const leadId = requireId(params.leadId, 'Lead ID')
         const fields =
           params.fields || 'Id,FirstName,LastName,Company,Email,Phone,Status,LeadSource'
         return `${instanceUrl}/services/data/v59.0/sobjects/Lead/${leadId}?fields=${encodeURIComponent(fields)}`
diff --git a/apps/sim/tools/salesforce/get_opportunities.ts b/apps/sim/tools/salesforce/get_opportunities.ts
@@ -4,7 +4,7 @@ import type {
   SalesforceGetOpportunitiesResponse,
 } from '@/tools/salesforce/types'
 import { QUERY_PAGING_OUTPUT, RESPONSE_METADATA_OUTPUT } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 const logger = createLogger('SalesforceGetOpportunities')
@@ -58,9 +58,9 @@ export const salesforceGetOpportunitiesTool: ToolConfig<
     url: (params) => {
       const instanceUrl = getInstanceUrl(params.idToken, params.instanceUrl)
       if (params.opportunityId) {
-        const opportunityId = params.opportunityId.trim()
+        const opportunityId = requireId(params.opportunityId, 'Opportunity ID')
         const fields = params.fields || 'Id,Name,AccountId,Amount,StageName,CloseDate,Probability'
-        return `${instanceUrl}/services/data/v59.0/sobjects/Opportunity/${opportunityId}?fields=${fields}`
+        return `${instanceUrl}/services/data/v59.0/sobjects/Opportunity/${opportunityId}?fields=${encodeURIComponent(fields)}`
       }
       const limit = params.limit ? Number.parseInt(params.limit) : 100
       const fields = params.fields || 'Id,Name,AccountId,Amount,StageName,CloseDate,Probability'
diff --git a/apps/sim/tools/salesforce/get_tasks.ts b/apps/sim/tools/salesforce/get_tasks.ts
@@ -1,6 +1,6 @@
 import type { SalesforceGetTasksParams, SalesforceGetTasksResponse } from '@/tools/salesforce/types'
 import { QUERY_PAGING_OUTPUT, RESPONSE_METADATA_OUTPUT } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceGetTasksTool: ToolConfig<
@@ -64,10 +64,10 @@ export const salesforceGetTasksTool: ToolConfig<
     url: (params) => {
       const instanceUrl = getInstanceUrl(params.idToken, params.instanceUrl)
       if (params.taskId) {
-        const taskId = params.taskId.trim()
+        const taskId = requireId(params.taskId, 'Task ID')
         const fields =
           params.fields || 'Id,Subject,Status,Priority,ActivityDate,WhoId,WhatId,OwnerId'
-        return `${instanceUrl}/services/data/v59.0/sobjects/Task/${taskId}?fields=${fields}`
+        return `${instanceUrl}/services/data/v59.0/sobjects/Task/${taskId}?fields=${encodeURIComponent(fields)}`
       }
       const limit = params.limit ? Number.parseInt(params.limit) : 100
       const fields = params.fields || 'Id,Subject,Status,Priority,ActivityDate,WhoId,WhatId,OwnerId'
diff --git a/apps/sim/tools/salesforce/update_account.ts b/apps/sim/tools/salesforce/update_account.ts
@@ -3,7 +3,7 @@ import type {
   SalesforceUpdateAccountResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_UPDATE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceUpdateAccountTool: ToolConfig<
@@ -125,7 +125,7 @@ export const salesforceUpdateAccountTool: ToolConfig<
   request: {
     url: (params) => {
       const instanceUrl = getInstanceUrl(params.idToken, params.instanceUrl)
-      const accountId = params.accountId.trim()
+      const accountId = requireId(params.accountId, 'Account ID')
 
       return `${instanceUrl}/services/data/v59.0/sobjects/Account/${accountId}`
     },
diff --git a/apps/sim/tools/salesforce/update_case.ts b/apps/sim/tools/salesforce/update_case.ts
@@ -3,7 +3,7 @@ import type {
   SalesforceUpdateCaseResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_UPDATE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceUpdateCaseTool: ToolConfig<
@@ -88,7 +88,7 @@ export const salesforceUpdateCaseTool: ToolConfig<
 
   request: {
     url: (params) => {
-      const caseId = params.caseId.trim()
+      const caseId = requireId(params.caseId, 'Case ID')
       return `${getInstanceUrl(params.idToken, params.instanceUrl)}/services/data/v59.0/sobjects/Case/${caseId}`
     },
     method: 'PATCH',
diff --git a/apps/sim/tools/salesforce/update_contact.ts b/apps/sim/tools/salesforce/update_contact.ts
@@ -4,7 +4,7 @@ import type {
   SalesforceUpdateContactResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_UPDATE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 const logger = createLogger('SalesforceContacts')
@@ -108,7 +108,7 @@ export const salesforceUpdateContactTool: ToolConfig<
   request: {
     url: (params) => {
       const instanceUrl = getInstanceUrl(params.idToken, params.instanceUrl)
-      const contactId = params.contactId.trim()
+      const contactId = requireId(params.contactId, 'Contact ID')
       return `${instanceUrl}/services/data/v59.0/sobjects/Contact/${contactId}`
     },
     method: 'PATCH',
diff --git a/apps/sim/tools/salesforce/update_lead.ts b/apps/sim/tools/salesforce/update_lead.ts
@@ -3,7 +3,7 @@ import type {
   SalesforceUpdateLeadResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_UPDATE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceUpdateLeadTool: ToolConfig<
@@ -83,7 +83,7 @@ export const salesforceUpdateLeadTool: ToolConfig<
 
   request: {
     url: (params) => {
-      const leadId = params.leadId.trim()
+      const leadId = requireId(params.leadId, 'Lead ID')
       return `${getInstanceUrl(params.idToken, params.instanceUrl)}/services/data/v59.0/sobjects/Lead/${leadId}`
     },
     method: 'PATCH',
diff --git a/apps/sim/tools/salesforce/update_opportunity.ts b/apps/sim/tools/salesforce/update_opportunity.ts
@@ -4,7 +4,7 @@ import type {
   SalesforceUpdateOpportunityResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_UPDATE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 const logger = createLogger('SalesforceUpdateOpportunity')
@@ -79,7 +79,7 @@ export const salesforceUpdateOpportunityTool: ToolConfig<
 
   request: {
     url: (params) => {
-      const opportunityId = params.opportunityId.trim()
+      const opportunityId = requireId(params.opportunityId, 'Opportunity ID')
       return `${getInstanceUrl(params.idToken, params.instanceUrl)}/services/data/v59.0/sobjects/Opportunity/${opportunityId}`
     },
     method: 'PATCH',
diff --git a/apps/sim/tools/salesforce/update_task.ts b/apps/sim/tools/salesforce/update_task.ts
@@ -3,7 +3,7 @@ import type {
   SalesforceUpdateTaskResponse,
 } from '@/tools/salesforce/types'
 import { SOBJECT_UPDATE_OUTPUT_PROPERTIES } from '@/tools/salesforce/types'
-import { extractErrorMessage, getInstanceUrl } from '@/tools/salesforce/utils'
+import { extractErrorMessage, getInstanceUrl, requireId } from '@/tools/salesforce/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const salesforceUpdateTaskTool: ToolConfig<
@@ -88,7 +88,7 @@ export const salesforceUpdateTaskTool: ToolConfig<
 
   request: {
     url: (params) => {
-      const taskId = params.taskId.trim()
+      const taskId = requireId(params.taskId, 'Task ID')
       return `${getInstanceUrl(params.idToken, params.instanceUrl)}/services/data/v59.0/sobjects/Task/${taskId}`
     },
     method: 'PATCH',
diff --git a/apps/sim/tools/salesforce/utils.ts b/apps/sim/tools/salesforce/utils.ts
@@ -36,6 +36,23 @@ export function getInstanceUrl(idToken?: string, instanceUrl?: string): string {
   throw new Error('Salesforce instance URL is required but not provided')
 }
 
+/**
+ * Trims a record ID and throws if it is missing or whitespace-only.
+ * Prevents whitespace-only IDs from collapsing into an empty URL path segment
+ * (e.g. `/sobjects/Account/`) and hitting Salesforce with a malformed request.
+ * @param value - The raw ID value from params
+ * @param label - Human-readable field name used in the error message
+ * @returns The trimmed, non-empty ID
+ * @throws Error if the ID is absent or whitespace-only
+ */
+export function requireId(value: string | undefined, label: string): string {
+  const trimmed = value?.trim()
+  if (!trimmed) {
+    throw new Error(`${label} is required. Please provide a valid Salesforce ${label}.`)
+  }
+  return trimmed
+}
+
 /**
  * Extracts a descriptive error message from Salesforce API responses
  * @param data - The response data from Salesforce API
