Skip to content

Commit 0c4e674

Browse files
authored
perf(server): stop calling our own API over HTTP during render, execution, and tool runs (#6660)
* fix(prefetch): parse the file-folder seed through its contract The audit found this key was the workspaceFilesKeys bug waiting to recur. The manager's record type and workspaceFileFolderSchema are two independent declarations that agree today by coincidence; the seed had no parse, so adding a column to one would have silently cached a shape a client fetch strips — and three of its fields are z.coerce.date(), the exact divergence that put ISO strings under the file-list key. Its sibling is immune because listWorkspaceFilesWithShares parses at the data layer. This does the same at the seed, and adds the shape-parity assertion the key never had. Verified falsifiable: removing the parse turns it red. Doing so also exposed the existing folder test as fixture-thin — a folder with only an id, which the contract rightly rejects — so it now uses a real row. Also points the credential block's fetchQuery at the exported staleTime constant instead of restating 60 * 1000; it was a fifth producer on that key free to drift from the four that share it. * fix(queries): stop a table cell edit throwing, and an upload gate failing shut Two functional bugs found auditing the query layer. patchCachedRows walked tableKeys.rowsRoot non-exact, but rowsRoot is a prefix: the find (search results) and write (pending writes) subtrees hang off it with non-paged shapes, and the updater's old.pages.map threw on them. It runs inside onMutate, so the whole cell edit rejected before reaching the server — reachable as soon as a find entry exists, i.e. after the user searches the table once. The sibling isDefaultOrderRowsQuery already excluded those subtrees and its docstring claimed they "never match"; that was only true of the sibling. Both now share one isRowListQueryKey helper so they cannot drift apart again. useCloudStorageConfigured combined staleTime: Infinity, retry: false, and the global retryOnMount: false on a workspace-independent key, so one transient failure left it errored for the tab's lifetime with no way back — navigating or switching workspace cannot change the key, and the upload path fails closed, so cloud-backed uploads stayed disabled until a full reload. useVoiceSettings carries the same three options and already escapes this with retryOnMount: true; this one now matches. Note: hooks/queries/workspace-files.test.tsx cannot load in a git worktree (pre-existing postcss resolution failure), so CI is the first place that file runs against this change. * perf(server): memoize the request-scoped workspace and entitlement reads The workspace row was read ~3x per workspace route and ~5x on settings, and the same Max-tier entitlement was resolved twice on one render. Memoization is deliberately partial. getWorkspaceWithOwner accepts a transaction and forUpdate, and live callers use both, so only the plain no-options read routes through the memo; a row read inside one caller's transaction or under a lock it alone holds can never be served to a later caller. includeArchived is part of the key so the two variants cannot alias. Three substitutions were considered and rejected as behavior changes, not optimizations: hostContext.ownerBilling resolves subscriptions differently from hasWorkspaceTierAccess and exposes no Max tier, so it cannot answer the Inbox/Sandbox gates; isOrganizationOnEnterprisePlan carries self-host short-circuits ownerBilling has no equivalent for; and widening WorkspaceHostContext to carry the full row would push owner and org ids onto the wire for every viewer to save a server-side read, since that type is a response contract rather than an internal struct. * fix(selectors): key CloudWatch lists by search, and stop a caller erasing the credential gate The CloudWatch log-group and log-stream selectors forwarded `search` into the request as `prefix` but left it out of the query key, so every keystroke resolved to the same fresh entry and no refetch fired. Server-side filtering was dead: a log group outside the first page could not be reached. An audit of all 69 selector definitions found these two and no others. useSelectorOptions resolved `args.enabled ?? definition.enabled(...)`, so a caller supplying its own gate replaced the definition's precondition rather than narrowing it. useSelectorDisplayName knows nothing about credentials, so a card holding a saved value with no credential context ran a query that could only reject. The two are now conjoined. The detail hooks keep the override deliberately — resolving one known id needs less context than listing, which their TSDoc already documents. The list-key fix has a test, proven to fail without it. The `enabled` change has none: loading use-selector-query pulls the selector registry and emcn CSS, which cannot resolve in a git worktree. * fix(queries): give optimistic rows collision-free ids generateTempId used Date.now(), so two rows created in the same millisecond shared an id and the first server response overwrote both — leaving one row duplicated and the other's real id lost until a refetch. Now uses generateId(), matching what the workflow mutations already do. Reachable by double-clicking create, or by any scripted or bulk create. Also documents the contract of fetchOAuthConnections, which reports an unknown connection state as disconnected. No consumer reads that field today — both read names and icons, and connection state comes from useWorkspaceCredentials — so letting the query reject would blank the suggested-action rows and drop the credential page to raw provider ids. The note is what stops a future consumer branching on it silently. * fix(queries): close six stale-data gaps found auditing the query layer Each was verified against the mutation that changes the data and the keys that expose it, not taken on report. - Workspace usage/credits were invalidated nowhere in the app. Six sites already refreshed subscriptionKeys after credits moved — post-run, post-wand, limit edits, upgrades, top-ups — and none touched workspace usage, so the credits chip and the run gate held their page-load values until a reload. Adds one shared invalidateWorkspaceUsage and calls it from all six. - Knowledge-base list doc counts went stale: document upload, delete, and bulk delete invalidated only the detail key, though the list carries docCount. - Plan switches that do not redirect refreshed only the host context, leaving subscription and credit state showing the previous plan. - The copilot tool-event handler invalidated a raw workflowKeys.list, which covers only the active scope and skips the selector prefix; it now uses the shared invalidateWorkflowLists like the other thirteen call sites. - scheduleKeys.byId was a strict prefix of scheduleKeys.schedule, so the two addressings aliased, and nothing invalidated byId. De-aliased and invalidated. Not changed: the CSV preview key already folds in the file version and storage key, so a content update addresses a different cache entry — version-in-key is the mechanism there, not a missing invalidation. Tests added for the usage and knowledge fixes, both proven to fail without them. The other four live in files that cannot load in a git worktree (pre-existing postcss resolution failure), so CI is where they first run. * improvement(queries): make the row-list prefix non-collidable, and share the usage refresh Two corrections from reviewing the previous commits. patchCachedRows was fixed with a predicate naming the sibling subtrees to skip — a denylist that rots the moment a fifth subtree is added under rowsRoot. The key factory already separated row lists under an 'infinite' segment; it just had no prefix accessor, so every caller reached for the parent and subtracted. Adding infiniteRowsRoot lets the walk be an allowlist by construction and deletes the predicate, the helper, and both docblocks explaining the subtraction. The searched-rows view is consequently no longer patched by a cell edit and is left to its own refetch — it holds a flat result, not pages. That is recorded on the function rather than left to be rediscovered. The delayed usage refresh was written out three times across two files, a duplication the previous commit enlarged rather than introduced. It is now one scheduleUsageRefresh beside the keys it invalidates, which also gives the bare 1000ms a name and one place to change it. * improvement(prefetch): budget the workspace file seed and narrow its columns * improvement(api): resolve credentials and checkpoint reverts in-process * improvement(executor): run router and evaluator provider calls in-process * improvement(queries): fix a second row-cache collision, and make the memoized workspace read actually dedupe * chore(test): type the evaluator provider-request helper instead of using any * improvement(perf): restore the parallel file read, and close the gaps a diff audit surfaced * improvement(perf): drop a duplicate authorization, parallelize the credential reads, and trim the comments
1 parent 618cee5 commit 0c4e674

58 files changed

Lines changed: 2285 additions & 1242 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/sim/app/api/auth/oauth/token/route.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ vi.mock('@/lib/oauth/credential-service', () => ({
2424

2525
vi.mock('@/lib/auth/credential-access', () => ({
2626
authorizeCredentialUse: mockAuthorizeCredentialUse,
27+
authorizeCredentialUseForAuth: mockAuthorizeCredentialUse,
2728
}))
2829

2930
import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors'

apps/sim/app/api/auth/oauth/token/route.ts

Lines changed: 25 additions & 252 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,9 @@ import { authorizeCredentialUse } from '@/lib/auth/credential-access'
1111
import { AuthType, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
1212
import { generateRequestId } from '@/lib/core/utils/request'
1313
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
14-
import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors'
15-
import {
16-
getCredential,
17-
getOAuthToken,
18-
refreshTokenIfNeeded,
19-
resolveOAuthAccountId,
20-
resolveServiceAccountToken,
21-
} from '@/lib/oauth/credential-service'
22-
import { extractSalesforceInstanceUrl, isSalesforceOAuthProviderId } from '@/lib/oauth/salesforce'
14+
import { getCredential, getOAuthToken } from '@/lib/oauth/credential-service'
15+
import { completeOAuthCredentialToken, resolveCredentialToken } from '@/lib/oauth/token-resolution'
2316
import { captureServerEvent } from '@/lib/posthog/server'
24-
import { extractZohoDeskBaseFromScope } from '@/tools/zoho_desk/host-allowlist'
2517

2618
export const dynamic = 'force-dynamic'
2719

@@ -123,194 +115,25 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
123115
}
124116
}
125117

126-
if (!credentialId) {
127-
return NextResponse.json({ error: 'Credential ID is required' }, { status: 400 })
128-
}
129-
130-
const resolved = await resolveOAuthAccountId(credentialId)
131-
if (resolved?.credentialType === 'service_account' && resolved.credentialId) {
132-
const authz = await authorizeCredentialUse(request, {
133-
credentialId,
134-
workflowId: workflowId ?? undefined,
135-
requireWorkflowIdForInternal: false,
136-
callerUserId,
137-
})
138-
if (!authz.ok) {
139-
return NextResponse.json({ error: authz.error || 'Unauthorized' }, { status: 403 })
140-
}
141-
142-
const saActorId = authz.requesterUserId
143-
const saWorkspaceId = resolved.workspaceId ?? authz.workspaceId ?? null
144-
const emitServiceAccountAccess = () => {
145-
if (!saActorId) return
146-
recordAudit({
147-
workspaceId: saWorkspaceId,
148-
actorId: saActorId,
149-
action: AuditAction.CREDENTIAL_ACCESSED,
150-
resourceType: AuditResourceType.CREDENTIAL,
151-
resourceId: resolved.credentialId ?? credentialId,
152-
description: `Accessed service account credential for provider ${resolved.providerId ?? 'unknown'}`,
153-
metadata: {
154-
provider: resolved.providerId,
155-
credentialType: 'service_account',
156-
},
157-
request,
158-
})
159-
captureServerEvent(
160-
saActorId,
161-
'credential_used',
162-
{
163-
credential_type: 'service_account',
164-
provider_id: resolved.providerId ?? 'unknown',
165-
...(saWorkspaceId ? { workspace_id: saWorkspaceId } : {}),
166-
},
167-
saWorkspaceId ? { groups: { workspace: saWorkspaceId } } : undefined
168-
)
169-
}
170-
171-
try {
172-
const result = await resolveServiceAccountToken(
173-
resolved.credentialId,
174-
resolved.providerId,
175-
scopes ?? [],
176-
impersonateEmail
177-
)
178-
emitServiceAccountAccess()
179-
return NextResponse.json(
180-
{
181-
accessToken: result.accessToken,
182-
cloudId: result.cloudId,
183-
domain: result.domain,
184-
instanceUrl: result.instanceUrl,
185-
apiDomain: result.apiDomain,
186-
authStyle: result.authStyle,
187-
},
188-
{ status: 200 }
189-
)
190-
} catch (error) {
191-
logger.error(`[${requestId}] Service account token error:`, error)
192-
if (error instanceof TokenServiceAccountValidationError) {
193-
// Classified provider outages are infra failures, not bad credentials.
194-
if (error.code === 'provider_unavailable') {
195-
return NextResponse.json(
196-
{ error: 'Credential provider is temporarily unavailable' },
197-
{ status: 502 }
198-
)
199-
}
200-
// A stored host that no longer resolves is a configuration failure —
201-
// surface the code so runtime consumers can say "check the host"
202-
// instead of a generic auth error.
203-
if (error.code === 'site_not_found') {
204-
return NextResponse.json(
205-
{
206-
code: error.code,
207-
error: 'Credential host not found — reconnect the credential with a valid host',
208-
},
209-
{ status: 400 }
210-
)
211-
}
212-
// A revoked/rotated-away or misconfigured stored secret — surface the
213-
// code so runtime consumers can prompt to reconnect the credential
214-
// rather than showing a generic auth failure.
215-
if (error.code === 'invalid_credentials') {
216-
return NextResponse.json(
217-
{
218-
code: error.code,
219-
error: 'Credential rejected by the provider — reconnect the credential',
220-
},
221-
{ status: 401 }
222-
)
223-
}
224-
}
225-
return NextResponse.json({ error: 'Failed to get service account token' }, { status: 401 })
226-
}
227-
}
228-
229-
const authz = await authorizeCredentialUse(request, {
118+
const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
119+
const result = await resolveCredentialToken(auth, {
120+
requestId,
230121
credentialId,
231122
workflowId: workflowId ?? undefined,
232-
requireWorkflowIdForInternal: false,
123+
scopes,
124+
impersonateEmail,
233125
callerUserId,
126+
auditRequest: request,
234127
})
235-
if (!authz.ok || !authz.credentialOwnerUserId) {
236-
return NextResponse.json({ error: authz.error || 'Unauthorized' }, { status: 403 })
237-
}
238-
239-
const resolvedCredentialId = authz.resolvedCredentialId || credentialId
240-
const credential = await getCredential(
241-
requestId,
242-
resolvedCredentialId,
243-
authz.credentialOwnerUserId
244-
)
245-
246-
if (!credential) {
247-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
248-
}
249-
250-
const oauthActorId = authz.requesterUserId
251-
const oauthWorkspaceId = authz.workspaceId ?? null
252-
253-
try {
254-
const { accessToken } = await refreshTokenIfNeeded(
255-
requestId,
256-
credential,
257-
resolvedCredentialId
258-
)
259-
260-
if (oauthActorId) {
261-
recordAudit({
262-
workspaceId: oauthWorkspaceId,
263-
actorId: oauthActorId,
264-
action: AuditAction.CREDENTIAL_ACCESSED,
265-
resourceType: AuditResourceType.CREDENTIAL,
266-
resourceId: resolvedCredentialId,
267-
description: `Accessed OAuth credential for provider ${credential.providerId}`,
268-
metadata: {
269-
provider: credential.providerId,
270-
credentialType: 'oauth',
271-
},
272-
request,
273-
})
274-
captureServerEvent(
275-
oauthActorId,
276-
'credential_used',
277-
{
278-
credential_type: 'oauth',
279-
provider_id: credential.providerId,
280-
...(oauthWorkspaceId ? { workspace_id: oauthWorkspaceId } : {}),
281-
},
282-
oauthWorkspaceId ? { groups: { workspace: oauthWorkspaceId } } : undefined
283-
)
284-
}
285-
286-
const instanceUrl = isSalesforceOAuthProviderId(credential.providerId)
287-
? extractSalesforceInstanceUrl(credential.scope)
288-
: undefined
289-
290-
// Zoho Desk persists its data-center-specific REST base URL in the scope
291-
// string (derived from the token response api_domain) so callers never
292-
// assume a host. Surface it as apiDomain for tool param injection.
293-
let apiDomain: string | undefined
294-
if (credential.providerId === 'zoho-desk' && credential.scope) {
295-
// Use the shared extractor, not a local regex: it also enforces https +
296-
// the Zoho apex allowlist. This value is injected into EVERY tool call,
297-
// so an unvalidated host here would receive the OAuth token.
298-
apiDomain = extractZohoDeskBaseFromScope(credential.scope)
299-
}
300128

129+
if (!result.ok) {
301130
return NextResponse.json(
302-
{
303-
accessToken,
304-
idToken: credential.idToken || undefined,
305-
...(instanceUrl && { instanceUrl }),
306-
...(apiDomain && { apiDomain }),
307-
},
308-
{ status: 200 }
131+
{ ...(result.code ? { code: result.code } : {}), error: result.error },
132+
{ status: result.status }
309133
)
310-
} catch (error) {
311-
logger.error(`[${requestId}] Failed to refresh access token:`, error)
312-
return NextResponse.json({ error: 'Failed to refresh access token' }, { status: 401 })
313134
}
135+
136+
return NextResponse.json(result.token, { status: 200 })
314137
} catch (error) {
315138
logger.error(`[${requestId}] Error getting access token`, error)
316139
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
@@ -366,70 +189,20 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
366189
return NextResponse.json({ error: 'No access token available' }, { status: 400 })
367190
}
368191

369-
const actorId = authz.requesterUserId
370-
const workspaceId = authz.workspaceId ?? null
371-
372-
try {
373-
const { accessToken } = await refreshTokenIfNeeded(
374-
requestId,
375-
credential,
376-
resolvedCredentialId
377-
)
378-
379-
if (actorId) {
380-
recordAudit({
381-
workspaceId,
382-
actorId,
383-
action: AuditAction.CREDENTIAL_ACCESSED,
384-
resourceType: AuditResourceType.CREDENTIAL,
385-
resourceId: resolvedCredentialId,
386-
description: `Accessed OAuth credential for provider ${credential.providerId}`,
387-
metadata: {
388-
provider: credential.providerId,
389-
credentialType: 'oauth',
390-
},
391-
request,
392-
})
393-
captureServerEvent(
394-
actorId,
395-
'credential_used',
396-
{
397-
credential_type: 'oauth',
398-
provider_id: credential.providerId,
399-
...(workspaceId ? { workspace_id: workspaceId } : {}),
400-
},
401-
workspaceId ? { groups: { workspace: workspaceId } } : undefined
402-
)
403-
}
404-
405-
const instanceUrl = isSalesforceOAuthProviderId(credential.providerId)
406-
? extractSalesforceInstanceUrl(credential.scope)
407-
: undefined
408-
409-
// Zoho Desk persists its data-center-specific REST base URL in the scope
410-
// string (derived from the token response api_domain) so callers never
411-
// assume a host. Surface it as apiDomain for tool param injection.
412-
let apiDomain: string | undefined
413-
if (credential.providerId === 'zoho-desk' && credential.scope) {
414-
// Use the shared extractor, not a local regex: it also enforces https +
415-
// the Zoho apex allowlist. This value is injected into EVERY tool call,
416-
// so an unvalidated host here would receive the OAuth token.
417-
apiDomain = extractZohoDeskBaseFromScope(credential.scope)
418-
}
192+
const result = await completeOAuthCredentialToken({
193+
requestId,
194+
credential,
195+
resolvedCredentialId,
196+
actorId: authz.requesterUserId,
197+
workspaceId: authz.workspaceId ?? null,
198+
auditRequest: request,
199+
})
419200

420-
return NextResponse.json(
421-
{
422-
accessToken,
423-
idToken: credential.idToken || undefined,
424-
...(instanceUrl && { instanceUrl }),
425-
...(apiDomain && { apiDomain }),
426-
},
427-
{ status: 200 }
428-
)
429-
} catch (error) {
430-
logger.error(`[${requestId}] Failed to refresh access token:`, error)
431-
return NextResponse.json({ error: 'Failed to refresh access token' }, { status: 401 })
201+
if (!result.ok) {
202+
return NextResponse.json({ error: result.error }, { status: result.status })
432203
}
204+
205+
return NextResponse.json(result.token, { status: 200 })
433206
} catch (error) {
434207
logger.error(`[${requestId}] Error fetching access token`, error)
435208
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })

0 commit comments

Comments
 (0)