Uh oh!
There was an error while loading. Please reload this page.
fix(atlassian): share one cached, retrying cloudId resolver across Jira, Confluence, and JSM - #6541
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Jira bulk read reuses the dispatcher’s successful discovery response via Adds 19 unit tests for normalization, caching, concurrency, retries, and credential isolation. Reviewed by Cursor Bugbot for commit 15e1008. Configure here. |
Greptile SummaryThe PR consolidates Atlassian resource discovery into a credential-scoped, promise-caching resolver with bounded retries and timeout handling.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/atlassian/discovery.ts | Introduces credential-scoped promise caching, canonical site selection, bounded discovery retries, and explicit timeout retry handling; the prior cache-isolation and timeout issues are addressed. |
| apps/sim/lib/atlassian/discovery.test.ts | Covers cache reuse, concurrent deduplication, credential isolation, rejection eviction, timeout and 5xx retries, bounded attempts, and site selection. |
| apps/sim/tools/jira/bulk_read.ts | Selects the cloud ID from the dispatcher’s existing successful accessible-resources payload, removing the previously reported redundant lookup. |
| apps/sim/tools/jsm/utils.ts | Moves Assets workspace discovery onto the shared retrying JSON helper and a credential-scoped promise cache. |
| apps/sim/tools/jira/utils.ts | Delegates Jira cloud-ID discovery to the shared resolver without changing the public helper contract. |
| apps/sim/tools/confluence/utils.ts | Delegates Confluence discovery and domain normalization to shared helpers while correctly surfacing non-OK discovery responses. |
Sequence Diagram
sequenceDiagram
participant Tool as Jira / Confluence / JSM Tool
participant Cache as Credential-scoped Discovery Cache
participant Resolver as Atlassian Discovery Resolver
participant API as Atlassian API
Tool->>Cache: Resolve resource + token digest
alt Cached or in flight
Cache-->>Tool: Shared promise result
else Cache miss
Cache->>Resolver: Start discovery
Resolver->>API: GET accessible resources
alt Timeout or transient 5xx
Resolver->>API: Retry within bounded budget
end
API-->>Resolver: Resource list
Resolver-->>Cache: Selected cloud ID
Cache-->>Tool: Result
end
Reviews (3): Last reviewed commit: "fix(jira): treat an empty bulk-read clou..." | Re-trigger Greptile
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…ra, 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.
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.
bcc3899 to
15e1008Comparewaleedlatif1
commented
Aug 11, 2026
waleedlatif1
commented
Aug 11, 2026
@cursor review |
Uh oh!
There was an error while loading. Please reload this page.
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.
waleedlatif1
commented
Aug 11, 2026
waleedlatif1
commented
Aug 11, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9173ccb. Configure here.
Summary
cloudIdfromaccessible-resourceson each invocation. Four hand-rolled copies of that lookup now read through one memoized resolver inlib/atlassian/discovery.tsRetry-After, and the request carries a timeout so a wedged fetch can't strand joined callersgetConfluenceCloudIdnever checked the response status, so a 500 surfaced asNo Confluence resources found(pointing at site permissions rather than the real fault);getAssetsWorkspaceIdused a bare fetch with no retry and no cacheType of Change
Testing
19 unit tests covering caching, in-flight dedup, reject-eviction, the bounded retry budget, 5xx replay, and the non-retained fallback. Verified each can fail by reverting the behavior it covers. Full
apps/simsuite green (22,760), plus type-check, lint, and the boundary/api-validation gates.Checklist