Skip to content

Commit e17deb6

Browse files
committed
fix(atlassian): address review findings
Keep trailing AQL and CQL order by clauses after scoped filters, honor escaped quotes, reject JSM approvalStatus/organizationId outside their ownership modes, move Trello card outputs into types.ts so the docs render them, and drop the now-unused Confluence COMMENTS_OUTPUT.
1 parent a60cd93 commit e17deb6

17 files changed

Lines changed: 602 additions & 224 deletions

File tree

‎apps/docs/content/docs/integrations/trello.mdx‎

Lines changed: 315 additions & 5 deletions
Large diffs are not rendered by default.

‎apps/sim/blocks/blocks/bitbucket.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,7 @@ export const BitbucketBlock: BlockConfig = {
815815
},
816816
{
817817
id: 'targetRef',
818-
title: 'Branch or Tag',
818+
title: 'Branch',
819819
type: 'short-input',
820820
required: true,
821821
condition: { field: 'operation', value: 'bitbucket_trigger_pipeline' },

‎apps/sim/lib/internal/confluence/operations.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,26 @@ describe('Confluence operations', () => {
178178
})
179179
})
180180

181+
it('keeps a trailing CQL order by after the parenthesized filter', async () => {
182+
const fetchMock = vi.fn(async () => Response.json({ results: [], totalSize: 0, _links: {} }))
183+
vi.stubGlobal('fetch', fetchMock)
184+
185+
await executeConfluenceSearch(
186+
{
187+
...CONNECTION,
188+
query: 'release',
189+
limit: 25,
190+
cql: 'title ~ "sort order by x" ORDER BY lastmodified desc',
191+
},
192+
{ headers: new Headers(), requestId: 'request-1' }
193+
)
194+
195+
const sentCql = new URL(String(fetchMock.mock.calls[0]?.[0])).searchParams.get('cql')
196+
expect(sentCql).toBe(
197+
'text ~ "release" AND (title ~ "sort order by x") ORDER BY lastmodified desc'
198+
)
199+
})
200+
181201
it('fails closed when stored-file authorization denies access', async () => {
182202
mockAssertToolFileAccess.mockResolvedValueOnce(Response.json({ error: 'Forbidden' }))
183203

‎apps/sim/lib/internal/confluence/operations.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,35 @@ export async function executeConfluenceGetPagesByLabel(
905905
}
906906
}
907907

908+
const CQL_ORDER_BY = /\s*\border\s+by\s/iy
909+
910+
/**
911+
* Splits a trailing `order by` clause off a CQL fragment so the condition can be
912+
* parenthesized while the ordering stays at the end, ignoring quoted values.
913+
*/
914+
function splitCqlOrderBy(cql: string): { condition: string; orderBy: string } {
915+
let quote: string | null = null
916+
let orderByStart = -1
917+
for (let index = 0; index < cql.length; index++) {
918+
const char = cql[index]
919+
if (char === '"' || char === "'") {
920+
let backslashes = 0
921+
for (let cursor = index - 1; cursor >= 0 && cql[cursor] === '\\'; cursor--) backslashes++
922+
if (backslashes % 2 === 0) {
923+
if (quote === null) quote = char
924+
else if (quote === char) quote = null
925+
}
926+
continue
927+
}
928+
if (quote !== null) continue
929+
if (index > 0 && !/\s/.test(cql[index - 1])) continue
930+
CQL_ORDER_BY.lastIndex = index
931+
if (CQL_ORDER_BY.test(cql)) orderByStart = index
932+
}
933+
if (orderByStart === -1) return { condition: cql, orderBy: '' }
934+
return { condition: cql.slice(0, orderByStart), orderBy: ` ${cql.slice(orderByStart).trim()}` }
935+
}
936+
908937
function escapeCql(value: string): string {
909938
return value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')
910939
}
@@ -916,7 +945,11 @@ export async function executeConfluenceSearch(
916945
assertCursor(input.cursor)
917946
const client = await createConfluenceClient(input, context.signal)
918947
let cql = `text ~ "${escapeCql(input.query)}"`
919-
if (input.cql?.trim()) cql += ` AND (${input.cql.trim()})`
948+
const extraCql = input.cql?.trim()
949+
if (extraCql) {
950+
const { condition, orderBy } = splitCqlOrderBy(extraCql)
951+
cql += condition.trim() ? ` AND (${condition.trim()})${orderBy}` : orderBy
952+
}
920953
const query = new URLSearchParams({
921954
cql,
922955
limit: input.limit.toString(),

‎apps/sim/lib/internal/jsm/assets.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,17 @@ const AQL_ORDER_BY = /\s+order\s+by\s/iy
147147

148148
/**
149149
* Splits a trailing `order by` clause off an AQL query, ignoring matches inside
150-
* double-quoted values, since AQL requires the ordering to stay at the end.
150+
* double-quoted values (including escaped `\\"`), since AQL requires the ordering to
151+
* stay at the end.
151152
*/
152153
function splitAqlOrderBy(qlQuery: string): { condition: string; orderBy: string } {
153154
let inQuotes = false
154155
let orderByStart = -1
155156
for (let index = 0; index < qlQuery.length; index++) {
156157
if (qlQuery[index] === '"') {
157-
inQuotes = !inQuotes
158+
let backslashes = 0
159+
for (let cursor = index - 1; cursor >= 0 && qlQuery[cursor] === '\\'; cursor--) backslashes++
160+
if (backslashes % 2 === 0) inQuotes = !inQuotes
158161
continue
159162
}
160163
if (inQuotes) continue

‎apps/sim/lib/internal/jsm/operations.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,4 +167,26 @@ describe('JSM operations', () => {
167167
}),
168168
})
169169
})
170+
171+
it.each([
172+
[{ approvalStatus: 'MY_PENDING_APPROVAL', requestOwnership: 'OWNED_REQUESTS' }, 'APPROVER'],
173+
[{ organizationId: '3', requestOwnership: 'APPROVER' }, 'ORGANIZATION'],
174+
])(
175+
'rejects %o, which Atlassian only accepts with requestOwnership %s',
176+
async (filters, owner) => {
177+
await expect(executeJsmGetRequests({ ...BASE, ...filters })).rejects.toThrow(owner)
178+
}
179+
)
180+
181+
it('does not split an order by inside an escaped quote', async () => {
182+
mocks.client.json.mockResolvedValueOnce({ values: [], total: 0, isLast: 'true' })
183+
await executeJsmSearchObjectsAql({
184+
...BASE,
185+
qlQuery: 'Name = "say \\" order by \\" x" order by Name',
186+
objectTypeId: '7',
187+
})
188+
expect(JSON.parse(String(mocks.client.json.mock.calls.at(-1)?.[1]?.body))).toEqual({
189+
qlQuery: '(Name = "say \\" order by \\" x") AND objectTypeId = 7 order by Name',
190+
})
191+
})
170192
})

‎apps/sim/lib/internal/jsm/service-desk.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,13 @@ export async function executeJsmGetRequests(input: JsmRequestsBody, signal?: Abo
200200
const result = validateEnum(input.approvalStatus, APPROVAL_STATUS, 'approvalStatus')
201201
if (!result.isValid) throw new JsmOperationError(result.error || 'Invalid approvalStatus', 400)
202202
}
203+
if (input.approvalStatus && input.requestOwnership !== 'APPROVER') {
204+
throw new JsmOperationError('approvalStatus requires requestOwnership APPROVER', 400)
205+
}
203206
if (input.organizationId) validateId(input.organizationId, 'organizationId')
207+
if (input.organizationId && input.requestOwnership !== 'ORGANIZATION') {
208+
throw new JsmOperationError('organizationId requires requestOwnership ORGANIZATION', 400)
209+
}
204210
const client = await createJsmClient(input, signal)
205211
const query = new URLSearchParams()
206212
append(query, 'serviceDeskId', input.serviceDeskId)

‎apps/sim/tools/confluence/types.ts‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -245,18 +245,6 @@ export const COMMENT_OUTPUT: OutputProperty = {
245245
properties: COMMENT_ITEM_PROPERTIES,
246246
}
247247

248-
/**
249-
* Comments array output definition for list endpoints.
250-
*/
251-
export const COMMENTS_OUTPUT: OutputProperty = {
252-
type: 'array',
253-
description: 'Array of Confluence comments',
254-
items: {
255-
type: 'object',
256-
properties: COMMENT_ITEM_PROPERTIES,
257-
},
258-
}
259-
260248
/**
261249
* Attachment item properties from Confluence API v2.
262250
* Based on GET /wiki/api/v2/attachments response structure.

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

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

‎apps/sim/tools/trello/create_card.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@ import {
44
extractTrelloErrorMessage,
55
mapTrelloCard,
66
TRELLO_API_BASE_URL,
7-
TRELLO_CARD_OUTPUT_DESCRIPTION,
8-
TRELLO_CARD_OUTPUT_PROPERTIES,
97
} from '@/tools/trello/shared'
108
import type { TrelloCreateCardParams, TrelloCreateCardResponse } from '@/tools/trello/types'
9+
import { TRELLO_CARD_OUTPUT_PROPERTIES } from '@/tools/trello/types'
1110
import type { ToolConfig } from '@/tools/types'
1211

1312
export const trelloCreateCardTool: ToolConfig<TrelloCreateCardParams, TrelloCreateCardResponse> = {
@@ -176,7 +175,8 @@ export const trelloCreateCardTool: ToolConfig<TrelloCreateCardParams, TrelloCrea
176175
outputs: {
177176
card: {
178177
type: 'json',
179-
description: `Created card (${TRELLO_CARD_OUTPUT_DESCRIPTION})`,
178+
description:
179+
'Created card (id, name, desc, url, shortUrl, shortLink, idShort, idBoard, idList, closed, pos, labelIds, labels, idMembers, idChecklists, due, start, dueComplete, dateLastActivity, badges, checklists, attachments, members)',
180180
optional: true,
181181
properties: TRELLO_CARD_OUTPUT_PROPERTIES,
182182
},

0 commit comments

Comments
 (0)