From b7f5efff0f9bc14ca7fe498ca0bbe49dad535f55 Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Tue, 1 Sep 2026 11:27:23 -0700 Subject: [PATCH 1/2] fix(selectors): close residual cancellation gaps --- .../application/execute-selector.test.ts | 28 ++++++ .../selectors/application/execute-selector.ts | 22 +++-- .../lib/selectors/server/credentials.test.ts | 94 ++++++++++++++++++- apps/sim/lib/selectors/server/credentials.ts | 53 +++++++++-- .../sim/lib/selectors/server/internal.test.ts | 38 ++++++++ apps/sim/lib/selectors/server/internal.ts | 4 +- .../providers/credential-bundle.test.ts | 34 +++++++ .../server/providers/credential-bundle.ts | 22 +++-- apps/sim/lib/selectors/server/types.ts | 2 + 9 files changed, 269 insertions(+), 28 deletions(-) diff --git a/apps/sim/lib/selectors/application/execute-selector.test.ts b/apps/sim/lib/selectors/application/execute-selector.test.ts index 992e8732c2e..bd5e9711c15 100644 --- a/apps/sim/lib/selectors/application/execute-selector.test.ts +++ b/apps/sim/lib/selectors/application/execute-selector.test.ts @@ -390,6 +390,34 @@ describe('executeSelector', () => { ]) }) + it('binds the request signal to credentials and does not present a late provider result', async () => { + const controller = new AbortController() + let markProviderStarted!: () => void + let finishProvider!: (result: { kind: 'list'; items: never[] }) => void + const providerStarted = new Promise((resolve) => { + markProviderStarted = resolve + }) + mocks.executeAttachment.mockImplementationOnce( + (args: ExecuteServerSelectorArgs) => + new Promise((resolve) => { + expect(args.credential?.signal).toBe(controller.signal) + markProviderStarted() + finishProvider = resolve + }) + ) + + const pending = execute({ signal: controller.signal }) + await providerStarted + + const abortReason = new DOMException('Selector request canceled', 'AbortError') + controller.abort(abortReason) + finishProvider({ kind: 'list', items: [] }) + + await expect(pending).rejects.toBe(abortReason) + expect(mocks.sanitize).not.toHaveBeenCalled() + expect(mocks.logger.info).not.toHaveBeenCalledWith('Executed selector', expect.anything()) + }) + it('records legacy service-account use once with its trusted provider id', async () => { mocks.authorizeCredential.mockResolvedValueOnce({ suppliedId: 'credential-1', diff --git a/apps/sim/lib/selectors/application/execute-selector.ts b/apps/sim/lib/selectors/application/execute-selector.ts index 75a109feb0a..11c6fdaef3d 100644 --- a/apps/sim/lib/selectors/application/execute-selector.ts +++ b/apps/sim/lib/selectors/application/execute-selector.ts @@ -151,15 +151,18 @@ async function executeAuthorizedSelector(args: { } const credential = attachment.credential - ? await authorizeSelectorCredential({ - principal: args.principal, - context: resolvedContext, - scope: args.input.scope, - workspaceId: args.context.workspaceId, - policy: attachment.credential, - protectedValues, - references: resolved.references, - }) + ? { + ...(await authorizeSelectorCredential({ + principal: args.principal, + context: resolvedContext, + scope: args.input.scope, + workspaceId: args.context.workspaceId, + policy: attachment.credential, + protectedValues, + references: resolved.references, + })), + signal: args.input.signal, + } : undefined /** @@ -220,6 +223,7 @@ async function executeAuthorizedSelector(args: { ? undefined : await attachment.destination.prepare(selectorArgs) const providerResult = await attachment.execute(selectorArgs, preparedDestination) + args.input.signal?.throwIfAborted() if (providerResult.diagnostics?.truncated) { logger.warn('Selector provider result reached a configured cap', { selectorKey: args.input.selectorKey, diff --git a/apps/sim/lib/selectors/server/credentials.test.ts b/apps/sim/lib/selectors/server/credentials.test.ts index 5d205fd7fc8..605232df8f5 100644 --- a/apps/sim/lib/selectors/server/credentials.test.ts +++ b/apps/sim/lib/selectors/server/credentials.test.ts @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ authorizeCredentialUse: vi.fn(), credentialProviderMatchesService: vi.fn(), getServiceConfig: vi.fn(), + resolveCredentialTokenBundle: vi.fn(), })) vi.mock('@/lib/auth/credential-access', () => ({ @@ -17,7 +18,7 @@ vi.mock('@/lib/auth/credential-access', () => ({ })) vi.mock('@/lib/oauth/credential-service', () => ({ - resolveCredentialTokenBundle: vi.fn(), + resolveCredentialTokenBundle: mocks.resolveCredentialTokenBundle, })) vi.mock('@/lib/oauth/utils', () => ({ @@ -25,7 +26,10 @@ vi.mock('@/lib/oauth/utils', () => ({ getServiceConfigByServiceId: mocks.getServiceConfig, })) -import { authorizeSelectorCredential } from '@/lib/selectors/server/credentials' +import { + authorizeSelectorCredential, + resolveSelectorOAuthAccessToken, +} from '@/lib/selectors/server/credentials' import { SelectorConnectionUnavailableError } from '@/lib/selectors/server/errors' import { createSelectorProtectedValues } from '@/lib/selectors/server/protected-values' @@ -137,3 +141,89 @@ describe('authorizeSelectorCredential', () => { }) }) }) + +describe('resolveSelectorOAuthAccessToken', () => { + beforeEach(() => vi.clearAllMocks()) + + it('rejects only the canceled waiter while shared credential work serves another caller', async () => { + let resolveShared!: (value: { accessToken: string }) => void + const sharedResolution = new Promise<{ accessToken: string }>((resolve) => { + resolveShared = resolve + }) + mocks.resolveCredentialTokenBundle.mockReturnValue(sharedResolution) + const canceledController = new AbortController() + const liveController = new AbortController() + const canceledProtectedValues = createSelectorProtectedValues() + const liveProtectedValues = createSelectorProtectedValues() + const canceledRecordUse = vi.fn() + const liveRecordUse = vi.fn() + const access = { + ok: true as const, + credentialOwnerUserId: 'owner-1', + resolvedCredentialId: 'credential-1', + } + + const canceledWaiter = resolveSelectorOAuthAccessToken({ + credential: { + suppliedId: 'credential-1', + access, + signal: canceledController.signal, + }, + serviceId: 'gmail', + protectedValues: canceledProtectedValues, + recordCredentialUse: canceledRecordUse, + }) + const liveWaiter = resolveSelectorOAuthAccessToken({ + credential: { + suppliedId: 'credential-1', + access, + signal: liveController.signal, + }, + serviceId: 'gmail', + protectedValues: liveProtectedValues, + recordCredentialUse: liveRecordUse, + }) + + const abortReason = new DOMException('Selector request canceled', 'AbortError') + canceledController.abort(abortReason) + await expect(canceledWaiter).rejects.toBe(abortReason) + + resolveShared({ accessToken: 'shared-access-token' }) + await expect(liveWaiter).resolves.toBe('shared-access-token') + + expect(canceledRecordUse).not.toHaveBeenCalled() + expect(canceledProtectedValues.contains('shared-access-token')).toBe(false) + expect(liveRecordUse).toHaveBeenCalledOnce() + expect(liveProtectedValues.contains('shared-access-token')).toBe(true) + expect(mocks.resolveCredentialTokenBundle).toHaveBeenCalledTimes(2) + for (const call of mocks.resolveCredentialTokenBundle.mock.calls) { + expect(call[5]).toEqual({ privacyMode: 'selector' }) + expect(call).not.toContain(canceledController.signal) + expect(call).not.toContain(liveController.signal) + } + }) + + it('does not start credential resolution for an already canceled selector', async () => { + const controller = new AbortController() + const abortReason = new DOMException('Selector request canceled', 'AbortError') + controller.abort(abortReason) + + await expect( + resolveSelectorOAuthAccessToken({ + credential: { + suppliedId: 'credential-1', + access: { + ok: true, + credentialOwnerUserId: 'owner-1', + resolvedCredentialId: 'credential-1', + }, + signal: controller.signal, + }, + serviceId: 'gmail', + protectedValues: createSelectorProtectedValues(), + }) + ).rejects.toBe(abortReason) + + expect(mocks.resolveCredentialTokenBundle).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/selectors/server/credentials.ts b/apps/sim/lib/selectors/server/credentials.ts index 9229adf8c55..79e7c975642 100644 --- a/apps/sim/lib/selectors/server/credentials.ts +++ b/apps/sim/lib/selectors/server/credentials.ts @@ -18,6 +18,41 @@ import type { } from '@/lib/selectors/server/types' import type { SelectorContext, SelectorScope } from '@/lib/selectors/types' +function selectorAbortReason(signal: AbortSignal): unknown { + return signal.reason ?? new DOMException('The operation was aborted.', 'AbortError') +} + +/** + * Makes one selector's wait abortable without attaching its signal to shared + * refresh or mint work that may still be serving other callers. + */ +export function waitForSelectorCredentialResolution( + resolution: Promise, + signal?: AbortSignal +): Promise { + if (!signal) return resolution + if (signal.aborted) return Promise.reject(selectorAbortReason(signal)) + + return new Promise((resolve, reject) => { + const onAbort = () => { + signal.removeEventListener('abort', onAbort) + reject(selectorAbortReason(signal)) + } + signal.addEventListener('abort', onAbort, { once: true }) + resolution.then( + (value) => { + signal.removeEventListener('abort', onAbort) + resolve(value) + }, + (error) => { + signal.removeEventListener('abort', onAbort) + reject(error) + } + ) + if (signal.aborted) onAbort() + }) +} + async function resolveCredentialProviderId(input: { credentialId: string credentialOwnerUserId: string @@ -116,6 +151,7 @@ export async function resolveSelectorOAuthAccessToken(input: { protectedValues: SelectorProtectedValues recordCredentialUse?: (providerId: string) => void }): Promise { + input.credential.signal?.throwIfAborted() if (input.credential.fixedToken) return input.credential.fixedToken const access = input.credential.access @@ -123,13 +159,16 @@ export async function resolveSelectorOAuthAccessToken(input: { throw new SelectorConnectionUnavailableError() } - const result = await resolveCredentialTokenBundle( - input.credential.suppliedId, - access.credentialOwnerUserId, - 'selector-execution', - input.scopes ? [...input.scopes] : undefined, - input.impersonateEmail, - { privacyMode: 'selector' } + const result = await waitForSelectorCredentialResolution( + resolveCredentialTokenBundle( + input.credential.suppliedId, + access.credentialOwnerUserId, + 'selector-execution', + input.scopes ? [...input.scopes] : undefined, + input.impersonateEmail, + { privacyMode: 'selector' } + ), + input.credential.signal ) const token = result?.accessToken diff --git a/apps/sim/lib/selectors/server/internal.test.ts b/apps/sim/lib/selectors/server/internal.test.ts index 0d02fd00eb0..f085c147afe 100644 --- a/apps/sim/lib/selectors/server/internal.test.ts +++ b/apps/sim/lib/selectors/server/internal.test.ts @@ -5,11 +5,16 @@ import { environmentUtilsMockFns, resetEnvironmentUtilsMock } from '@sim/testing import { beforeEach, describe, expect, it, vi } from 'vitest' const mockListWorkflows = vi.hoisted(() => vi.fn()) +const mockFetchOpenRouterEmbeddingModelCatalog = vi.hoisted(() => vi.fn()) vi.mock('@/lib/workflows/application/list-workflows', () => ({ listWorkflows: { execute: mockListWorkflows }, })) +vi.mock('@/lib/embeddings/openrouter-model-catalog.server', () => ({ + fetchOpenRouterEmbeddingModelCatalog: mockFetchOpenRouterEmbeddingModelCatalog, +})) + import { SelectorOptionsUnavailableError } from '@/lib/selectors/server/errors' import { internalSelectorAttachments } from '@/lib/selectors/server/internal' import { createSelectorProtectedValues } from '@/lib/selectors/server/protected-values' @@ -112,3 +117,36 @@ describe('sim.workflows selector', () => { expect(mockListWorkflows).toHaveBeenCalledTimes(40) }) }) + +describe('providers.openrouterEmbeddingModels selector', () => { + beforeEach(() => { + vi.clearAllMocks() + resetEnvironmentUtilsMock() + }) + + it('passes the selector signal to the OpenRouter catalog fetch', async () => { + const controller = new AbortController() + mockFetchOpenRouterEmbeddingModelCatalog.mockResolvedValue([ + { id: 'openai/text-embedding-3-small', maxInputTokens: 8_191 }, + ]) + + await expect( + internalSelectorAttachments['providers.openrouterEmbeddingModels'].execute({ + ...workflowArgs(), + selectorKey: 'providers.openrouterEmbeddingModels', + signal: controller.signal, + }) + ).resolves.toEqual({ + kind: 'list', + items: [ + { + id: 'openai/text-embedding-3-small', + label: 'openai/text-embedding-3-small', + }, + ], + }) + + expect(mockFetchOpenRouterEmbeddingModelCatalog).toHaveBeenCalledOnce() + expect(mockFetchOpenRouterEmbeddingModelCatalog).toHaveBeenCalledWith(controller.signal) + }) +}) diff --git a/apps/sim/lib/selectors/server/internal.ts b/apps/sim/lib/selectors/server/internal.ts index 36563f80ccc..4a5241cdf27 100644 --- a/apps/sim/lib/selectors/server/internal.ts +++ b/apps/sim/lib/selectors/server/internal.ts @@ -298,10 +298,10 @@ export const internalSelectorAttachments = { }, 'providers.openrouterEmbeddingModels': { destination: 'fixed', - async execute() { + async execute(args: ExecuteServerSelectorArgs) { if (isProviderBlacklisted('openrouter')) return listSelectorResult([]) const models = filterBlacklistedModels( - (await fetchOpenRouterEmbeddingModelCatalog()).map((model) => model.id) + (await fetchOpenRouterEmbeddingModelCatalog(args.signal)).map((model) => model.id) ) return listSelectorResult([...new Set(models)].map((model) => ({ id: model, label: model }))) }, diff --git a/apps/sim/lib/selectors/server/providers/credential-bundle.test.ts b/apps/sim/lib/selectors/server/providers/credential-bundle.test.ts index 34ca50b57ff..6dc5c5740bb 100644 --- a/apps/sim/lib/selectors/server/providers/credential-bundle.test.ts +++ b/apps/sim/lib/selectors/server/providers/credential-bundle.test.ts @@ -36,4 +36,38 @@ describe('selector credential bundles', () => { expect(protectedValues.contains('cloud-1')).toBe(true) expect(protectedValues.contains('prefix-cloud-1-suffix')).toBe(false) }) + + it('preserves a selector abort without canceling the shared resolution', async () => { + let resolveShared!: (value: { accessToken: string }) => void + const sharedResolution = new Promise<{ accessToken: string }>((resolve) => { + resolveShared = resolve + }) + mockResolveCredentialAccessToken.mockReturnValue(sharedResolution) + const controller = new AbortController() + const protectedValues = createSelectorProtectedValues() + const pending = resolveSelectorCredentialBundle({ + credential: { + suppliedId: 'credential-1', + access: { ok: true, credentialOwnerUserId: 'owner-1' }, + signal: controller.signal, + }, + protectedValues, + }) + const abortReason = new DOMException('Selector request canceled', 'AbortError') + + controller.abort(abortReason) + await expect(pending).rejects.toBe(abortReason) + + resolveShared({ accessToken: 'shared-access-token' }) + await Promise.resolve() + expect(protectedValues.contains('shared-access-token')).toBe(false) + expect(mockResolveCredentialAccessToken).toHaveBeenCalledWith( + 'credential-1', + 'owner-1', + 'selector-execution', + undefined, + undefined, + { privacyMode: 'selector' } + ) + }) }) diff --git a/apps/sim/lib/selectors/server/providers/credential-bundle.ts b/apps/sim/lib/selectors/server/providers/credential-bundle.ts index 7976f6c20ae..027c0320d52 100644 --- a/apps/sim/lib/selectors/server/providers/credential-bundle.ts +++ b/apps/sim/lib/selectors/server/providers/credential-bundle.ts @@ -2,6 +2,7 @@ import { resolveCredentialTokenBundle, type ServiceAccountTokenResult, } from '@/lib/oauth/credential-service' +import { waitForSelectorCredentialResolution } from '@/lib/selectors/server/credentials' import { SelectorConnectionUnavailableError } from '@/lib/selectors/server/errors' import type { AuthorizedSelectorCredential, @@ -23,6 +24,7 @@ export async function resolveSelectorCredentialBundle(input: { const credential = input.credential if (!credential) throw new SelectorConnectionUnavailableError() + credential.signal?.throwIfAborted() if (credential.fixedToken) { if (input.providerId) { input.recordCredentialUse?.(credential.providerId ?? input.providerId) @@ -35,15 +37,19 @@ export async function resolveSelectorCredentialBundle(input: { let bundle: ServiceAccountTokenResult | null try { - bundle = await resolveCredentialTokenBundle( - credential.suppliedId, - ownerUserId, - 'selector-execution', - input.scopes ? [...input.scopes] : undefined, - input.impersonateEmail, - { privacyMode: 'selector' } + bundle = await waitForSelectorCredentialResolution( + resolveCredentialTokenBundle( + credential.suppliedId, + ownerUserId, + 'selector-execution', + input.scopes ? [...input.scopes] : undefined, + input.impersonateEmail, + { privacyMode: 'selector' } + ), + credential.signal ) - } catch { + } catch (error) { + if (credential.signal?.aborted) throw error throw new SelectorConnectionUnavailableError() } if (!bundle?.accessToken) throw new SelectorConnectionUnavailableError() diff --git a/apps/sim/lib/selectors/server/types.ts b/apps/sim/lib/selectors/server/types.ts index d5b2bdcfec1..4249782a7e9 100644 --- a/apps/sim/lib/selectors/server/types.ts +++ b/apps/sim/lib/selectors/server/types.ts @@ -49,6 +49,8 @@ export interface AuthorizedSelectorCredential { fixedToken?: string /** Trusted provider id loaded during server-side credential binding. */ providerId?: string + /** Cancels only this selector's wait for shared credential resolution. */ + signal?: AbortSignal } export interface SelectorProtectedValues { From 5ee3a4a1c0ad1daae324399a3f3c819de04b6436 Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Tue, 1 Sep 2026 11:43:30 -0700 Subject: [PATCH 2/2] fix(selectors): recheck post-resolution cancellation --- .../lib/selectors/server/credentials.test.ts | 30 +++++++++++++++++++ apps/sim/lib/selectors/server/credentials.ts | 1 + .../providers/credential-bundle.test.ts | 28 +++++++++++++++++ .../server/providers/credential-bundle.ts | 1 + 4 files changed, 60 insertions(+) diff --git a/apps/sim/lib/selectors/server/credentials.test.ts b/apps/sim/lib/selectors/server/credentials.test.ts index 605232df8f5..17138d2079a 100644 --- a/apps/sim/lib/selectors/server/credentials.test.ts +++ b/apps/sim/lib/selectors/server/credentials.test.ts @@ -226,4 +226,34 @@ describe('resolveSelectorOAuthAccessToken', () => { expect(mocks.resolveCredentialTokenBundle).not.toHaveBeenCalled() }) + + it('rechecks cancellation before consuming a fulfilled credential result', async () => { + mocks.resolveCredentialTokenBundle.mockResolvedValue({ + accessToken: 'fulfilled-access-token', + }) + const controller = new AbortController() + const protectedValues = createSelectorProtectedValues() + const recordCredentialUse = vi.fn() + const abortReason = new DOMException('Selector request canceled', 'AbortError') + + const pending = resolveSelectorOAuthAccessToken({ + credential: { + suppliedId: 'credential-1', + access: { + ok: true, + credentialOwnerUserId: 'owner-1', + resolvedCredentialId: 'credential-1', + }, + signal: controller.signal, + }, + serviceId: 'gmail', + protectedValues, + recordCredentialUse, + }) + queueMicrotask(() => controller.abort(abortReason)) + + await expect(pending).rejects.toBe(abortReason) + expect(protectedValues.contains('fulfilled-access-token')).toBe(false) + expect(recordCredentialUse).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/selectors/server/credentials.ts b/apps/sim/lib/selectors/server/credentials.ts index 79e7c975642..2bc68c62275 100644 --- a/apps/sim/lib/selectors/server/credentials.ts +++ b/apps/sim/lib/selectors/server/credentials.ts @@ -170,6 +170,7 @@ export async function resolveSelectorOAuthAccessToken(input: { ), input.credential.signal ) + input.credential.signal?.throwIfAborted() const token = result?.accessToken if (!token) throw new SelectorConnectionUnavailableError() diff --git a/apps/sim/lib/selectors/server/providers/credential-bundle.test.ts b/apps/sim/lib/selectors/server/providers/credential-bundle.test.ts index 6dc5c5740bb..78607b9b596 100644 --- a/apps/sim/lib/selectors/server/providers/credential-bundle.test.ts +++ b/apps/sim/lib/selectors/server/providers/credential-bundle.test.ts @@ -70,4 +70,32 @@ describe('selector credential bundles', () => { { privacyMode: 'selector' } ) }) + + it('rechecks cancellation before consuming a fulfilled credential bundle', async () => { + mockResolveCredentialAccessToken.mockResolvedValue({ + accessToken: 'fulfilled-access-token', + cloudId: 'cloud-1', + }) + const controller = new AbortController() + const protectedValues = createSelectorProtectedValues() + const recordCredentialUse = vi.fn() + const abortReason = new DOMException('Selector request canceled', 'AbortError') + + const pending = resolveSelectorCredentialBundle({ + credential: { + suppliedId: 'credential-1', + access: { ok: true, credentialOwnerUserId: 'owner-1' }, + signal: controller.signal, + }, + protectedValues, + providerId: 'atlassian', + recordCredentialUse, + }) + queueMicrotask(() => controller.abort(abortReason)) + + await expect(pending).rejects.toBe(abortReason) + expect(protectedValues.contains('fulfilled-access-token')).toBe(false) + expect(protectedValues.contains('cloud-1')).toBe(false) + expect(recordCredentialUse).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/selectors/server/providers/credential-bundle.ts b/apps/sim/lib/selectors/server/providers/credential-bundle.ts index 027c0320d52..5ce91a2557e 100644 --- a/apps/sim/lib/selectors/server/providers/credential-bundle.ts +++ b/apps/sim/lib/selectors/server/providers/credential-bundle.ts @@ -48,6 +48,7 @@ export async function resolveSelectorCredentialBundle(input: { ), credential.signal ) + credential.signal?.throwIfAborted() } catch (error) { if (credential.signal?.aborted) throw error throw new SelectorConnectionUnavailableError()