Skip to content

Commit a3d3100

Browse files
committed
fix(zoho-desk): overwrite operation-scoped params instead of omitting them
The block mapper scoped params by destructuring them out of the spread, on the assumption that a key left out of the return value never reaches the tool. It does: both call sites merge the mapper's output on top of the original inputs (`{ ...inputs, ...transformedParams }`), so an omitted key is restored. The serializer is what actually held this together, and it has a gap — an advanced subBlock with a retained value is emitted for every operation while the block's advanced toggle is off, because that branch returns on isNonEmptyValue without evaluating the subBlock's condition. So a Sort By set on List Tickets reached List Comments, and a ticket Include reached Get Contact, each rejected by Zoho. Out-of-range from/limit reached the wire for the same reason. Every scoped param is now assigned unconditionally, undefined included, so the merge cannot resurrect a stale value. Also fixes a crash this branch introduced: clearing the Departments multi-select stores [], which reached the comma-list normalizer and threw on .split. The helper now takes arrays, which is what that subBlock actually stores. The block-to-tool tests now model the real merge rather than the mapper's return value alone — the previous version passed while production threw on the same input. Corrects two comments that misstated where Zoho documents customFields and errorMessage, and splits the shared include subBlock, since Get Ticket accepts contract and skills and List Tickets does not.
1 parent 11c5bd6 commit a3d3100

8 files changed

Lines changed: 231 additions & 87 deletions

File tree

apps/docs/content/docs/en/integrations/zoho_desk.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ List tickets from a Zoho Desk organization with optional filters. Returns a list
5454
| `apiDomain` | string | No | Zoho Desk data-center REST base URL |
5555
| `orgId` | string | Yes | Zoho Desk organization ID |
5656
| `from` | number | No | Pagination start index \(0-based\) |
57-
| `limit` | number | No | Number of tickets to return \(1-100, default 10\) |
57+
| `limit` | number | No | Number of tickets to return \(1-100\) |
5858
| `departmentIds` | string | No | Filter by department ID \(comma-separated for multiple\) |
5959
| `status` | string | No | Filter by status, including custom statuses. Comma-separate to match multiple \(e.g. "Open,On Hold"\) |
6060
| `priority` | string | No | Filter by priority. Comma-separate to match multiple \(e.g. "High,Urgent"\) |
6161
| `assignee` | string | No | Filter by assignee: an agent ID, or "Unassigned". Comma-separate to match multiple. |
62-
| `channel` | string | No | Filter by origin channel \(e.g. Email, Web, Phone\). Comma-separate to match multiple. |
62+
| `channel` | string | No | Filter by origin channel, spelled as your portal spells it. Comma-separate to match multiple. |
6363
| `receivedInDays` | number | No | Only tickets whose last customer response was within the last 15, 30, or 90 days \(Zoho filters on customerResponseTime, despite the name\) |
6464
| `sortBy` | string | No | Sort field: createdTime, customerResponseTime, or responseDueDate. Prefix with - for descending. |
6565
| `include` | string | No | Comma-separated related data to embed. Allowed: contacts, products, departments, team, isRead, assignee |

apps/sim/blocks/blocks/zoho-desk.ts

Lines changed: 92 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,32 @@ const OPERATIONS_NEEDING_ORG = [
1818
'get_attachment',
1919
]
2020

21+
/**
22+
* Collapse the three "not supplied" shapes to `undefined`. The workflow
23+
* serializer initializes untouched subBlocks to `null`, and a cleared field
24+
* arrives as `''`; both mean the same thing as absent.
25+
*/
26+
function orUndefined(value: unknown): unknown {
27+
if (value === undefined || value === null || value === '') return undefined
28+
if (typeof value === 'string') {
29+
const trimmed = value.trim()
30+
return trimmed || undefined
31+
}
32+
return value
33+
}
34+
35+
/**
36+
* Coerce a pagination input to an integer at or above `min`, or `undefined`.
37+
* Anything out of range is discarded rather than forwarded: Zoho answers a
38+
* negative or fractional index with an opaque provider error.
39+
*/
40+
function toPaginationValue(value: unknown, min: number): number | undefined {
41+
const resolved = orUndefined(value)
42+
if (resolved === undefined) return undefined
43+
const parsed = Number(resolved)
44+
return Number.isInteger(parsed) && parsed >= min ? parsed : undefined
45+
}
46+
2147
export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
2248
type: 'zoho_desk',
2349
name: 'Zoho Desk',
@@ -397,7 +423,20 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
397423
title: 'Include',
398424
type: 'short-input',
399425
placeholder: 'e.g. contacts,assignee',
400-
condition: { field: 'operation', value: ['list_tickets', 'get_ticket'] },
426+
condition: { field: 'operation', value: 'list_tickets' },
427+
mode: 'advanced',
428+
},
429+
// Split from the list_tickets `include` above rather than shared: Get Ticket
430+
// additionally accepts `contract` and `skills`, which List Tickets does not
431+
// document. A shared subBlock keeps its value across an operation change, so
432+
// `skills` set here would follow the user to List Tickets and put an
433+
// undocumented token on the wire.
434+
{
435+
id: 'ticketInclude',
436+
title: 'Include',
437+
type: 'short-input',
438+
placeholder: 'e.g. contacts,contract,skills',
439+
condition: { field: 'operation', value: 'get_ticket' },
401440
mode: 'advanced',
402441
},
403442
{
@@ -485,9 +524,19 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
485524
config: {
486525
tool: (params) => `zoho_desk_${params.operation}`,
487526
params: (params) => {
488-
// Pull raw pagination out of the spread so invalid values never reach the
489-
// tool; only re-add them when Number() yields a finite value (a non-numeric
490-
// typo would otherwise become NaN and produce an invalid Zoho query param).
527+
// IMPORTANT: destructuring a key out of `rest` does NOT keep it from the
528+
// tool. Both call sites merge this function's return value on top of the
529+
// original inputs (`{ ...inputs, ...transformedParams }` in
530+
// executor/handlers/generic/generic-handler.ts, and the same shape in
531+
// providers/utils.ts), so a key left out of `result` is simply restored
532+
// from `inputs`. The only way to scope a param to an operation is to
533+
// OVERWRITE it with `undefined`, which every tool then treats as unset.
534+
//
535+
// This matters because a `mode: 'advanced'` subBlock with a retained
536+
// value is serialized for every operation when the block's advanced
537+
// toggle is off - serializer/index.ts returns on `isNonEmptyValue`
538+
// without evaluating the subBlock's `condition` - so stale advanced
539+
// values genuinely do arrive here under an unrelated operation.
491540
const {
492541
oauthCredential,
493542
from: rawFrom,
@@ -504,6 +553,7 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
504553
commentSortBy: rawCommentSortBy,
505554
threadSortBy: rawThreadSortBy,
506555
include: rawInclude,
556+
ticketInclude: rawTicketInclude,
507557
contactInclude: rawContactInclude,
508558
threadInclude: rawThreadInclude,
509559
assigneeFilter: rawAssigneeFilter,
@@ -525,37 +575,36 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
525575
: typeof rawDepartmentIds === 'string'
526576
? rawDepartmentIds.trim()
527577
: ''
528-
if (departmentIds) result.departmentIds = departmentIds
578+
// Always assigned, never conditionally: an emptied multi-select stores
579+
// `[]`, and leaving the key unset would let that array through to the
580+
// tool, where the comma-list normalizer would throw on `.split`.
581+
result.departmentIds = departmentIds || undefined
529582

530583
// contentType is the comment's content type; its default would otherwise
531584
// serialize for every operation (e.g. get_attachment, which has no such
532585
// param). Only forward it for add_comment so the UI can't imply an option
533586
// that has no effect elsewhere.
534-
if (params.operation === 'add_comment' && typeof contentType === 'string' && contentType) {
535-
result.contentType = contentType
536-
}
587+
result.contentType =
588+
params.operation === 'add_comment' && typeof contentType === 'string' && contentType
589+
? contentType
590+
: undefined
537591

538592
// Zoho documents from >= 0 and limit >= 1 as integers; a negative or
539593
// fractional value reaches the API as an opaque provider error, so drop
540594
// anything outside those bounds here rather than round-tripping it.
541595
// `null` is checked explicitly: the serializer initializes untouched
542596
// subBlocks to null, and Number(null) is 0 — which would otherwise inject
543597
// from=0 on every operation instead of leaving the param unset.
544-
if (rawFrom !== undefined && rawFrom !== null && rawFrom !== '') {
545-
const from = Number(rawFrom)
546-
if (Number.isInteger(from) && from >= 0) result.from = from
547-
}
548-
if (rawLimit !== undefined && rawLimit !== null && rawLimit !== '') {
549-
const limit = Number(rawLimit)
550-
if (Number.isInteger(limit) && limit >= 1) result.limit = limit
551-
}
598+
result.from = toPaginationValue(rawFrom, 0)
599+
result.limit = toPaginationValue(rawLimit, 1)
552600
// Gated for the same reason as contentType above: isPublic carries a
553601
// defaultValue, so forwarding it unconditionally would serialize a
554602
// comment-only field onto every other operation's params. Destructured
555603
// out of `rest` so the default never reaches non-comment operations.
556-
if (params.operation === 'add_comment' && isPublic !== undefined) {
557-
result.isPublic = isPublic === true || isPublic === 'true'
558-
}
604+
result.isPublic =
605+
params.operation === 'add_comment' && isPublic !== undefined
606+
? isPublic === true || isPublic === 'true'
607+
: undefined
559608
// Gated to update_ticket for the same reason as contentType and isPublic
560609
// above: the subBlock keeps its value when the operation changes, so
561610
// stale (or half-typed) JSON left behind after switching away from
@@ -576,12 +625,8 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
576625
: params.operation === 'update_ticket'
577626
? rawPriority
578627
: undefined
579-
if (activeStatus !== undefined && activeStatus !== null && activeStatus !== '') {
580-
result.status = activeStatus
581-
}
582-
if (activePriority !== undefined && activePriority !== null && activePriority !== '') {
583-
result.priority = activePriority
584-
}
628+
result.status = orUndefined(activeStatus)
629+
result.priority = orUndefined(activePriority)
585630

586631
// sortBy and include are per-operation for the same reason: three list
587632
// endpoints accept three different sort fields, and get_contact accepts a
@@ -596,44 +641,32 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
596641
: params.operation === 'list_threads'
597642
? rawThreadSortBy
598643
: undefined
599-
if (activeSortBy !== undefined && activeSortBy !== null && activeSortBy !== '') {
600-
result.sortBy = activeSortBy
601-
}
644+
result.sortBy = orUndefined(activeSortBy)
602645

603646
const activeInclude =
604-
params.operation === 'list_tickets' || params.operation === 'get_ticket'
647+
params.operation === 'list_tickets'
605648
? rawInclude
606-
: params.operation === 'get_contact'
607-
? rawContactInclude
608-
: params.operation === 'get_thread'
609-
? rawThreadInclude
610-
: undefined
611-
if (activeInclude !== undefined && activeInclude !== null && activeInclude !== '') {
612-
result.include = activeInclude
613-
}
649+
: params.operation === 'get_ticket'
650+
? rawTicketInclude
651+
: params.operation === 'get_contact'
652+
? rawContactInclude
653+
: params.operation === 'get_thread'
654+
? rawThreadInclude
655+
: undefined
656+
result.include = orUndefined(activeInclude)
614657

615658
// Gated for the same stale-value reason as the filters above: these three
616659
// are list_tickets-only query params, and no other operation declares them.
617-
if (params.operation === 'list_tickets') {
618-
if (typeof rawAssigneeFilter === 'string' && rawAssigneeFilter.trim()) {
619-
result.assignee = rawAssigneeFilter.trim()
620-
}
621-
if (typeof rawChannelFilter === 'string' && rawChannelFilter.trim()) {
622-
result.channel = rawChannelFilter.trim()
623-
}
624-
// Forward whatever was supplied and let the tool judge it. Filtering
625-
// here on shape would swallow 30.5 or a non-numeric value, and the
626-
// tool would then run without the filter and return the entire queue
627-
// as though the requested window had applied. Only the empty
628-
// "Any time" option is dropped, because that genuinely means no filter.
629-
if (
630-
rawReceivedInDays !== undefined &&
631-
rawReceivedInDays !== null &&
632-
rawReceivedInDays !== ''
633-
) {
634-
result.receivedInDays = Number(rawReceivedInDays)
635-
}
636-
}
660+
const isListTickets = params.operation === 'list_tickets'
661+
result.assignee = isListTickets ? orUndefined(rawAssigneeFilter) : undefined
662+
result.channel = isListTickets ? orUndefined(rawChannelFilter) : undefined
663+
// Forward whatever was supplied and let the tool judge it. Filtering here
664+
// on shape would swallow 30.5 or a non-numeric value, and the tool would
665+
// then run without the filter and return the entire queue as though the
666+
// requested window had applied. Only the empty "Any time" option is
667+
// dropped, because that genuinely means no filter.
668+
const receivedInDays = isListTickets ? orUndefined(rawReceivedInDays) : undefined
669+
result.receivedInDays = receivedInDays === undefined ? undefined : Number(receivedInDays)
637670

638671
if (params.operation === 'update_ticket' && rawCustomFields !== undefined) {
639672
if (typeof rawCustomFields === 'string') {
@@ -686,7 +719,8 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
686719
customFields: { type: 'json', description: 'Custom field values' },
687720
href: { type: 'string', description: 'Attachment download href' },
688721
fileName: { type: 'string', description: 'Downloaded file name' },
689-
include: { type: 'string', description: 'Related data to include on ticket operations' },
722+
include: { type: 'string', description: 'Related data to include when listing tickets' },
723+
ticketInclude: { type: 'string', description: 'Related data to include on a single ticket' },
690724
contactInclude: { type: 'string', description: 'Related data to include on a contact' },
691725
threadInclude: { type: 'string', description: 'Related data to include on a thread' },
692726
sortBy: { type: 'string', description: 'Sort field for listing tickets' },

apps/sim/tools/generated/tool-metadata.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

apps/sim/tools/zoho_desk/list_tickets.test.ts

Lines changed: 108 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,25 @@ describe('zohoDeskListTicketsTool request url', () => {
5353
})
5454

5555
/**
56-
* The block maps subBlock values onto tool params, so a value the mapper drops
57-
* never reaches the tool's validation and the request silently runs unfiltered.
58-
* These cover the seam rather than either side of it.
56+
* Models the real block-to-tool seam. The executor merges the mapper's output
57+
* ON TOP of the serialized inputs (`{ ...inputs, ...transformedParams }` in
58+
* executor/handlers/generic/generic-handler.ts), so the mapper can overwrite a
59+
* key but never remove one. A test that passes only the mapper's return value
60+
* would miss every stale input the merge restores — which is precisely the
61+
* class of bug these cover.
5962
*/
60-
describe('ZohoDeskBlock receivedInDays reaches the tool intact', () => {
63+
function throughSeam(inputs: Record<string, unknown>): string {
6164
const buildParams = ZohoDeskBlock.tools.config?.params
65+
if (!buildParams) throw new Error('ZohoDeskBlock is missing tools.config.params')
6266
const buildUrl = zohoDeskListTicketsTool.request.url as (p: Record<string, unknown>) => string
67+
const withCreds = { accessToken: 'tok', orgId: '700', ...inputs }
68+
const transformed = buildParams(withCreds) as Record<string, unknown>
69+
return buildUrl({ ...withCreds, ...transformed })
70+
}
6371

64-
const throughBlock = (receivedInDays: unknown) => {
65-
if (!buildParams) throw new Error('ZohoDeskBlock is missing tools.config.params')
66-
const mapped = buildParams({ operation: 'list_tickets', receivedInDays }) as Record<
67-
string,
68-
unknown
69-
>
70-
return buildUrl({ accessToken: 'tok', orgId: '700', ...mapped })
71-
}
72+
describe('ZohoDeskBlock receivedInDays reaches the tool intact', () => {
73+
const throughBlock = (receivedInDays: unknown) =>
74+
throughSeam({ operation: 'list_tickets', receivedInDays })
7275

7376
it('carries a supported window through to the query', () => {
7477
expect(new URL(throughBlock('30')).searchParams.get('receivedInDays')).toBe('30')
@@ -84,3 +87,96 @@ describe('ZohoDeskBlock receivedInDays reaches the tool intact', () => {
8487
expect(new URL(throughBlock('')).searchParams.has('receivedInDays')).toBe(false)
8588
})
8689
})
90+
91+
/**
92+
* Get Ticket accepts `contract` and `skills`; List Tickets does not document
93+
* either. A shared subBlock would carry them across an operation switch and put
94+
* an undocumented token on the wire, so each endpoint owns its own field.
95+
*/
96+
describe('ZohoDeskBlock keeps the two include vocabularies apart', () => {
97+
const buildParams = ZohoDeskBlock.tools.config?.params
98+
99+
const merged = (operation: string) => {
100+
if (!buildParams) throw new Error('ZohoDeskBlock is missing tools.config.params')
101+
const inputs = {
102+
operation,
103+
include: 'contacts,assignee',
104+
ticketInclude: 'contacts,skills',
105+
}
106+
return { ...inputs, ...(buildParams(inputs) as Record<string, unknown>) }
107+
}
108+
109+
it('sends the list vocabulary to list_tickets', () => {
110+
expect(merged('list_tickets').include).toBe('contacts,assignee')
111+
})
112+
113+
it('sends the single-ticket vocabulary to get_ticket', () => {
114+
expect(merged('get_ticket').include).toBe('contacts,skills')
115+
})
116+
117+
it('never leaks skills onto the list endpoint', () => {
118+
expect(merged('list_tickets').include).not.toContain('skills')
119+
})
120+
})
121+
122+
/**
123+
* A `mode: 'advanced'` subBlock keeps its value when the operation changes, and
124+
* the serializer emits it for ANY operation while the block's advanced toggle is
125+
* off — it returns on `isNonEmptyValue` without evaluating the subBlock's
126+
* `condition`. So these stale values genuinely arrive under the wrong operation,
127+
* and the mapper has to overwrite them rather than merely decline to set them.
128+
*/
129+
describe('ZohoDeskBlock overwrites stale advanced values from another operation', () => {
130+
const buildParams = ZohoDeskBlock.tools.config?.params
131+
132+
const merged = (inputs: Record<string, unknown>) => {
133+
if (!buildParams) throw new Error('ZohoDeskBlock is missing tools.config.params')
134+
return { ...inputs, ...(buildParams(inputs) as Record<string, unknown>) }
135+
}
136+
137+
it('does not carry a List Tickets sort field onto List Comments', () => {
138+
const result = merged({ operation: 'list_comments', ticketId: '1', sortBy: 'createdTime' })
139+
expect(result.sortBy).toBeUndefined()
140+
})
141+
142+
it('does not carry a ticket include onto Get Contact or Get Thread', () => {
143+
expect(
144+
merged({ operation: 'get_contact', contactId: '9', include: 'contacts,assignee' }).include
145+
).toBeUndefined()
146+
expect(
147+
merged({ operation: 'get_thread', ticketId: '1', threadId: '2', include: 'contacts' }).include
148+
).toBeUndefined()
149+
})
150+
151+
it('does not carry list-only filters onto another operation', () => {
152+
const result = merged({
153+
operation: 'get_ticket',
154+
ticketId: '1',
155+
assigneeFilter: 'Unassigned',
156+
channelFilter: 'Email',
157+
receivedInDays: '30',
158+
})
159+
expect(result.assignee).toBeUndefined()
160+
expect(result.channel).toBeUndefined()
161+
expect(result.receivedInDays).toBeUndefined()
162+
})
163+
164+
it('discards an out-of-range pagination value instead of forwarding it', () => {
165+
const result = merged({ operation: 'list_tickets', from: '-5', limit: '0' })
166+
expect(result.from).toBeUndefined()
167+
expect(result.limit).toBeUndefined()
168+
})
169+
170+
it('survives an emptied department multi-select without throwing', () => {
171+
expect(() => throughSeam({ operation: 'list_tickets', departmentIds: [] })).not.toThrow()
172+
expect(
173+
new URL(throughSeam({ operation: 'list_tickets', departmentIds: [] })).searchParams.has(
174+
'departmentIds'
175+
)
176+
).toBe(false)
177+
})
178+
179+
it('does not throw on the "Any time" option the dropdown seeds', () => {
180+
expect(() => throughSeam({ operation: 'list_tickets', receivedInDays: '' })).not.toThrow()
181+
})
182+
})

apps/sim/tools/zoho_desk/list_tickets.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const zohoDeskListTicketsTool: ToolConfig<ZohoDeskListTicketsParams, Zoho
4949
type: 'number',
5050
required: false,
5151
visibility: 'user-or-llm',
52-
description: 'Number of tickets to return (1-100, default 10)',
52+
description: 'Number of tickets to return (1-100)',
5353
},
5454
departmentIds: {
5555
type: 'string',
@@ -82,7 +82,7 @@ export const zohoDeskListTicketsTool: ToolConfig<ZohoDeskListTicketsParams, Zoho
8282
required: false,
8383
visibility: 'user-or-llm',
8484
description:
85-
'Filter by origin channel (e.g. Email, Web, Phone). Comma-separate to match multiple.',
85+
'Filter by origin channel, spelled as your portal spells it. Comma-separate to match multiple.',
8686
},
8787
receivedInDays: {
8888
type: 'number',

0 commit comments

Comments
 (0)