Skip to content

Commit 48d59ca

Browse files
authored
fix(connectors): make selector dropdowns resolve options the drain has not reached (#6027)
* fix(connectors): stop the connector selector implying an option does not exist The connector selector field ignored the drain state the shared selector hook already exposes. Paginated selectors fill in the background and the combobox filters client-side, so a not-yet-drained option is genuinely absent from the list — and the dropdown said "No spaces found", which reads as "this space does not exist" and sends users off to enter the value by hand. That is what happened with a Confluence space on a site with thousands of personal spaces. It now reports that the list is still filling, surfaces a failed drain instead of claiming to load forever, and is honest when the drain stops at its page cap. * feat(connectors): resolve selector options by exact key without waiting on the drain The connector selector fills by draining pages in the background and filters client-side, so an option is only findable once its page has arrived. On a large Confluence site that is ~38 seconds, during which searching for a real space returned nothing. Resolves the typed value and the selected value directly through the selector's `fetchById`, merging both into the option list, so an exact key is selectable immediately regardless of drain progress. `confluence.spaces.fetchById` now uses the documented v2 `keys` filter instead of scanning only the first page — which never resolved a space sorting beyond it. Adds `onSearchChange` to the shared Combobox so a consumer can observe the search box, held in a ref so its identity stays stable for the handlers that capture it without declaring it. Also fixes a pre-existing hazard in useSelectorOptionDetail: a caller-supplied `enabled` replaced the guard that checks a definition declares `fetchById`, so `queryFn`'s non-null assertion would throw for the ~12 selectors without one. * fix(connectors): paginate the Confluence pages selector instead of returning one page `/api/tools/confluence/pages` issued a single request and discarded the `_links.next` cursor Confluence returns, so the picker showed only the first `limit` pages of however many exist — a search for a real page found nothing, with no error and no truncation signal. Threads the documented opaque cursor and moves the selector to `fetchPage`, so the option list drains like `confluence.spaces` and reports real `hasMore` / `truncated` state. The `title` server-side filter is unchanged and now paginates too. `limit` is deliberately left at its existing default: the endpoint's maximum is not confirmable from Atlassian's published reference, and raising it is not needed for correctness. Also guards `data.results`, which threw when the response omitted it. * fix(connectors): query both space statuses explicitly on exact-key lookup The exact-key lookup omitted `status`, assuming that matched a space whether it was current or archived. Atlassian documents a `current,archived` default for `/pages` but documents no default for `/spaces`, where `status` takes a single value rather than an array — so the assumption was unverified, and resolving only current spaces would silently miss archived ones. Archived spaces are reachable through the paged path and sync works against them. Queries `current` first and falls back to `archived` only when it finds nothing, so the common case stays one request and the behaviour no longer depends on an undocumented default. * fix(connectors): revert pages drain, restore CloudWatch labels, tighten key lookup Six adversarial verification passes against the provider spec and the surrounding plumbing found three defects in the prior commits. Reverts the Confluence pages selector to a single request. Draining it was not strictly better: with no search term `title` is unset, so opening the dropdown walked the entire site — up to 50 sequential requests and 2,500 options where there had been one request and 50 — and the route forwards no abort signal upstream, so superseded drains still bill the tenant's rate limit. The same fan-out reached `loadAllSelectorOptions`, and the justification rested on `title` semantics Atlassian does not publish. The list cap is a real gap, but it needs confirmed server-side search, not brute force. Keeps the `results` guard. Restores selector display names for CloudWatch blocks. Narrowing a caller's `enabled` against `definition.enabled` disabled a detail query those selectors satisfy without AWS context, so collapsed blocks rendered "-" instead of the log group name. A caller that opts in is now narrowed only by the hard precondition that `fetchById` exists, which is what `queryFn` actually asserts. Queries both space statuses concurrently rather than sequentially: the key is user-typed text, so a miss dominates while typing and paid two round-trips. Each row now falls back to the status its own call requested. Also drops the "enter the value directly" empty states — the combobox is not editable, so there is no such affordance — and dedupes `onSearchChange`, which several reset paths fire redundantly. * fix(connectors): keep resolved option labels and survive a half-failed key lookup Addresses three review findings. A key lookup failed entirely when either status leg errored, discarding a match the other leg had found. Only a total failure is fatal now. Resolved option labels are remembered for the lifetime of the field. Both lookups key on values that change — the search box clears on select and close, and a multi-select field resolves no id at all — so the label for a just-picked option vanished a debounce later and the trigger fell back to a raw id. This covers the multi-select case, which is what the Confluence space field uses. A failed exact-value lookup now reads differently from a failed list load, rather than being reported as "still loading" or "none found". * fix(connectors): drop remembered option labels when the selector context changes Resolved ids are only meaningful within one selector context, so switching credential, domain, or a dependency has to discard what was remembered under the old one — the queries re-key, but the remembered labels would linger and mislabel until the field remounted. Keyed on the serialized context rather than its identity: the context memo also depends on `sourceConfig`, so its identity changes on unrelated field edits and would clear the cache far more often than intended. * refactor(connectors): resolve selected option labels with useQueries Replaces the remembered-label map with per-id queries over the selected values, following the pattern the knowledge-base selector already uses. The map only ever held ids searched for in the current session, so a multi-select field restored from saved config still rendered raw ids — the case that actually matters. It also needed two effects and a serialized-context key to avoid leaking labels across a context change, all of which disappear: queries key on the context, so a label cannot outlive it. Gates the speculative lookup of typed text on a new `resolvesUnknownIds` flag, set only where `fetchById` returns null for an id that does not exist. Most implementations resolve a record by id, so every partial keystroke was a failed upstream request, retried once, and its error made "could not check that exact value" the normal empty state on those selectors. Also: pass handlers straight to the combobox rather than through identity wrappers that defeated its memo, move the latest-callback ref write into an effect, rename the raw search setter so the deduping write path cannot be bypassed, and correct the prop and staleTime docs.
1 parent 60f2d03 commit 48d59ca

7 files changed

Lines changed: 335 additions & 56 deletions

File tree

apps/sim/app/api/tools/confluence/selector-spaces/route.ts

Lines changed: 84 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,27 @@ const PAGE_LIMIT = 250
2323

2424
type SpaceStatus = 'current' | 'archived'
2525

26+
/** A row as Confluence returns it. `status` is not marked required in the v2 schema. */
27+
interface SpaceRow {
28+
id: string
29+
name: string
30+
key: string
31+
status?: SpaceStatus
32+
}
33+
34+
interface SpacesResponse {
35+
results?: SpaceRow[]
36+
_links?: { next?: string }
37+
}
38+
39+
/** A row as this selector emits it, with `status` always resolved. */
40+
interface SelectorSpace {
41+
id: string
42+
name: string
43+
key: string
44+
status: SpaceStatus
45+
}
46+
2647
/**
2748
* Cursor format: `<status>:<innerCursor>`. Empty inner cursor means "first page
2849
* of that status". When current is exhausted we hand back `archived:` so the
@@ -45,7 +66,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4566
const parsed = await parseRequest(confluenceSpacesSelectorContract, request, {})
4667
if (!parsed.success) return parsed.response
4768

48-
const { credential, workflowId, domain, cursor } = parsed.data.body
69+
const { credential, workflowId, domain, cursor, spaceKey } = parsed.data.body
4970

5071
if (!credential) {
5172
logger.error('Missing credential in request')
@@ -101,32 +122,73 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
101122
const baseUrl = `https://api.atlassian.com/ex/confluence/${cloudIdValidation.sanitized}/wiki/api/v2/spaces`
102123
const { status, inner } = parseCursor(cursor)
103124

104-
const params = new URLSearchParams({ limit: String(PAGE_LIMIT), status })
105-
if (inner) params.set('cursor', inner)
106-
const url = `${baseUrl}?${params.toString()}`
125+
const requestSpaces = async (
126+
search: URLSearchParams
127+
): Promise<{ ok: true; data: SpacesResponse } | { ok: false; response: NextResponse }> => {
128+
const response = await fetch(`${baseUrl}?${search.toString()}`, {
129+
method: 'GET',
130+
headers: { Accept: 'application/json', Authorization: `Bearer ${accessToken}` },
131+
})
132+
133+
if (!response.ok) {
134+
const errorText = await response.text()
135+
const message = parseAtlassianErrorMessage(response.status, response.statusText, errorText)
136+
logger.error('Confluence API error response', { error: message, status: response.status })
137+
return { ok: false, response: NextResponse.json({ error: message }, { status: 502 }) }
138+
}
107139

108-
const response = await fetch(url, {
109-
method: 'GET',
110-
headers: { Accept: 'application/json', Authorization: `Bearer ${accessToken}` },
111-
})
140+
return { ok: true, data: await response.json() }
141+
}
112142

113-
if (!response.ok) {
114-
const errorText = await response.text()
115-
const message = parseAtlassianErrorMessage(response.status, response.statusText, errorText)
116-
logger.error('Confluence API error response', { error: message, status: response.status })
117-
return NextResponse.json({ error: message }, { status: 502 })
143+
const toSpaces = (rows: SpaceRow[] | undefined, queried: SpaceStatus): SelectorSpace[] =>
144+
(rows ?? []).map((space) => ({
145+
id: space.id,
146+
name: space.name,
147+
key: space.key,
148+
// Trust the row's own status; fall back to the status this call asked for.
149+
status: space.status ?? queried,
150+
}))
151+
152+
if (spaceKey) {
153+
/**
154+
* Exact-key lookup, bypassing the paged drain the dropdown otherwise depends
155+
* on. Both statuses are queried explicitly rather than omitting `status`: it
156+
* takes a single value on this endpoint (unlike `/pages`, where it is an array
157+
* with a documented `current,archived` default) and `/spaces` documents no
158+
* default, while archived spaces are reachable in the paged path and sync works
159+
* against them. Concurrent because the key is user-typed text, so a miss — which
160+
* dominates while typing — would otherwise pay two round-trips.
161+
*/
162+
const [current, archived] = await Promise.all([
163+
requestSpaces(
164+
new URLSearchParams({ keys: spaceKey, limit: String(PAGE_LIMIT), status: 'current' })
165+
),
166+
requestSpaces(
167+
new URLSearchParams({ keys: spaceKey, limit: String(PAGE_LIMIT), status: 'archived' })
168+
),
169+
])
170+
// Only a total failure is fatal: one leg erroring must not discard a match
171+
// the other leg found.
172+
if (!current.ok && !archived.ok) return current.response
173+
174+
// A single resolution, never a page in a drained stream, so no cursor.
175+
return NextResponse.json({
176+
spaces: [
177+
...(current.ok ? toSpaces(current.data.results, 'current') : []),
178+
...(archived.ok ? toSpaces(archived.data.results, 'archived') : []),
179+
],
180+
nextCursor: undefined,
181+
})
118182
}
119183

120-
const data = await response.json()
121-
const spaces = (data.results || []).map((space: { id: string; name: string; key: string }) => ({
122-
id: space.id,
123-
name: space.name,
124-
key: space.key,
125-
status,
126-
}))
184+
const params = new URLSearchParams({ limit: String(PAGE_LIMIT), status })
185+
if (inner) params.set('cursor', inner)
186+
const result = await requestSpaces(params)
187+
if (!result.ok) return result.response
188+
const data = result.data
127189

128190
let nextInner: string | undefined
129-
const nextLink = data._links?.next as string | undefined
191+
const nextLink = data._links?.next
130192
if (nextLink) {
131193
try {
132194
nextInner = new URL(nextLink, 'https://placeholder').searchParams.get('cursor') || undefined
@@ -142,7 +204,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
142204
nextCursor = 'archived:'
143205
}
144206

145-
return NextResponse.json({ spaces, nextCursor })
207+
return NextResponse.json({ spaces: toSpaces(data.results, status), nextCursor })
146208
} catch (error) {
147209
logger.error('Error listing Confluence spaces:', error)
148210
return NextResponse.json(

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx

Lines changed: 98 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,23 @@
11
'use client'
22

3-
import { useMemo } from 'react'
3+
import { useMemo, useState } from 'react'
44
import { ChipCombobox, type ComboboxOption, Loader } from '@sim/emcn'
5+
import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state'
56
import { SELECTOR_CONTEXT_FIELDS } from '@/lib/workflows/subblocks/context'
67
import type {
78
ConfigFieldMap,
89
ConfigFieldValue,
910
} from '@/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields'
1011
import { getDependsOnFields } from '@/blocks/utils'
1112
import type { ConnectorConfigField } from '@/connectors/types'
13+
import { getSelectorDefinition } from '@/hooks/selectors/registry'
1214
import type { SelectorContext, SelectorKey } from '@/hooks/selectors/types'
13-
import { useSelectorOptions } from '@/hooks/selectors/use-selector-query'
15+
import {
16+
useSelectorOptionDetail,
17+
useSelectorOptionDetails,
18+
useSelectorOptions,
19+
} from '@/hooks/selectors/use-selector-query'
20+
import { useDebounce } from '@/hooks/use-debounce'
1421

1522
interface ConnectorSelectorFieldProps {
1623
field: ConnectorConfigField & { selectorKey: SelectorKey }
@@ -34,6 +41,7 @@ export function ConnectorSelectorField({
3441
disabled,
3542
}: ConnectorSelectorFieldProps) {
3643
const isMulti = Boolean(field.multi)
44+
const [searchTerm, setSearchTerm] = useState('')
3745

3846
const context = useMemo<SelectorContext>(() => {
3947
const ctx: SelectorContext = {}
@@ -62,15 +70,68 @@ export function ConnectorSelectorField({
6270
}, [field.dependsOn, sourceConfig, configFields, canonicalModes])
6371

6472
const isEnabled = !disabled && !!credentialId && depsResolved
65-
const { data: options = [], isLoading } = useSelectorOptions(field.selectorKey, {
73+
const {
74+
data: options = [],
75+
isLoading,
76+
hasMore,
77+
isFetchingMore,
78+
truncated,
79+
error,
80+
} = useSelectorOptions(field.selectorKey, {
6681
context,
6782
enabled: isEnabled,
6883
})
6984

70-
const comboboxOptions = useMemo<ComboboxOption[]>(
71-
() => options.map((opt) => ({ label: opt.label, value: opt.id })),
72-
[options]
85+
/**
86+
* Label every selected value, including values restored from saved config that no
87+
* in-session search would have resolved. Queries are keyed on `context`, so a label
88+
* can never outlive the context that produced it, and they share keys with the
89+
* speculative lookup below so an already-resolved id costs no extra request.
90+
*/
91+
const singleValue = Array.isArray(value) ? value[0] : value
92+
const selectedIds = useMemo(
93+
() => (Array.isArray(value) ? value : value ? [value] : []).filter(Boolean),
94+
[value]
7395
)
96+
const selectedOptions = useSelectorOptionDetails(field.selectorKey, {
97+
context,
98+
detailIds: isEnabled ? selectedIds : [],
99+
})
100+
101+
/**
102+
* The option list fills by draining pages in the background and the combobox filters
103+
* it client-side, so an option is only findable once its page has arrived. Where the
104+
* selector's `fetchById` tolerates an unknown id, whatever the user typed is resolved
105+
* directly so an exact key is selectable immediately. Gated on that flag because most
106+
* implementations resolve a record by id, where a partial keystroke is a guaranteed
107+
* failed upstream request rather than an empty result.
108+
*/
109+
const resolvesUnknownIds = Boolean(getSelectorDefinition(field.selectorKey).resolvesUnknownIds)
110+
const debouncedSearch = useDebounce(searchTerm.trim(), SEARCH_DEBOUNCE_MS)
111+
const { data: searchedOption } = useSelectorOptionDetail(field.selectorKey, {
112+
context,
113+
detailId:
114+
resolvesUnknownIds && isEnabled && debouncedSearch.length > 0 ? debouncedSearch : undefined,
115+
})
116+
117+
const emptyMessage = getEmptyMessage(field.title.toLowerCase(), {
118+
error,
119+
hasMore,
120+
isFetchingMore,
121+
truncated,
122+
})
123+
124+
const comboboxOptions = useMemo<ComboboxOption[]>(() => {
125+
const base = options.map((opt) => ({ label: opt.label, value: opt.id }))
126+
const seen = new Set(base.map((opt) => opt.value))
127+
const extras: ComboboxOption[] = []
128+
for (const option of searchedOption ? [...selectedOptions, searchedOption] : selectedOptions) {
129+
if (seen.has(option.id)) continue
130+
seen.add(option.id)
131+
extras.push({ label: option.label, value: option.id })
132+
}
133+
return extras.length > 0 ? [...extras, ...base] : base
134+
}, [options, selectedOptions, searchedOption])
74135

75136
if (isLoading && isEnabled) {
76137
return (
@@ -88,8 +149,9 @@ export function ConnectorSelectorField({
88149
multiSelect
89150
options={comboboxOptions}
90151
multiSelectValues={multiValues}
91-
onMultiSelectChange={(values) => onChange(values)}
152+
onMultiSelectChange={onChange}
92153
searchable
154+
onSearchChange={setSearchTerm}
93155
searchPlaceholder={`Search ${field.title.toLowerCase()}...`}
94156
placeholder={
95157
!credentialId
@@ -99,18 +161,18 @@ export function ConnectorSelectorField({
99161
: field.placeholder || `Select ${field.title.toLowerCase()}`
100162
}
101163
disabled={disabled || !credentialId || !depsResolved}
102-
emptyMessage={`No ${field.title.toLowerCase()} found`}
164+
emptyMessage={emptyMessage}
103165
/>
104166
)
105167
}
106168

107-
const singleValue = Array.isArray(value) ? value[0] : value
108169
return (
109170
<ChipCombobox
110171
options={comboboxOptions}
111172
value={singleValue || undefined}
112-
onChange={(next) => onChange(next)}
173+
onChange={onChange}
113174
searchable
175+
onSearchChange={setSearchTerm}
114176
searchPlaceholder={`Search ${field.title.toLowerCase()}...`}
115177
placeholder={
116178
!credentialId
@@ -120,11 +182,36 @@ export function ConnectorSelectorField({
120182
: field.placeholder || `Select ${field.title.toLowerCase()}`
121183
}
122184
disabled={disabled || !credentialId || !depsResolved}
123-
emptyMessage={`No ${field.title.toLowerCase()} found`}
185+
emptyMessage={emptyMessage}
124186
/>
125187
)
126188
}
127189

190+
/**
191+
* Only visible once the first page has landed (`isLoading` renders a spinner
192+
* before that), so "no match" here means no match among the options drained
193+
* *so far* — a flat "none found" would wrongly read as "does not exist".
194+
*
195+
* `error` is checked before `hasMore`: a failed page halts the drain but leaves
196+
* `hasMore` set, which would otherwise claim to be loading forever.
197+
*/
198+
function getEmptyMessage(
199+
noun: string,
200+
state: {
201+
error: Error | null
202+
hasMore: boolean
203+
isFetchingMore: boolean
204+
truncated: boolean
205+
}
206+
): string {
207+
if (state.error) return 'No match — the list failed to load. Try reopening'
208+
if (state.hasMore || state.isFetchingMore) return 'No match yet — still loading…'
209+
if (state.truncated) return 'No match — too many to list. Try a more exact term'
210+
// `noun` is singular on some connectors ("Base") and plural on others ("Spaces"),
211+
// so only this settled message puts it behind a quantifier.
212+
return `No ${noun} found`
213+
}
214+
128215
function resolveDepValue(
129216
depFieldId: string,
130217
configFields: ConnectorConfigField[],

apps/sim/hooks/selectors/providers/confluence/selectors.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,12 @@ export const confluenceSelectors = {
4949
nextCursor: data.nextCursor,
5050
}
5151
},
52+
/** The server filters by key and returns nothing for a key that does not exist. */
53+
resolvesUnknownIds: true,
5254
/**
53-
* Resolves a single space label. Hits only the first page — the dropdown's
54-
* `fetchPage` stream populates the options cache for spaces beyond page 1,
55-
* and `useSelectorOptionMap` merges them in. Walking all pages here would
56-
* double API load since the stream is already running in parallel.
55+
* Resolves a single space by key via the server's exact-key lookup, independent
56+
* of how far the page drain has run — a space sorting beyond page 1 would
57+
* otherwise never resolve, which on a large site is most of them.
5758
*/
5859
fetchById: async ({ context, detailId, signal }: SelectorQueryArgs) => {
5960
if (!detailId) return null
@@ -64,6 +65,7 @@ export const confluenceSelectors = {
6465
credential: credentialId,
6566
workflowId: context.workflowId,
6667
domain,
68+
spaceKey: detailId,
6769
},
6870
signal,
6971
})
@@ -87,6 +89,16 @@ export const confluenceSelectors = {
8789
search ?? '',
8890
],
8991
enabled: ({ context }) => Boolean(context.oauthCredential && context.domain),
92+
/**
93+
* Deliberately a single request, not a drain. `/pages` is cursor-paginated and
94+
* this list is therefore capped at `limit`, which is a real gap — but draining it
95+
* is worse: with no search term `title` is unset, so the drain would walk the
96+
* entire site (up to `MAX_AUTO_DRAIN_PAGES` requests) every time the dropdown
97+
* opens, and the route does not forward an abort signal upstream, so superseded
98+
* drains still bill the tenant's rate limit. Fixing this properly needs
99+
* server-side search whose `title` semantics have been confirmed against a live
100+
* instance, not brute-force loading.
101+
*/
90102
fetchList: async ({ context, search, signal }: SelectorQueryArgs) => {
91103
const credentialId = ensureCredential(context, 'confluence.pages')
92104
const domain = ensureDomain(context, 'confluence.pages')

apps/sim/hooks/selectors/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,13 @@ export interface SelectorDefinition {
136136
*/
137137
fetchPage?: (args: SelectorPageArgs) => Promise<SelectorPage>
138138
fetchById?: (args: SelectorQueryArgs) => Promise<SelectorOption | null>
139+
/**
140+
* Set when `fetchById` tolerates an id that may not exist, returning `null` rather
141+
* than erroring. Only then is it safe to speculatively resolve whatever a user has
142+
* typed — most implementations resolve a record by id and would turn every partial
143+
* keystroke into a failed upstream request.
144+
*/
145+
resolvesUnknownIds?: boolean
139146
enabled?: (args: SelectorQueryArgs) => boolean
140147
staleTime?: number
141148
}

0 commit comments

Comments
 (0)