fix(atlassian): share one cached, retrying cloudId resolver across Jira, Confluence, and JSM (#6541)

* fix(atlassian): share one cached, retrying cloudId resolver across Jira, Confluence, and JSM

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.

* fix(atlassian): key discovery answers by credential and retry timeouts

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.

* 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.

Author: waleedlatif1

apps/sim/lib/atlassian/discovery.test.ts

@@ -0,0 +1,212 @@
+/**
+ * @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<string, unknown> = {}) {
+  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<typeof vi.fn>
+
+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 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)
+  })
+
+  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)
+  })
+
+  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')
+  })
+})