From cb0de8e584b7756a522a84b351ab65a087db953d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 11 Aug 2026 08:36:44 -0700 Subject: [PATCH 1/3] fix(atlassian): share one cached, retrying cloudId resolver across Jira, Confluence, and JSM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Jira, Confluence, and JSM tool re-resolved its site `cloudId` from `accessible-resources` on each invocation, so a run touching several Atlassian blocks paid a round trip per block and failed outright if any one of them caught a transient fault. A single Atlassian 500 took down a production run this way: the shared retry predicate covers 429/502/503/504 but not 500, so the call was never replayed. Four hand-rolled copies of that lookup now read through one memoized resolver. It caches the promise rather than the value, so concurrent callers join a lookup already in flight and a rejection is evicted instead of pinned for the TTL. Only an exact domain match is retained — a single-site fallback is a property of the calling token, not of the domain, so it answers its own caller without answering the next one. Discovery is an idempotent GET, so it replays transient 5xx. That is scoped here rather than widened into the shared predicate, which also guards non-idempotent writes; it also keeps the failure out of a whole-block replay, which would re-run a write a JSM block had already performed. The budget is tighter than the shared ~31s default — four attempts across ~3.5s — and the request carries a timeout so a wedged fetch cannot strand the callers joined to it. Two defects fall out of the consolidation. `getConfluenceCloudId` never checked the response status, so a 500 parsed as JSON, failed the array check, and surfaced as `No Confluence resources found` — pointing at site permissions rather than the transient fault. `getAssetsWorkspaceId` used a bare fetch with no retry and no cache, leaving the Assets path with two uncached discovery hops. --- apps/sim/lib/atlassian/discovery.test.ts | 181 ++++++++++++++++++ apps/sim/lib/atlassian/discovery.ts | 227 +++++++++++++++++++++++ apps/sim/tools/confluence/utils.ts | 61 ++---- apps/sim/tools/jira/bulk_read.ts | 25 +-- apps/sim/tools/jira/utils.ts | 55 +----- apps/sim/tools/jsm/utils.ts | 43 +++-- 6 files changed, 461 insertions(+), 131 deletions(-) create mode 100644 apps/sim/lib/atlassian/discovery.test.ts create mode 100644 apps/sim/lib/atlassian/discovery.ts diff --git a/apps/sim/lib/atlassian/discovery.test.ts b/apps/sim/lib/atlassian/discovery.test.ts new file mode 100644 index 00000000000..1a7d49fec27 --- /dev/null +++ b/apps/sim/lib/atlassian/discovery.test.ts @@ -0,0 +1,181 @@ +/** + * @vitest-environment node + */ +import { createMockResponse } from '@sim/testing' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + clearAtlassianCloudIdCache, + normalizeAtlassianSiteUrl, + resolveAtlassianCloudId, +} from '@/lib/atlassian/discovery' + +const SITE = 'https://acme.atlassian.net' +const CLOUD_ID = 'cloud-abc' + +/** Options for the site under test; override only the field a case is exercising. */ +function options(over: Record = {}) { + return { domain: 'acme.atlassian.net', accessToken: 't', product: 'Jira', ...over } as Parameters< + typeof resolveAtlassianCloudId + >[0] +} + +/** Tiny delays so retry cases do not spend real seconds sleeping. */ +const FAST = { initialDelayMs: 1, maxDelayMs: 1 } + +function sites(entries: Array<{ id: string; url: string }>) { + return createMockResponse({ json: entries }) +} + +function failure(status: number, body: unknown = { key: 'unexpectedError' }) { + return createMockResponse({ status, json: body }) +} + +let fetchMock: ReturnType + +beforeEach(() => { + clearAtlassianCloudIdCache() + fetchMock = vi.fn() + vi.stubGlobal('fetch', fetchMock) +}) + +afterEach(() => { + vi.unstubAllGlobals() + vi.restoreAllMocks() +}) + +describe('normalizeAtlassianSiteUrl', () => { + it.each([ + ['acme.atlassian.net', SITE], + ['https://acme.atlassian.net', SITE], + ['http://ACME.atlassian.net/', SITE], + [' acme.atlassian.net// ', SITE], + ])('normalizes %s', (input, expected) => { + expect(normalizeAtlassianSiteUrl(input)).toBe(expected) + }) +}) + +describe('resolveAtlassianCloudId', () => { + it('resolves an exact domain match', async () => { + fetchMock.mockResolvedValue(sites([{ id: CLOUD_ID, url: SITE }])) + + await expect(resolveAtlassianCloudId(options())).resolves.toBe(CLOUD_ID) + }) + + it('serves a repeat lookup from cache without a second request', async () => { + fetchMock.mockResolvedValue(sites([{ id: CLOUD_ID, url: SITE }])) + + await resolveAtlassianCloudId(options()) + await resolveAtlassianCloudId(options()) + + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('collapses concurrent lookups into one request', async () => { + fetchMock.mockResolvedValue(sites([{ id: CLOUD_ID, url: SITE }])) + + const resolved = await Promise.all([ + resolveAtlassianCloudId(options()), + resolveAtlassianCloudId(options()), + resolveAtlassianCloudId(options()), + ]) + + expect(resolved).toEqual([CLOUD_ID, CLOUD_ID, CLOUD_ID]) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it.each([500, 503, 507])( + 'retries a %i and succeeds, instead of failing the call', + async (status) => { + fetchMock + .mockResolvedValueOnce(failure(status)) + .mockResolvedValueOnce(sites([{ id: CLOUD_ID, url: SITE }])) + + await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).resolves.toBe(CLOUD_ID) + } + ) + + it('keeps the transient-5xx condition when a caller tunes the retry budget', async () => { + fetchMock + .mockResolvedValueOnce(failure(500)) + .mockResolvedValueOnce(sites([{ id: CLOUD_ID, url: SITE }])) + + // Shaped like VALIDATE_RETRY_OPTIONS: counts only, no retryCondition. + await expect( + resolveAtlassianCloudId(options({ retryOptions: { maxRetries: 3, ...FAST } })) + ).resolves.toBe(CLOUD_ID) + }) + + it('gives up on a persistent fault within a bounded attempt budget', async () => { + fetchMock.mockImplementation(async () => failure(500)) + + // Delays only. `maxRetries` still comes from the discovery budget, so this + // fails if the shared default of 5 ever leaks back in. + await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).rejects.toThrow( + /Failed to fetch Jira accessible resources: 500/ + ) + expect(fetchMock).toHaveBeenCalledTimes(4) + }) + + it('does not retry a client error', async () => { + fetchMock.mockResolvedValue(failure(403, { message: 'nope' })) + + await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).rejects.toThrow( + /Failed to fetch Jira accessible resources: 403/ + ) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('does not pin a failure in the cache', async () => { + fetchMock.mockResolvedValueOnce(failure(403, { message: 'nope' })) + await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).rejects.toThrow() + + fetchMock.mockResolvedValueOnce(sites([{ id: CLOUD_ID, url: SITE }])) + await expect(resolveAtlassianCloudId(options())).resolves.toBe(CLOUD_ID) + expect(fetchMock).toHaveBeenCalledTimes(2) + }) + + it('surfaces a non-OK status rather than reporting no resources', async () => { + fetchMock.mockImplementation(async () => failure(500)) + + await expect( + resolveAtlassianCloudId(options({ product: 'Confluence', retryOptions: FAST })) + ).rejects.toThrow(/Failed to fetch Confluence accessible resources: 500/) + }) + + it('does not retain a single-site fallback, since it is token-specific', async () => { + fetchMock.mockImplementation(async () => + sites([{ id: 'other-cloud', url: 'https://other.atlassian.net' }]) + ) + + await expect(resolveAtlassianCloudId(options())).resolves.toBe('other-cloud') + await resolveAtlassianCloudId(options()) + + expect(fetchMock).toHaveBeenCalledTimes(2) + }) + + it('rejects rather than throwing synchronously on a missing domain', async () => { + const call = resolveAtlassianCloudId(options({ domain: undefined })) + + await expect(call).rejects.toThrow() + expect(fetchMock).not.toHaveBeenCalled() + }) + + it('reports the available sites when several are accessible and none match', async () => { + fetchMock.mockResolvedValue( + sites([ + { id: 'a', url: 'https://one.atlassian.net' }, + { id: 'b', url: 'https://two.atlassian.net' }, + ]) + ) + + await expect(resolveAtlassianCloudId(options())).rejects.toThrow( + /Available sites: https:\/\/one.atlassian.net, https:\/\/two.atlassian.net/ + ) + }) + + it('rejects when the token can see no sites', async () => { + fetchMock.mockResolvedValue(sites([])) + + await expect(resolveAtlassianCloudId(options())).rejects.toThrow('No Jira resources found') + }) +}) diff --git a/apps/sim/lib/atlassian/discovery.ts b/apps/sim/lib/atlassian/discovery.ts new file mode 100644 index 00000000000..ee5822564bc --- /dev/null +++ b/apps/sim/lib/atlassian/discovery.ts @@ -0,0 +1,227 @@ +import { parseRetryAfter } from '@sim/utils/retry' +import { LRUCache } from 'lru-cache' +import { + type HTTPError, + isRetryableError, + type RetryOptions, + retryWithExponentialBackoff, +} from '@/lib/knowledge/documents/utils' + +const ACCESSIBLE_RESOURCES_URL = 'https://api.atlassian.com/oauth/token/accessible-resources' + +const DISCOVERY_REQUEST_TIMEOUT_MS = 10_000 + +/** + * A site's `cloudId` is a property of the site rather than of the caller — the + * unauthenticated `https://{domain}/_edge/tenant_info` endpoint serves the same + * value — which is what makes one entry safe to share across callers. Every other + * use of this cache justifies its own key at the call site. + * + * Bounded and short-lived on purpose: `max` stops a long-lived replica from + * accumulating an entry for every tenant it has ever served, and the TTL lets a + * re-pointed site be picked up without a redeploy. + */ +const DISCOVERY_CACHE_MAX_ENTRIES = 64 +const DISCOVERY_CACHE_TTL_MS = 10 * 60 * 1000 + +/** + * Builds a memo for one kind of discovery answer. Call at module scope — an + * `LRUCache` preallocates its backing arrays, so a per-request instance would + * both leak and defeat the point. + */ +export function createAtlassianDiscoveryCache() { + const cache = new LRUCache>({ + max: DISCOVERY_CACHE_MAX_ENTRIES, + ttl: DISCOVERY_CACHE_TTL_MS, + }) + + return { + /** + * Returns the memoized answer for `key`, starting `resolver` on a miss. + * + * The promise — not the resolved value — is stored, so callers arriving while + * a lookup is in flight join it instead of starting their own, and a rejection + * is evicted rather than pinned for the TTL. `retain: false` drops the entry + * once it settles, so a token-specific result is not served to later callers. + */ + resolve( + key: string, + resolver: () => Promise<{ value: string; retain: boolean }> + ): Promise { + const cached = cache.get(key) + if (cached) return cached + + const promise = resolver() + .then(({ value, retain }) => { + if (!retain) cache.delete(key) + return value + }) + .catch((error) => { + cache.delete(key) + throw error + }) + + cache.set(key, promise) + return promise + }, + + /** Drops every memoized answer. Exists for tests. */ + clear(): void { + cache.clear() + }, + } +} + +/** + * Discovery is an idempotent GET, so replaying a 5xx cannot duplicate work — which + * is why this widening is scoped here rather than pushed into the shared predicate + * that also guards non-idempotent writes. + * + * The budget is deliberately tighter than the shared ~31s default: discovery is a + * fast hop a block waits on, so four attempts across ~3.5s is the useful range. A + * `Retry-After` still wins over the backoff, capped at `maxDelayMs`, so a + * server-directed wait can exceed that window. + * + * `>= 500` rather than `=== 500` matches `lib/knowledge/reranker.ts` and + * `lib/embeddings/client.ts`, and covers the 502/507/52x an edge can emit. + */ +export const ATLASSIAN_DISCOVERY_RETRY_OPTIONS: RetryOptions = { + maxRetries: 3, + initialDelayMs: 500, + maxDelayMs: 8000, + retryCondition: (error) => { + if (isRetryableError(error)) return true + const status = (error as { status?: unknown } | null)?.status + return typeof status === 'number' && status >= 500 + }, +} + +interface AccessibleResource { + id: string + url: string +} + +interface ResolveAtlassianCloudIdOptions { + domain: string + accessToken: string + /** Product name woven into the failure messages, e.g. `Jira` or `Confluence`. */ + product: string + retryOptions?: RetryOptions +} + +const cloudIdCache = createAtlassianDiscoveryCache() + +/** + * Reduces an Atlassian site domain to the canonical `https://host` form that + * `accessible-resources` reports in its `url` field. + */ +export function normalizeAtlassianSiteUrl(domain: string): string { + return `https://${domain + .trim() + .replace(/^https?:\/\//i, '') + .replace(/\/+$/, '')}`.toLowerCase() +} + +/** + * GETs an Atlassian discovery endpoint as JSON, replaying transient faults. + * + * Deliberately not built on `fetchWithRetry`: that helper decides whether a + * non-OK status enters the retry loop using a hardcoded `isRetryableError`, so a + * caller-supplied `retryCondition` can never widen the set — a 500 comes back as + * an un-thrown response and the transient-5xx budget above is silently inert. + * Raising the failure here also keeps `failureLabel` on every status, rather than + * the bare `HTTP ` the shared wrapper emits for the few it does throw on, + * which is the string operators and alerts match on. + * + * @param retryOptions - Merged over the discovery defaults, so a caller passing + * counts and delays only (as the connectors do) keeps the transient-5xx condition. + */ +export function fetchAtlassianDiscoveryJson( + url: string, + headers: Record, + failureLabel: string, + retryOptions?: RetryOptions +): Promise { + return retryWithExponentialBackoff( + async () => { + const response = await fetch(url, { + method: 'GET', + headers, + signal: AbortSignal.timeout(DISCOVERY_REQUEST_TIMEOUT_MS), + }) + + if (!response.ok) { + const errorText = await response.text() + const error: HTTPError = new Error( + `${failureLabel}: ${response.status} - ${errorText || response.statusText}` + ) + error.status = response.status + error.statusText = response.statusText + const retryAfterMs = parseRetryAfter(response.headers.get('Retry-After')) + if (retryAfterMs) error.retryAfterMs = retryAfterMs + throw error + } + + return (await response.json()) as T + }, + { ...ATLASSIAN_DISCOVERY_RETRY_OPTIONS, ...retryOptions } + ) +} + +function fetchAccessibleResources( + accessToken: string, + product: string, + retryOptions: RetryOptions | undefined +): Promise { + return fetchAtlassianDiscoveryJson( + ACCESSIBLE_RESOURCES_URL, + { Authorization: `Bearer ${accessToken}`, Accept: 'application/json' }, + `Failed to fetch ${product} accessible resources`, + retryOptions + ) +} + +async function discoverCloudId( + siteUrl: string, + { domain, accessToken, product, retryOptions }: ResolveAtlassianCloudIdOptions +): Promise<{ value: string; retain: boolean }> { + const resources = await fetchAccessibleResources(accessToken, product, retryOptions) + + if (!Array.isArray(resources) || resources.length === 0) { + throw new Error(`No ${product} resources found`) + } + + const match = resources.find((r) => normalizeAtlassianSiteUrl(r.url) === siteUrl) + if (match) { + return { value: match.id, retain: true } + } + + // A single-site fallback is a property of this token, not of the domain. + if (resources.length === 1) { + return { value: resources[0].id, retain: false } + } + + throw new Error( + `Could not match ${product} domain "${domain}" to any accessible resource. ` + + `Available sites: ${resources.map((r) => r.url).join(', ')}` + ) +} + +/** + * Resolves the Atlassian `cloudId` for a site domain, memoized across callers. + * + * Jira, Confluence, and JSM all read the same `accessible-resources` endpoint, so + * a run touching several Atlassian blocks shares one round trip and one retry + * budget. + */ +export async function resolveAtlassianCloudId( + options: ResolveAtlassianCloudIdOptions +): Promise { + const siteUrl = normalizeAtlassianSiteUrl(options.domain) + return cloudIdCache.resolve(siteUrl, () => discoverCloudId(siteUrl, options)) +} + +/** Drops every memoized `cloudId`. Exists for tests. */ +export function clearAtlassianCloudIdCache(): void { + cloudIdCache.clear() +} diff --git a/apps/sim/tools/confluence/utils.ts b/apps/sim/tools/confluence/utils.ts index 1045cb9d322..b18263cbcc4 100644 --- a/apps/sim/tools/confluence/utils.ts +++ b/apps/sim/tools/confluence/utils.ts @@ -1,57 +1,32 @@ +import { normalizeAtlassianSiteUrl, resolveAtlassianCloudId } from '@/lib/atlassian/discovery' import type { RetryOptions } from '@/lib/knowledge/documents/utils' -import { fetchWithRetry } from '@/lib/knowledge/documents/utils' + +const SITE_URL_SCHEME = 'https://' /** - * Strips protocol and trailing slashes from a Confluence domain to produce - * a bare host (e.g. `yoursite.atlassian.net`). + * Strips protocol and trailing slashes and lowercases a Confluence domain to + * produce a bare host (e.g. `yoursite.atlassian.net`). + * + * Derived from the canonical site-URL form rather than repeating its regexes, so + * the host a connector builds and the key the resolver caches under cannot drift. */ export function normalizeConfluenceDomainHost(domain: string): string { - return domain - .trim() - .replace(/^https?:\/\//i, '') - .replace(/\/+$/, '') + return normalizeAtlassianSiteUrl(domain).slice(SITE_URL_SCHEME.length) } -export async function getConfluenceCloudId( +/** + * Resolves the `cloudId` for a Confluence site. Memoized per domain by the + * shared Atlassian resolver, which Jira and JSM read through as well. + * + * The resolver raises non-OK statuses with the product name attached, so an + * upstream fault is reported as such rather than as a missing-site error. + */ +export function getConfluenceCloudId( domain: string, accessToken: string, retryOptions?: RetryOptions ): Promise { - const response = await fetchWithRetry( - 'https://api.atlassian.com/oauth/token/accessible-resources', - { - method: 'GET', - headers: { - Authorization: `Bearer ${accessToken}`, - Accept: 'application/json', - }, - }, - retryOptions - ) - - const resources = await response.json() - - if (!Array.isArray(resources) || resources.length === 0) { - throw new Error('No Confluence resources found') - } - - const normalized = `https://${normalizeConfluenceDomainHost(domain)}`.toLowerCase() - const match = resources.find( - (r: { url: string }) => r.url.toLowerCase().replace(/\/+$/, '') === normalized - ) - - if (match) { - return match.id - } - - if (resources.length === 1) { - return resources[0].id - } - - throw new Error( - `Could not match Confluence domain "${domain}" to any accessible resource. ` + - `Available sites: ${resources.map((r: { url: string }) => r.url).join(', ')}` - ) + return resolveAtlassianCloudId({ domain, accessToken, product: 'Confluence', retryOptions }) } function decodeHtmlEntities(text: string): string { diff --git a/apps/sim/tools/jira/bulk_read.ts b/apps/sim/tools/jira/bulk_read.ts index 91f76651020..cdeabcfa8a3 100644 --- a/apps/sim/tools/jira/bulk_read.ts +++ b/apps/sim/tools/jira/bulk_read.ts @@ -1,6 +1,6 @@ import type { JiraRetrieveBulkParams, JiraRetrieveResponseBulk } from '@/tools/jira/types' import { TIMESTAMP_OUTPUT } from '@/tools/jira/types' -import { extractAdfText, normalizeDomain } from '@/tools/jira/utils' +import { extractAdfText, getJiraCloudId } from '@/tools/jira/utils' import type { ToolConfig } from '@/tools/types' export const jiraBulkRetrieveTool: ToolConfig = { @@ -51,7 +51,7 @@ export const jiraBulkRetrieveTool: ToolConfig { + transformResponse: async (_response: Response, params?: JiraRetrieveBulkParams) => { const MAX_TOTAL = 1000 const PAGE_SIZE = 100 @@ -68,25 +68,8 @@ export const jiraBulkRetrieveTool: ToolConfig { - if (params?.cloudId) return params.cloudId - const accessibleResources = await response.json() - if (!Array.isArray(accessibleResources) || accessibleResources.length === 0) { - throw new Error('No Jira resources found') - } - const normalizedInput = normalizeDomain(params?.domain ?? '') - const matchedResource = accessibleResources.find( - (r: { url: string }) => r.url.toLowerCase().replace(/\/+$/, '') === normalizedInput - ) - if (matchedResource) return matchedResource.id - if (accessibleResources.length === 1) return accessibleResources[0].id - throw new Error( - `Could not match Jira domain "${params?.domain}" to any accessible resource. ` + - `Available sites: ${accessibleResources.map((r: { url: string }) => r.url).join(', ')}` - ) - } - - const cloudId = await resolveCloudId() + const cloudId = + params?.cloudId ?? (await getJiraCloudId(params?.domain ?? '', params!.accessToken)) const projectKey = await resolveProjectKey(cloudId, params!.accessToken, params!.projectId) if (!/^[A-Za-z][A-Za-z0-9_]*$/.test(projectKey)) { throw new Error( diff --git a/apps/sim/tools/jira/utils.ts b/apps/sim/tools/jira/utils.ts index 3ce0db24107..8aac4bb82da 100644 --- a/apps/sim/tools/jira/utils.ts +++ b/apps/sim/tools/jira/utils.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import { resolveAtlassianCloudId } from '@/lib/atlassian/discovery' import type { RetryOptions } from '@/lib/knowledge/documents/utils' import { fetchWithRetry } from '@/lib/knowledge/documents/utils' @@ -160,58 +161,16 @@ export function normalizeJiraWorklogTimestamp(value: string): string { return s } -export function normalizeDomain(domain: string): string { - return `https://${domain - .trim() - .replace(/^https?:\/\//i, '') - .replace(/\/+$/, '')}`.toLowerCase() -} - -export async function getJiraCloudId( +/** + * Resolves the `cloudId` for a Jira site. Memoized per domain by the shared + * Atlassian resolver, which Confluence and JSM read through as well. + */ +export function getJiraCloudId( domain: string, accessToken: string, retryOptions?: RetryOptions ): Promise { - const response = await fetchWithRetry( - 'https://api.atlassian.com/oauth/token/accessible-resources', - { - method: 'GET', - headers: { - Authorization: `Bearer ${accessToken}`, - Accept: 'application/json', - }, - }, - retryOptions - ) - - if (!response.ok) { - const errorText = await response.text() - throw new Error(`Failed to fetch Jira accessible resources: ${response.status} - ${errorText}`) - } - - const resources = await response.json() - - if (!Array.isArray(resources) || resources.length === 0) { - throw new Error('No Jira resources found') - } - - const normalized = normalizeDomain(domain) - const match = resources.find( - (r: { url: string }) => r.url.toLowerCase().replace(/\/+$/, '') === normalized - ) - - if (match) { - return match.id - } - - if (resources.length === 1) { - return resources[0].id - } - - throw new Error( - `Could not match Jira domain "${domain}" to any accessible resource. ` + - `Available sites: ${resources.map((r: { url: string }) => r.url).join(', ')}` - ) + return resolveAtlassianCloudId({ domain, accessToken, product: 'Jira', retryOptions }) } /** diff --git a/apps/sim/tools/jsm/utils.ts b/apps/sim/tools/jsm/utils.ts index df9a46e4af3..b4c36d804f6 100644 --- a/apps/sim/tools/jsm/utils.ts +++ b/apps/sim/tools/jsm/utils.ts @@ -2,9 +2,19 @@ * Shared utilities for Jira Service Management tools */ +import { + createAtlassianDiscoveryCache, + fetchAtlassianDiscoveryJson, +} from '@/lib/atlassian/discovery' import { getJiraCloudId } from '@/tools/jira/utils' import type { AssetObject, RawAssetObject } from '@/tools/jsm/types' +/** + * Atlassian provisions a single Assets workspace per site, so the answer is a + * property of the `cloudId` and safe to share across callers. + */ +const assetsWorkspaceCache = createAtlassianDiscoveryCache() + /** * Resolve the Jira `cloudId` and Assets `workspaceId` needed for an Assets API * call, using the request params when present and falling back to discovery. @@ -113,27 +123,22 @@ export function getAssetsApiBaseUrl(cloudId: string, workspaceId: string): strin * @returns The Assets workspace ID for the site * @throws If discovery fails or no workspace is provisioned */ -export async function getAssetsWorkspaceId(cloudId: string, accessToken: string): Promise { - const response = await fetch( - `https://api.atlassian.com/ex/jira/${cloudId}/rest/servicedeskapi/assets/workspace`, - { method: 'GET', headers: getJsmHeaders(accessToken) } - ) - - if (!response.ok) { - const errorText = await response.text() - throw new Error( - `Failed to resolve Assets workspace: ${response.status} - ${errorText || response.statusText}` +export function getAssetsWorkspaceId(cloudId: string, accessToken: string): Promise { + return assetsWorkspaceCache.resolve(cloudId, async () => { + const data = await fetchAtlassianDiscoveryJson<{ values?: Array<{ workspaceId?: string }> }>( + `https://api.atlassian.com/ex/jira/${cloudId}/rest/servicedeskapi/assets/workspace`, + getJsmHeaders(accessToken), + 'Failed to resolve Assets workspace' ) - } - const data = await response.json() - const workspaceId: string | undefined = data?.values?.[0]?.workspaceId + const workspaceId = data?.values?.[0]?.workspaceId - if (!workspaceId) { - throw new Error( - 'No Assets workspace found for this site. Assets (Insight) may not be enabled on the Jira instance.' - ) - } + if (!workspaceId) { + throw new Error( + 'No Assets workspace found for this site. Assets (Insight) may not be enabled on the Jira instance.' + ) + } - return workspaceId + return { value: workspaceId, retain: true } + }) } From 15e10080fca19973f68e18dab04ebd968b1c5a21 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 11 Aug 2026 09:19:01 -0700 Subject: [PATCH 2/3] fix(atlassian): key discovery answers by credential and retry timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1. Three fixes. The cache keyed on the normalized domain alone, so a caller joining a lookup already in flight inherited whichever credential started it — taking that token's authorization failure, or its single-site fallback pointing at a different site. Retaining only exact matches closed that for settled entries but not for the in-flight window, which is where it actually bites. Keys now carry a digest of the access token, so an answer is only ever reused by the credential that earned it. That also removes the reason the cache needed a `retain` channel. The request's own `AbortSignal.timeout` rejects with a `TimeoutError` that has no status and no message the shared predicate matches, so a slow site failed on the first attempt despite the retry budget. It is now explicitly retryable — only `TimeoutError`, since an `AbortError` means a caller cancelled — and the per- request timeout drops to 5s so four attempts stay bounded. Jira bulk read had been pointed at the cached resolver, but the tool's own configured request IS the discovery call and `transformResponse` only runs on a 2xx. It was therefore re-issuing a request whose answer it already held. It now matches against that payload through the shared selector, so the matching logic stays in one place without a second round trip. --- apps/sim/lib/atlassian/discovery.test.ts | 41 +++++++++-- apps/sim/lib/atlassian/discovery.ts | 88 +++++++++++++++--------- apps/sim/tools/jira/bulk_read.ts | 10 ++- apps/sim/tools/jsm/utils.ts | 8 ++- 4 files changed, 103 insertions(+), 44 deletions(-) diff --git a/apps/sim/lib/atlassian/discovery.test.ts b/apps/sim/lib/atlassian/discovery.test.ts index 1a7d49fec27..aadef8bcc4e 100644 --- a/apps/sim/lib/atlassian/discovery.test.ts +++ b/apps/sim/lib/atlassian/discovery.test.ts @@ -142,14 +142,45 @@ describe('resolveAtlassianCloudId', () => { ).rejects.toThrow(/Failed to fetch Confluence accessible resources: 500/) }) - it('does not retain a single-site fallback, since it is token-specific', async () => { - fetchMock.mockImplementation(async () => - sites([{ id: 'other-cloud', url: 'https://other.atlassian.net' }]) + it('does not serve one credential answer to another', async () => { + fetchMock + .mockResolvedValueOnce(sites([{ id: 'token-a-cloud', url: SITE }])) + .mockResolvedValueOnce(sites([{ id: 'token-b-cloud', url: SITE }])) + + await expect(resolveAtlassianCloudId(options({ accessToken: 'a' }))).resolves.toBe( + 'token-a-cloud' ) + await expect(resolveAtlassianCloudId(options({ accessToken: 'b' }))).resolves.toBe( + 'token-b-cloud' + ) + expect(fetchMock).toHaveBeenCalledTimes(2) + }) - await expect(resolveAtlassianCloudId(options())).resolves.toBe('other-cloud') - await resolveAtlassianCloudId(options()) + it('does not let a concurrent caller inherit another credential lookup', async () => { + // Token A sees only a different site, so it falls back; token B matches exactly. + // Joining A's in-flight promise would hand B the wrong site. + fetchMock + .mockResolvedValueOnce(sites([{ id: 'a-only-cloud', url: 'https://other.atlassian.net' }])) + .mockResolvedValueOnce(sites([{ id: CLOUD_ID, url: SITE }])) + + const [a, b] = await Promise.all([ + resolveAtlassianCloudId(options({ accessToken: 'a' })), + resolveAtlassianCloudId(options({ accessToken: 'b' })), + ]) + + expect(a).toBe('a-only-cloud') + expect(b).toBe(CLOUD_ID) + expect(fetchMock).toHaveBeenCalledTimes(2) + }) + + it('retries a request that timed out', async () => { + fetchMock + .mockRejectedValueOnce( + Object.assign(new Error('The operation timed out.'), { name: 'TimeoutError' }) + ) + .mockResolvedValueOnce(sites([{ id: CLOUD_ID, url: SITE }])) + await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).resolves.toBe(CLOUD_ID) expect(fetchMock).toHaveBeenCalledTimes(2) }) diff --git a/apps/sim/lib/atlassian/discovery.ts b/apps/sim/lib/atlassian/discovery.ts index ee5822564bc..5a27b134dc2 100644 --- a/apps/sim/lib/atlassian/discovery.ts +++ b/apps/sim/lib/atlassian/discovery.ts @@ -1,3 +1,4 @@ +import { sha256Hex } from '@sim/security/hash' import { parseRetryAfter } from '@sim/utils/retry' import { LRUCache } from 'lru-cache' import { @@ -9,7 +10,7 @@ import { const ACCESSIBLE_RESOURCES_URL = 'https://api.atlassian.com/oauth/token/accessible-resources' -const DISCOVERY_REQUEST_TIMEOUT_MS = 10_000 +const DISCOVERY_REQUEST_TIMEOUT_MS = 5_000 /** * A site's `cloudId` is a property of the site rather than of the caller — the @@ -41,25 +42,20 @@ export function createAtlassianDiscoveryCache() { * * The promise — not the resolved value — is stored, so callers arriving while * a lookup is in flight join it instead of starting their own, and a rejection - * is evicted rather than pinned for the TTL. `retain: false` drops the entry - * once it settles, so a token-specific result is not served to later callers. + * is evicted rather than pinned for the TTL. + * + * `key` must identify the credential as well as the resource — a joined caller + * receives the first caller's outcome, so a domain-only key would let one + * token's authorization failure or single-site fallback answer another's. */ - resolve( - key: string, - resolver: () => Promise<{ value: string; retain: boolean }> - ): Promise { + resolve(key: string, resolver: () => Promise): Promise { const cached = cache.get(key) if (cached) return cached - const promise = resolver() - .then(({ value, retain }) => { - if (!retain) cache.delete(key) - return value - }) - .catch((error) => { - cache.delete(key) - throw error - }) + const promise = resolver().catch((error) => { + cache.delete(key) + throw error + }) cache.set(key, promise) return promise @@ -91,6 +87,11 @@ export const ATLASSIAN_DISCOVERY_RETRY_OPTIONS: RetryOptions = { maxDelayMs: 8000, retryCondition: (error) => { if (isRetryableError(error)) return true + // The request's own `AbortSignal.timeout` rejects with a `TimeoutError` that + // carries no status and no message the shared predicate matches, so without + // this a slow site would fail on the first attempt. Only `TimeoutError` — an + // `AbortError` would mean a caller cancelled and does not want a replay. + if (error instanceof Error && error.name === 'TimeoutError') return true const status = (error as { status?: unknown } | null)?.status return typeof status === 'number' && status >= 500 }, @@ -181,34 +182,47 @@ function fetchAccessibleResources( ) } -async function discoverCloudId( - siteUrl: string, - { domain, accessToken, product, retryOptions }: ResolveAtlassianCloudIdOptions -): Promise<{ value: string; retain: boolean }> { - const resources = await fetchAccessibleResources(accessToken, product, retryOptions) +/** + * Cache key for a discovery answer, scoped to the credential that produced it. + * + * What `accessible-resources` reports depends on the token, so an answer is only + * reusable by the same token. The digest keeps the raw token out of the key. + */ +export function atlassianDiscoveryKey(resource: string, accessToken: string): string { + return `${resource}:${sha256Hex(accessToken).slice(0, 16)}` +} +/** + * Picks the `cloudId` for `domain` out of an `accessible-resources` payload. + * + * Separate from the fetch so a caller that already holds the payload can match + * against it instead of issuing the request a second time. + */ +export function selectAtlassianCloudId( + resources: unknown, + domain: string, + product: string +): string { if (!Array.isArray(resources) || resources.length === 0) { throw new Error(`No ${product} resources found`) } - const match = resources.find((r) => normalizeAtlassianSiteUrl(r.url) === siteUrl) - if (match) { - return { value: match.id, retain: true } - } + const siteUrl = normalizeAtlassianSiteUrl(domain) + const match = (resources as AccessibleResource[]).find( + (r) => normalizeAtlassianSiteUrl(r.url) === siteUrl + ) + if (match) return match.id - // A single-site fallback is a property of this token, not of the domain. - if (resources.length === 1) { - return { value: resources[0].id, retain: false } - } + if (resources.length === 1) return (resources as AccessibleResource[])[0].id throw new Error( `Could not match ${product} domain "${domain}" to any accessible resource. ` + - `Available sites: ${resources.map((r) => r.url).join(', ')}` + `Available sites: ${(resources as AccessibleResource[]).map((r) => r.url).join(', ')}` ) } /** - * Resolves the Atlassian `cloudId` for a site domain, memoized across callers. + * Resolves the Atlassian `cloudId` for a site domain, memoized per credential. * * Jira, Confluence, and JSM all read the same `accessible-resources` endpoint, so * a run touching several Atlassian blocks shares one round trip and one retry @@ -217,8 +231,16 @@ async function discoverCloudId( export async function resolveAtlassianCloudId( options: ResolveAtlassianCloudIdOptions ): Promise { - const siteUrl = normalizeAtlassianSiteUrl(options.domain) - return cloudIdCache.resolve(siteUrl, () => discoverCloudId(siteUrl, options)) + const { domain, accessToken, product, retryOptions } = options + const key = atlassianDiscoveryKey(normalizeAtlassianSiteUrl(domain), accessToken) + + return cloudIdCache.resolve(key, async () => + selectAtlassianCloudId( + await fetchAccessibleResources(accessToken, product, retryOptions), + domain, + product + ) + ) } /** Drops every memoized `cloudId`. Exists for tests. */ diff --git a/apps/sim/tools/jira/bulk_read.ts b/apps/sim/tools/jira/bulk_read.ts index cdeabcfa8a3..c611b7953c6 100644 --- a/apps/sim/tools/jira/bulk_read.ts +++ b/apps/sim/tools/jira/bulk_read.ts @@ -1,6 +1,7 @@ +import { selectAtlassianCloudId } from '@/lib/atlassian/discovery' import type { JiraRetrieveBulkParams, JiraRetrieveResponseBulk } from '@/tools/jira/types' import { TIMESTAMP_OUTPUT } from '@/tools/jira/types' -import { extractAdfText, getJiraCloudId } from '@/tools/jira/utils' +import { extractAdfText } from '@/tools/jira/utils' import type { ToolConfig } from '@/tools/types' export const jiraBulkRetrieveTool: ToolConfig = { @@ -51,7 +52,7 @@ export const jiraBulkRetrieveTool: ToolConfig { + transformResponse: async (response: Response, params?: JiraRetrieveBulkParams) => { const MAX_TOTAL = 1000 const PAGE_SIZE = 100 @@ -68,8 +69,11 @@ export const jiraBulkRetrieveTool: ToolConfig { - return assetsWorkspaceCache.resolve(cloudId, async () => { + return assetsWorkspaceCache.resolve(atlassianDiscoveryKey(cloudId, accessToken), async () => { const data = await fetchAtlassianDiscoveryJson<{ values?: Array<{ workspaceId?: string }> }>( `https://api.atlassian.com/ex/jira/${cloudId}/rest/servicedeskapi/assets/workspace`, getJsmHeaders(accessToken), @@ -139,6 +141,6 @@ export function getAssetsWorkspaceId(cloudId: string, accessToken: string): Prom ) } - return { value: workspaceId, retain: true } + return workspaceId }) } From 9173ccbecba05af21ec852d4f02c8607c0cb6313 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 11 Aug 2026 10:09:04 -0700 Subject: [PATCH 3/3] fix(jira): treat an empty bulk-read cloudId as missing The consolidation replaced a truthiness check with `??`, so an empty-string `cloudId` counted as supplied and bulk read skipped discovery entirely, building its request URL around an empty id. Back to `||`, matching every sibling tool. --- apps/sim/tools/jira/bulk_read.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/sim/tools/jira/bulk_read.ts b/apps/sim/tools/jira/bulk_read.ts index c611b7953c6..ac6a55add87 100644 --- a/apps/sim/tools/jira/bulk_read.ts +++ b/apps/sim/tools/jira/bulk_read.ts @@ -73,7 +73,7 @@ export const jiraBulkRetrieveTool: ToolConfig