Skip to content

Commit e0d2a87

Browse files
committed
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.
1 parent 63603b5 commit e0d2a87

7 files changed

Lines changed: 106 additions & 91 deletions

File tree

apps/sim/app/api/tools/confluence/pages/route.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
2323
const parsed = await parseRequest(confluencePagesSelectorContract, request, {})
2424
if (!parsed.success) return parsed.response
2525

26-
const { domain, accessToken, title, cloudId: providedCloudId, limit, cursor } = parsed.data.body
26+
const { domain, accessToken, title, cloudId: providedCloudId, limit } = parsed.data.body
2727

2828
const cloudId = providedCloudId || (await getConfluenceCloudId(domain, accessToken))
2929

@@ -43,10 +43,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4343
queryParams.append('title', title)
4444
}
4545

46-
if (cursor) {
47-
queryParams.append('cursor', cursor)
48-
}
49-
5046
const queryString = queryParams.toString()
5147
const url = queryString ? `${baseUrl}?${queryString}` : baseUrl
5248

@@ -86,24 +82,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8682
}
8783
}
8884

89-
/**
90-
* Confluence paginates with an opaque cursor carried in `_links.next`. Reading
91-
* it is what lets the selector continue past the first page — without it the
92-
* dropdown silently showed only `limit` pages of however many exist.
93-
*/
94-
let nextCursor: string | undefined
95-
const nextLink = data._links?.next as string | undefined
96-
if (nextLink) {
97-
try {
98-
nextCursor =
99-
new URL(nextLink, 'https://placeholder').searchParams.get('cursor') || undefined
100-
} catch {
101-
nextCursor = undefined
102-
}
103-
}
104-
10585
return NextResponse.json({
106-
files: (data.results || []).map((page: any) => ({
86+
files: data.results.map((page: any) => ({
10787
id: page.id,
10888
name: page.title,
10989
mimeType: 'confluence/page',
@@ -112,7 +92,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
11292
spaceId: page.spaceId,
11393
webViewLink: page._links?.webui || '',
11494
})),
115-
nextCursor,
11695
})
11796
} catch (error) {
11897
logger.error('Error fetching Confluence pages:', error)

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

Lines changed: 64 additions & 38 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
@@ -103,7 +124,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
103124

104125
const requestSpaces = async (
105126
search: URLSearchParams
106-
): Promise<{ ok: true; data: any } | { ok: false; response: NextResponse }> => {
127+
): Promise<{ ok: true; data: SpacesResponse } | { ok: false; response: NextResponse }> => {
107128
const response = await fetch(`${baseUrl}?${search.toString()}`, {
108129
method: 'GET',
109130
headers: { Accept: 'application/json', Authorization: `Bearer ${accessToken}` },
@@ -119,46 +140,54 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
119140
return { ok: true, data: await response.json() }
120141
}
121142

122-
let data: any
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+
123152
if (spaceKey) {
124153
/**
125154
* Exact-key lookup, bypassing the paged drain the dropdown otherwise depends
126-
* on. `status` is queried explicitly per value rather than omitted: it takes a
127-
* single value on this endpoint (unlike `/pages`, where it is an array with a
128-
* documented `current,archived` default), and no default is documented for
129-
* `/spaces`. Archived spaces are reachable in the paged path and sync works
130-
* against them, so resolving only `current` would silently miss them.
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.
131161
*/
132-
let result = await requestSpaces(
133-
new URLSearchParams({ keys: spaceKey, limit: String(PAGE_LIMIT), status: 'current' })
134-
)
135-
if (!result.ok) return result.response
136-
if (!result.data.results?.length) {
137-
result = await requestSpaces(
162+
const [current, archived] = await Promise.all([
163+
requestSpaces(
164+
new URLSearchParams({ keys: spaceKey, limit: String(PAGE_LIMIT), status: 'current' })
165+
),
166+
requestSpaces(
138167
new URLSearchParams({ keys: spaceKey, limit: String(PAGE_LIMIT), status: 'archived' })
139-
)
140-
if (!result.ok) return result.response
141-
}
142-
data = result.data
143-
} else {
144-
const params = new URLSearchParams({ limit: String(PAGE_LIMIT), status })
145-
if (inner) params.set('cursor', inner)
146-
const result = await requestSpaces(params)
147-
if (!result.ok) return result.response
148-
data = result.data
149-
}
150-
const spaces = (data.results || []).map(
151-
(space: { id: string; name: string; key: string; status?: SpaceStatus }) => ({
152-
id: space.id,
153-
name: space.name,
154-
key: space.key,
155-
// An exact-key lookup is not scoped to one status, so trust the row's own.
156-
status: space.status ?? status,
168+
),
169+
])
170+
if (!current.ok) return current.response
171+
if (!archived.ok) return archived.response
172+
173+
// A single resolution, never a page in a drained stream, so no cursor.
174+
return NextResponse.json({
175+
spaces: [
176+
...toSpaces(current.data.results, 'current'),
177+
...toSpaces(archived.data.results, 'archived'),
178+
],
179+
nextCursor: undefined,
157180
})
158-
)
181+
}
182+
183+
const params = new URLSearchParams({ limit: String(PAGE_LIMIT), status })
184+
if (inner) params.set('cursor', inner)
185+
const result = await requestSpaces(params)
186+
if (!result.ok) return result.response
187+
const data = result.data
159188

160189
let nextInner: string | undefined
161-
const nextLink = data._links?.next as string | undefined
190+
const nextLink = data._links?.next
162191
if (nextLink) {
163192
try {
164193
nextInner = new URL(nextLink, 'https://placeholder').searchParams.get('cursor') || undefined
@@ -167,17 +196,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
167196
}
168197
}
169198

170-
// An exact-key lookup is a single resolution, never a page in a drained stream.
171199
let nextCursor: string | undefined
172-
if (spaceKey) {
173-
nextCursor = undefined
174-
} else if (nextInner) {
200+
if (nextInner) {
175201
nextCursor = `${status}:${nextInner}`
176202
} else if (status === 'current') {
177203
nextCursor = 'archived:'
178204
}
179205

180-
return NextResponse.json({ spaces, nextCursor })
206+
return NextResponse.json({ spaces: toSpaces(data.results, status), nextCursor })
181207
} catch (error) {
182208
logger.error('Error listing Confluence spaces:', error)
183209
return NextResponse.json(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ function getEmptyMessage(
194194
): string {
195195
// `field.title` is singular on some connectors ("Base") and plural on others
196196
// ("Spaces"), so only the settled message puts the noun behind a quantifier.
197-
if (state.error) return 'No match — the list is incomplete. Enter the value directly'
197+
if (state.error) return 'No match — the list failed to load. Try reopening'
198198
if (state.hasMore || state.isFetchingMore) return 'No match yet — still loading…'
199-
if (state.truncated) return 'No match in what loaded — enter the value directly'
199+
if (state.truncated) return 'No match — too many to list. Try a more exact term'
200200
return `No ${noun} found`
201201
}
202202

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

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,16 @@ export const confluenceSelectors = {
8989
],
9090
enabled: ({ context }) => Boolean(context.oauthCredential && context.domain),
9191
/**
92-
* Paged rather than a single fetch: `/pages` is cursor-paginated, so one request
93-
* returned only the first `limit` pages of however many exist and the rest were
94-
* unreachable — a search for a real page silently found nothing. `search` is
95-
* still forwarded as the server-side `title` filter, and pagination now applies
96-
* to the filtered stream too.
92+
* Deliberately a single request, not a drain. `/pages` is cursor-paginated and
93+
* this list is therefore capped at `limit`, which is a real gap — but draining it
94+
* is worse: with no search term `title` is unset, so the drain would walk the
95+
* entire site (up to `MAX_AUTO_DRAIN_PAGES` requests) every time the dropdown
96+
* opens, and the route does not forward an abort signal upstream, so superseded
97+
* drains still bill the tenant's rate limit. Fixing this properly needs
98+
* server-side search whose `title` semantics have been confirmed against a live
99+
* instance, not brute-force loading.
97100
*/
98-
fetchPage: async ({ context, search, cursor, signal }) => {
101+
fetchList: async ({ context, search, signal }: SelectorQueryArgs) => {
99102
const credentialId = ensureCredential(context, 'confluence.pages')
100103
const domain = ensureDomain(context, 'confluence.pages')
101104
const bundle = await fetchOAuthToken(credentialId, context.workflowId)
@@ -108,17 +111,13 @@ export const confluenceSelectors = {
108111
accessToken: bundle.accessToken,
109112
cloudId: bundle.cloudId,
110113
title: search,
111-
cursor,
112114
},
113115
signal,
114116
})
115-
return {
116-
items: (data.files || []).map((file) => ({
117-
id: file.id,
118-
label: file.name,
119-
})),
120-
nextCursor: data.nextCursor,
121-
}
117+
return (data.files || []).map((file) => ({
118+
id: file.id,
119+
label: file.name,
120+
}))
122121
},
123122
fetchById: async ({ context, detailId, signal }: SelectorQueryArgs) => {
124123
if (!detailId) return null

apps/sim/hooks/selectors/use-selector-query.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -178,18 +178,23 @@ export function useSelectorOptionDetail(
178178
detailId: resolvedDetailId,
179179
}
180180
const hasRealDetailId = Boolean(resolvedDetailId)
181-
const baseEnabled =
182-
hasRealDetailId && definition.fetchById !== undefined
183-
? definition.enabled
184-
? definition.enabled(queryArgs)
185-
: true
186-
: false
187181
/**
188-
* A caller's `enabled` narrows `baseEnabled` rather than replacing it: `queryFn`
189-
* asserts `fetchById` is defined, so dropping that guard would throw for every
190-
* selector that declares no `fetchById`.
182+
* Hard precondition: `queryFn` asserts `fetchById` is defined, so this must hold
183+
* however the caller configures the query — otherwise the assertion throws for the
184+
* many selectors that declare no `fetchById`.
191185
*/
192-
const enabled = (args.enabled ?? true) && baseEnabled
186+
const canResolveDetail = hasRealDetailId && definition.fetchById !== undefined
187+
/**
188+
* `definition.enabled` describes when the *list* can be fetched, so it gates on
189+
* context a list needs (credential, domain, region). Resolving one already-known id
190+
* can need far less — `cloudwatch.*` echoes the id back without calling AWS at all —
191+
* so a caller that opts in explicitly is only narrowed by the hard precondition.
192+
* Callers that pass nothing keep the list predicate as their default.
193+
*/
194+
const enabled =
195+
args.enabled !== undefined
196+
? args.enabled && canResolveDetail
197+
: canResolveDetail && (definition.enabled ? definition.enabled(queryArgs) : true)
193198

194199
const query = useQuery<SelectorOption | null>({
195200
queryKey: [...definition.getQueryKey(queryArgs), 'detail', resolvedDetailId ?? 'none'],

apps/sim/lib/api/contracts/selectors/confluence.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ export const confluencePagesBodySchema = z.object({
2424
cloudId: optionalString,
2525
title: optionalString,
2626
limit: z.number().int().positive().optional().default(50),
27-
/** Opaque Confluence cursor for the next page, echoed back from `nextCursor`. */
28-
cursor: optionalString,
2927
})
3028

3129
/**
@@ -388,7 +386,7 @@ export const confluenceSpacesSelectorContract = definePostSelector(
388386
export const confluencePagesSelectorContract = definePostSelector(
389387
'/api/tools/confluence/pages',
390388
confluencePagesBodySchema,
391-
z.object({ files: z.array(fileOptionSchema), nextCursor: optionalString })
389+
z.object({ files: z.array(fileOptionSchema) })
392390
)
393391

394392
export const confluencePageSelectorContract = definePostSelector(

packages/emcn/src/components/combobox/combobox.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,16 @@ const Combobox = memo(
205205
*/
206206
const onSearchChangeRef = useRef(onSearchChange)
207207
onSearchChangeRef.current = onSearchChange
208-
/** Single write path for the search box so `onSearchChange` cannot be missed on a reset. */
208+
/**
209+
* Single write path for the search box so `onSearchChange` cannot be missed on a
210+
* reset. Deduped because several paths reset redundantly — Escape both handles the
211+
* key and lets the popover dismiss, and an editable select blurs after selecting —
212+
* which `setSearchQuery` absorbed silently but a consumer callback would not.
213+
*/
214+
const searchQueryRef = useRef('')
209215
const updateSearchQuery = useCallback((next: string) => {
216+
if (searchQueryRef.current === next) return
217+
searchQueryRef.current = next
210218
setSearchQuery(next)
211219
onSearchChangeRef.current?.(next)
212220
}, [])

0 commit comments

Comments
 (0)