fix(atlassian): address final review

Make Jira search heavy fields (ADF description, custom fields, attachments,
links, subtasks) opt-in through fields, move transformIssueData into jira/utils,
keep AQL order by splitting at parenthesis depth 0, let the object-types /flat
fallback never add a failure, send next=true with Confluence search cursors, and
harden Trello ID lists, Jira parent, and Confluence comment ids for agent calls.

Author: waleedlatif1

apps/docs/content/docs/integrations/jira.mdx

@@ -548,7 +548,7 @@ Search for Jira issues using JQL (Jira Query Language)
 | `jql` | string | Yes | JQL query string to search for issues \(e.g., "project = PROJ AND status = Open"\) |
 | `nextPageToken` | string | No | Cursor token for the next page of results. Omit for the first page. |
 | `maxResults` | number | No | Maximum number of results to return per page \(default: 50\) |
-| `fields` | array | No | Array of field names to return \(default: all fields\). |
+| `fields` | array | No | Array of field names to return \(default: all fields\). When set, each issue also includes descriptionAdf, customFields, attachments, issuelinks, and subtasks. |
 | `expand` | string | No | Comma-separated expansions: renderedFields, names, schema, transitions, operations, editmeta, changelog |
 
 #### Output
@@ -562,8 +562,8 @@ Search for Jira issues using JQL (Jira Query Language)
 |   ↳ `self` | string | REST API URL for this issue |
 |   ↳ `summary` | string | Issue summary |
 |   ↳ `description` | string | Issue description text \(extracted from ADF\) |
-|   ↳ `descriptionAdf` | json | Issue description as the raw Atlassian Document Format \(ADF\) document |
-|   ↳ `renderedDescription` | string | Issue description rendered as HTML \(requires the renderedFields expansion\) |
+|   ↳ `descriptionAdf` | json | Issue description as the raw Atlassian Document Format \(ADF\) document. Only included when the fields param is set |
+|   ↳ `renderedDescription` | string | Issue description rendered as HTML. Only included when expand includes renderedFields |
 |   ↳ `environment` | string | Environment text \(extracted from ADF\) |
 |   ↳ `status` | object | Issue status |
 |     ↳ `id` | string | Status ID |
@@ -644,7 +644,7 @@ Search for Jira issues using JQL (Jira Query Language)
 |     ↳ `id` | string | Parent issue ID |
 |     ↳ `key` | string | Parent issue key |
 |     ↳ `summary` | string | Parent issue summary |
-|   ↳ `issuelinks` | array | Linked issues |
+|   ↳ `issuelinks` | array | Linked issues. Only included when the fields param is set |
 |     ↳ `id` | string | Issue link ID |
 |     ↳ `type` | object | Link type information |
 |       ↳ `id` | string | Link type ID |
@@ -661,7 +661,7 @@ Search for Jira issues using JQL (Jira Query Language)
 |       ↳ `key` | string | Issue key |
 |       ↳ `statusName` | string | Issue status name |
 |       ↳ `summary` | string | Issue summary |
-|   ↳ `subtasks` | array | Subtask issues |
+|   ↳ `subtasks` | array | Subtask issues. Only included when the fields param is set |
 |     ↳ `id` | string | Subtask issue ID |
 |     ↳ `key` | string | Subtask issue key |
 |     ↳ `summary` | string | Subtask summary |
@@ -673,7 +673,7 @@ Search for Jira issues using JQL (Jira Query Language)
 |   ↳ `watches` | object | Watch information |
 |     ↳ `watchCount` | number | Number of watchers |
 |     ↳ `isWatching` | boolean | Whether the current user is watching |
-|   ↳ `attachments` | array | Attachment metadata \(no file contents\) |
+|   ↳ `attachments` | array | Attachment metadata \(no file contents\). Only included when the fields param is set |
 |     ↳ `id` | string | Attachment ID |
 |     ↳ `filename` | string | Attachment file name |
 |     ↳ `mimeType` | string | MIME type of the attachment |
@@ -690,7 +690,7 @@ Search for Jira issues using JQL (Jira Query Language)
 |       ↳ `timeZone` | string | User timezone |
 |     ↳ `authorName` | string | Attachment author display name |
 |     ↳ `created` | string | ISO 8601 timestamp when the attachment was created |
-|   ↳ `customFields` | json | Every customfield_* value Jira returned, keyed by field ID \(e.g., sprint, story points, team\). Use the names output or Get Fields to map IDs to names. |
+|   ↳ `customFields` | json | Every customfield_* value Jira returned, keyed by field ID \(e.g., sprint, story points, team\). Only included when the fields param is set; use the names output or Get Fields to map IDs to names. |
 | `nextPageToken` | string | Cursor token for the next page. Null when no more results. |
 | `isLast` | boolean | Whether this is the last page of results |
 | `names` | json | Map of field ID to display name \(e.g., customfield_10020 → "Sprint"\). Only returned when expand includes names. |

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

@@ -363,8 +363,8 @@ Update an existing card on Trello
 | `start` | string | No | Start date \(ISO 8601 format\) |
 | `pos` | string | No | Position of the card in its list \(top, bottom, or positive float\) |
 | `idBoard` | string | No | Trello board ID to move the card to \(use with idList\) |
-| `labelIds` | array | No | Replace the card labels with these label IDs. An empty list leaves labels unchanged; use Remove Label to take one off |
-| `memberIds` | array | No | Replace the card members with these member IDs. An empty list leaves members unchanged; use Remove Member to take one off |
+| `labelIds` | array | No | Replace the card labels with these label IDs \(array or comma-separated string\). An empty list leaves labels unchanged; use Remove Label to take one off |
+| `memberIds` | array | No | Replace the card members with these member IDs \(array or comma-separated string\). An empty list leaves members unchanged; use Remove Member to take one off |
 
 #### Output
 

apps/sim/blocks/blocks/jira.ts

@@ -1619,7 +1619,7 @@ Return ONLY the comment text - no explanations.`,
     issues: {
       type: 'json',
       description:
-        'Array of matching issues with key, summary, description, descriptionAdf, status, assignee, reporter, priority, labels, components, fixVersions, parent, subtasks, issuelinks, attachments, timetracking, customFields, dates',
+        'Array of matching issues with key, summary, description, status, assignee, reporter, priority, labels, components, fixVersions, parent, timetracking, dates. descriptionAdf, subtasks, issuelinks, attachments, and customFields are only included when Fields is set; renderedDescription needs the renderedFields expansion',
     },
 
     comments: {

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

@@ -200,6 +200,36 @@ describe('Confluence operations', () => {
     )
   })
 
+  it.each([
+    [
+      'search',
+      () =>
+        executeConfluenceSearch(
+          { ...CONNECTION, query: 'release', limit: 25, cursor: 'cursor-2' },
+          { headers: new Headers(), requestId: 'request-1' }
+        ),
+    ],
+    [
+      'search in space',
+      () =>
+        executeConfluenceSearchInSpace(
+          { ...CONNECTION, spaceKey: 'ENG', limit: 25, cursor: 'cursor-2' },
+          { headers: new Headers(), requestId: 'request-1' }
+        ),
+    ],
+  ])('sends next=true with the cursor on %s, as Confluence next links do', async (_, run) => {
+    const fetchMock = vi.fn<typeof fetch>(async () =>
+      Response.json({ results: [], totalSize: 0, _links: {} })
+    )
+    vi.stubGlobal('fetch', fetchMock)
+
+    await run()
+
+    const params = new URL(String(fetchMock.mock.calls.at(-1)?.[0])).searchParams
+    expect(params.get('cursor')).toBe('cursor-2')
+    expect(params.get('next')).toBe('true')
+  })
+
   it('fails closed when stored-file authorization denies access', async () => {
     mockAssertToolFileAccess.mockResolvedValueOnce(Response.json({ error: 'Forbidden' }))
 

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

@@ -938,6 +938,16 @@ function escapeCql(value: string): string {
   return value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')
 }
 
+/**
+ * Pages the v1 `/search` endpoint forward. Confluence's own `_links.next` URLs carry
+ * `next=true` alongside the cursor, so a resumed page sends both.
+ */
+function setSearchCursor(query: URLSearchParams, cursor: string | undefined) {
+  if (!cursor) return
+  query.set('next', 'true')
+  query.set('cursor', cursor)
+}
+
 export async function executeConfluenceSearch(
   input: ConfluenceSearchBody,
   context: ConfluenceOperationContext
@@ -955,7 +965,7 @@ export async function executeConfluenceSearch(
     limit: input.limit.toString(),
     expand: 'content.space',
   })
-  if (input.cursor) query.set('cursor', input.cursor)
+  setSearchCursor(query, input.cursor)
   const data = await client.json(client.rest(`/search?${query}`), {}, context.signal)
   const results = toArray(data.results).map((value) => {
     const result = toRecord(value)
@@ -1005,7 +1015,7 @@ export async function executeConfluenceSearchInSpace(
   if (input.contentType) cql += ` AND type = "${escapeCql(input.contentType)}"`
   assertCursor(input.cursor)
   const query = new URLSearchParams({ cql, limit: cappedLimit(input.limit) })
-  if (input.cursor) query.set('cursor', input.cursor)
+  setSearchCursor(query, input.cursor)
   const data = await client.json(client.rest(`/search?${query}`), {}, context.signal)
   const results = toArray(data.results).map((value) => {
     const result = toRecord(value)

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

@@ -1,3 +1,5 @@
+import { createLogger } from '@sim/logger'
+import { getErrorMessage } from '@sim/utils/errors'
 import { toArray, toRecord } from '@sim/utils/object'
 import type { ContractBody } from '@/lib/api/contracts'
 import type {
@@ -15,6 +17,8 @@ import { createJsmAssetsClient } from '@/lib/internal/jsm/client'
 import { JsmOperationError } from '@/lib/internal/jsm/errors'
 import { mapAssetObject } from '@/tools/jsm/utils'
 
+const logger = createLogger('JsmAssets')
+
 type ListObjectSchemasInput = ContractBody<typeof jsmListObjectSchemasContract>
 type GetObjectSchemaInput = ContractBody<typeof jsmGetObjectSchemaContract>
 type ListObjectTypesInput = ContractBody<typeof jsmListObjectTypesContract>
@@ -85,6 +89,36 @@ function toObjectTypeList(value: unknown): unknown[] {
  * documented `/flat` listing, which returns every type in the schema, with
  * `excludeAbstract` applied client-side because `/flat` does not accept it.
  */
+function isAbort(error: unknown, signal?: AbortSignal): boolean {
+  if (signal?.aborted) return true
+  return error instanceof Error && (error.name === 'AbortError' || error.name === 'TimeoutError')
+}
+
+/**
+ * Reads the flat object-types listing used as a fallback for an empty schema listing.
+ * A failure other than an abort yields an empty list, so the fallback never fails a
+ * request the primary listing already answered.
+ */
+async function listFlatObjectTypes(
+  client: Awaited<ReturnType<typeof createJsmAssetsClient>>,
+  schemaPath: string,
+  schemaId: string,
+  signal?: AbortSignal
+): Promise<unknown[]> {
+  try {
+    return toObjectTypeList(
+      await client.value(client.assets(`${schemaPath}/flat`), {}, signal, true)
+    )
+  } catch (error) {
+    if (isAbort(error, signal)) throw error
+    logger.warn('Flat object-types fallback failed, returning the empty schema listing', {
+      schemaId,
+      error: getErrorMessage(error),
+    })
+    return []
+  }
+}
+
 export async function executeJsmListObjectTypes(input: ListObjectTypesInput, signal?: AbortSignal) {
   const client = await createJsmAssetsClient(input, signal)
   const query = new URLSearchParams()
@@ -101,9 +135,7 @@ export async function executeJsmListObjectTypes(input: ListObjectTypesInput, sig
     )
   )
   if (objectTypes.length === 0) {
-    const flat = toObjectTypeList(
-      await client.value(client.assets(`${schemaPath}/flat`), {}, signal, true)
-    )
+    const flat = await listFlatObjectTypes(client, schemaPath, input.schemaId, signal)
     objectTypes =
       String(input.excludeAbstract) === 'true'
         ? flat.filter((objectType) => toRecord(objectType).abstractObjectType !== true)
@@ -143,24 +175,32 @@ export async function executeJsmGetObjectTypeAttributes(
 
 const ASSETS_NUMERIC_ID = /^\d+$/
 
-const AQL_ORDER_BY = /\s+order\s+by\s/iy
+const AQL_ORDER_BY = /order\s+by\s/iy
+const AQL_CLAUSE_BOUNDARY = /[\s)"]/
 
 /**
- * Splits a trailing `order by` clause off an AQL query, ignoring matches inside
- * double-quoted values (including escaped `\\"`), since AQL requires the ordering to
- * stay at the end.
+ * Splits a trailing `order by` clause off an AQL query, since AQL requires the ordering
+ * to stay at the end. Only a clause at parenthesis depth 0 and outside double-quoted
+ * values (including escaped `\\"`) counts, so `order by` inside a function argument or
+ * a string stays in the condition.
  */
 function splitAqlOrderBy(qlQuery: string): { condition: string; orderBy: string } {
   let inQuotes = false
+  let depth = 0
   let orderByStart = -1
   for (let index = 0; index < qlQuery.length; index++) {
-    if (qlQuery[index] === '"') {
+    const char = qlQuery[index]
+    if (char === '"') {
       let backslashes = 0
       for (let cursor = index - 1; cursor >= 0 && qlQuery[cursor] === '\\'; cursor--) backslashes++
       if (backslashes % 2 === 0) inQuotes = !inQuotes
       continue
     }
     if (inQuotes) continue
+    if (char === '(') depth++
+    else if (char === ')') depth = Math.max(0, depth - 1)
+    if (depth !== 0) continue
+    if (index > 0 && !AQL_CLAUSE_BOUNDARY.test(qlQuery[index - 1] ?? '')) continue
     AQL_ORDER_BY.lastIndex = index
     if (AQL_ORDER_BY.test(qlQuery)) orderByStart = index
   }
@@ -185,7 +225,11 @@ function scopeAqlQuery(input: SearchObjectsAqlInput): string {
   ].filter((scope): scope is string => scope !== null)
   if (scopes.length === 0) return input.qlQuery
   const { condition, orderBy } = splitAqlOrderBy(input.qlQuery)
-  return `(${condition.trim()}) AND ${scopes.join(' AND ')}${orderBy}`
+  const trimmedCondition = condition.trim()
+  const scoped = trimmedCondition
+    ? `(${trimmedCondition}) AND ${scopes.join(' AND ')}`
+    : scopes.join(' AND ')
+  return orderBy ? `${scoped} ${orderBy}` : scoped
 }
 
 export async function executeJsmSearchObjectsAql(

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

@@ -1,3 +1,4 @@
+import { getMockLogger } from '@sim/testing/mocks/logger.mock'
 import { describe, expect, it, vi } from 'vitest'
 
 const mocks = vi.hoisted(() => {
@@ -189,4 +190,48 @@ describe('JSM operations', () => {
       qlQuery: '(Name = "say \\" order by \\" x") AND objectTypeId = 7 order by Name',
     })
   })
+
+  it.each([
+    ['order by Name', 'objectTypeId = 7 order by Name'],
+    ['  ORDER BY "Created Date" desc', 'objectTypeId = 7 ORDER BY "Created Date" desc'],
+    [
+      'object having inboundReferences(objectType = Host order by Name)',
+      '(object having inboundReferences(objectType = Host order by Name)) AND objectTypeId = 7',
+    ],
+    [
+      'object having inboundReferences(objectType = Host order by Name) order by Key',
+      '(object having inboundReferences(objectType = Host order by Name)) AND objectTypeId = 7 order by Key',
+    ],
+  ])('scopes the AQL query %j as %j', async (qlQuery, expected) => {
+    mocks.client.json.mockResolvedValueOnce({ values: [], total: 0, isLast: 'true' })
+    await executeJsmSearchObjectsAql({ ...BASE, qlQuery, objectTypeId: '7' })
+    expect(JSON.parse(String(mocks.client.json.mock.calls.at(-1)?.[1]?.body))).toEqual({
+      qlQuery: expected,
+    })
+  })
+
+  it('returns the empty schema listing when the flat fallback fails', async () => {
+    mocks.client.value.mockReset()
+    mocks.client.value
+      .mockResolvedValueOnce([])
+      .mockRejectedValueOnce(new Error('Jira request failed with status 404'))
+    const result = await executeJsmListObjectTypes({ ...BASE, schemaId: '6' })
+    expect(result.output).toMatchObject({ objectTypes: [], total: 0 })
+    expect(getMockLogger('JsmAssets').warn).toHaveBeenCalledWith(
+      expect.any(String),
+      expect.objectContaining({ schemaId: '6' })
+    )
+  })
+
+  it('still propagates an abort from the flat fallback', async () => {
+    const controller = new AbortController()
+    mocks.client.value.mockReset()
+    mocks.client.value.mockResolvedValueOnce([]).mockImplementationOnce(async () => {
+      controller.abort()
+      throw new DOMException('The operation was aborted.', 'AbortError')
+    })
+    await expect(
+      executeJsmListObjectTypes({ ...BASE, schemaId: '6' }, controller.signal)
+    ).rejects.toThrow('aborted')
+  })
 })

apps/sim/tools/confluence/create_comment.test.ts

@@ -0,0 +1,26 @@
+import { describe, expect, it } from 'vitest'
+import {
+  type ConfluenceCreateCommentParams,
+  confluenceCreateCommentTool,
+} from '@/tools/confluence/create_comment'
+
+function operationInput(parentCommentId: unknown) {
+  return confluenceCreateCommentTool.operation.input({
+    accessToken: 'token',
+    domain: 'example.atlassian.net',
+    pageId: '123',
+    comment: 'Reply',
+    parentCommentId,
+  } as ConfluenceCreateCommentParams) as Record<string, unknown>
+}
+
+describe('confluence_create_comment operation input', () => {
+  it.each([
+    [456, '456'],
+    [' 456 ', '456'],
+    ['', undefined],
+    [undefined, undefined],
+  ])('normalizes parentCommentId %j to %j', (parentCommentId, expected) => {
+    expect(operationInput(parentCommentId).parentCommentId).toBe(expected)
+  })
+})

apps/sim/tools/confluence/create_comment.ts

@@ -86,7 +86,7 @@ export const confluenceCreateCommentTool: InternalToolConfig<
         cloudId: params.cloudId,
         pageId: params.pageId,
         comment: params.comment,
-        parentCommentId: params.parentCommentId?.trim() || undefined,
+        parentCommentId: String(params.parentCommentId ?? '').trim() || undefined,
       }
     },
   },