Skip to content

Commit a2a3fdd

Browse files
last_output count fixed, other nits addressed
1 parent 1af2375 commit a2a3fdd

File tree

4 files changed

+140
-52
lines changed

4 files changed

+140
-52
lines changed

apps/sim/executor/execution/block-executor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger, type Logger } from '@sim/logger'
22
import { redactApiKeys } from '@/lib/core/security/redaction'
3-
import { hydrateCacheReferences } from '@/lib/paginated-cache/paginate'
43
import { getBaseUrl } from '@/lib/core/utils/urls'
4+
import { hydrateCacheReferences } from '@/lib/paginated-cache/paginate'
55
import {
66
containsUserFileWithMetadata,
77
hydrateUserFilesWithBase64,

apps/sim/lib/paginated-cache/paginate.test.ts

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,7 @@ vi.mock('@/lib/paginated-cache/redis-cache', () => ({
2828
import { autoPaginate, hydrateCacheReferences } from '@/lib/paginated-cache/paginate'
2929
import type { ToolResponse } from '@/tools/types'
3030

31-
function makePageResponse(
32-
items: unknown[],
33-
hasMore: boolean,
34-
cursor: string | null
35-
): ToolResponse {
31+
function makePageResponse(items: unknown[], hasMore: boolean, cursor: string | null): ToolResponse {
3632
return {
3733
success: true,
3834
output: {
@@ -131,7 +127,7 @@ describe('autoPaginate', () => {
131127
expect.objectContaining({ _type: 'paginated_cache_ref', totalPages: 3, totalItems: 3 })
132128
)
133129
expect(result.output.paging).toEqual({ has_more: false, after_cursor: null })
134-
expect(result.output.metadata).toEqual({ total_returned: 1, has_more: false })
130+
expect(result.output.metadata).toEqual({ total_returned: 3, has_more: false })
135131
})
136132

137133
it('respects maxPages', async () => {
@@ -206,7 +202,61 @@ describe('autoPaginate', () => {
206202
})
207203

208204
const storedCacheId = mockStoreMetadata.mock.calls[0][0] as string
209-
expect(storedCacheId).toMatch(/^exec-42:zendesk_get_tickets:tickets:\d+$/)
205+
expect(storedCacheId).toMatch(
206+
/^exec-42:zendesk_get_tickets:tickets:[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/
207+
)
208+
})
209+
210+
it('patches metadata.total_returned with cumulative count', async () => {
211+
const initialResult = makePageResponse([{ id: 1 }], true, 'cursor-1')
212+
mockExecuteTool.mockResolvedValueOnce(makePageResponse([{ id: 2 }, { id: 3 }], false, null))
213+
214+
const result = await autoPaginate({
215+
initialResult,
216+
params: {},
217+
paginationConfig,
218+
executeTool: mockExecuteTool,
219+
toolId: 'zendesk_get_tickets',
220+
executionId: 'exec-1',
221+
})
222+
223+
expect(result.output.metadata).toEqual(
224+
expect.objectContaining({ total_returned: 3, has_more: false })
225+
)
226+
})
227+
})
228+
229+
describe('cleanupPaginatedCache', () => {
230+
let mockScan: ReturnType<typeof vi.fn>
231+
let mockDel: ReturnType<typeof vi.fn>
232+
233+
beforeEach(() => {
234+
vi.clearAllMocks()
235+
mockScan = vi.fn().mockResolvedValue(['0', []])
236+
mockDel = vi.fn().mockResolvedValue(1)
237+
mockGetRedisClient.mockReturnValue({ scan: mockScan, del: mockDel })
238+
})
239+
240+
it('scans with prefix-based patterns and deletes matching keys', async () => {
241+
mockScan
242+
.mockResolvedValueOnce(['0', ['pagcache:page:exec-1:tool:field:uuid:0']])
243+
.mockResolvedValueOnce(['0', ['pagcache:meta:exec-1:tool:field:uuid']])
244+
245+
const { cleanupPaginatedCache } = await import('@/lib/paginated-cache/paginate')
246+
await cleanupPaginatedCache('exec-1')
247+
248+
expect(mockScan).toHaveBeenCalledWith('0', 'MATCH', 'pagcache:page:exec-1:*', 'COUNT', 100)
249+
expect(mockScan).toHaveBeenCalledWith('0', 'MATCH', 'pagcache:meta:exec-1:*', 'COUNT', 100)
250+
expect(mockDel).toHaveBeenCalledTimes(2)
251+
})
252+
253+
it('no-ops when Redis is unavailable', async () => {
254+
mockGetRedisClient.mockReturnValue(null)
255+
256+
const { cleanupPaginatedCache } = await import('@/lib/paginated-cache/paginate')
257+
await cleanupPaginatedCache('exec-1')
258+
259+
expect(mockScan).not.toHaveBeenCalled()
210260
})
211261
})
212262

apps/sim/lib/paginated-cache/paginate.ts

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
import crypto from 'node:crypto'
12
import { createLogger } from '@sim/logger'
23
import { getRedisClient } from '@/lib/core/config/redis'
34
import { RedisPaginatedCache } from '@/lib/paginated-cache/redis-cache'
4-
import { isPaginatedCacheReference } from '@/lib/paginated-cache/types'
55
import type { PaginatedCacheReference, ToolPaginationConfig } from '@/lib/paginated-cache/types'
6+
import { isPaginatedCacheReference } from '@/lib/paginated-cache/types'
67
import type { ToolResponse } from '@/tools/types'
78

89
const logger = createLogger('Paginator')
@@ -23,8 +24,14 @@ interface AutoPaginateOptions {
2324
}
2425

2526
export async function autoPaginate(options: AutoPaginateOptions): Promise<ToolResponse> {
26-
const { initialResult, params, paginationConfig: config, executeTool, toolId, executionId } =
27-
options
27+
const {
28+
initialResult,
29+
params,
30+
paginationConfig: config,
31+
executeTool,
32+
toolId,
33+
executionId,
34+
} = options
2835
const maxPages = config.maxPages ?? DEFAULT_MAX_PAGES
2936

3037
const redis = getRedisClient()
@@ -33,7 +40,7 @@ export async function autoPaginate(options: AutoPaginateOptions): Promise<ToolRe
3340
}
3441

3542
const cache = new RedisPaginatedCache(redis)
36-
const cacheId = `${executionId}:${toolId}:${config.pageField}:${Date.now()}`
43+
const cacheId = `${executionId}:${toolId}:${config.pageField}:${crypto.randomUUID()}`
3744

3845
let totalItems = 0
3946
let pageIndex = 0
@@ -78,11 +85,18 @@ export async function autoPaginate(options: AutoPaginateOptions): Promise<ToolRe
7885

7986
logger.info('Auto-pagination complete', { cacheId, totalPages, totalItems, toolId })
8087

88+
const lastMeta = (lastOutput as Record<string, unknown>).metadata
89+
const patchedMetadata =
90+
typeof lastMeta === 'object' && lastMeta !== null
91+
? { ...lastMeta, total_returned: totalItems }
92+
: lastMeta
93+
8194
return {
8295
...initialResult,
8396
output: {
8497
...lastOutput,
8598
[config.pageField]: reference,
99+
...(patchedMetadata !== undefined && { metadata: patchedMetadata }),
86100
},
87101
}
88102
}
@@ -165,21 +179,23 @@ export async function cleanupPaginatedCache(executionId: string): Promise<void>
165179
return
166180
}
167181

168-
const pattern = `pagcache:*${executionId}:*`
182+
const patterns = [`pagcache:page:${executionId}:*`, `pagcache:meta:${executionId}:*`]
169183

170184
try {
171-
let cursor = '0'
172185
let deletedCount = 0
173186

174-
do {
175-
const [nextCursor, keys] = await redis.scan(cursor, 'MATCH', pattern, 'COUNT', 100)
176-
cursor = nextCursor
177-
178-
if (keys.length > 0) {
179-
await redis.del(...keys)
180-
deletedCount += keys.length
181-
}
182-
} while (cursor !== '0')
187+
for (const pattern of patterns) {
188+
let cursor = '0'
189+
do {
190+
const [nextCursor, keys] = await redis.scan(cursor, 'MATCH', pattern, 'COUNT', 100)
191+
cursor = nextCursor
192+
193+
if (keys.length > 0) {
194+
await redis.del(...keys)
195+
deletedCount += keys.length
196+
}
197+
} while (cursor !== '0')
198+
}
183199

184200
if (deletedCount > 0) {
185201
logger.info(`Cleaned up ${deletedCount} paginated cache entries for execution ${executionId}`)

apps/sim/tools/index.ts

Lines changed: 51 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
1313
import { getBaseUrl, getInternalApiBaseUrl } from '@/lib/core/utils/urls'
1414
import { SIM_VIA_HEADER, serializeCallChain } from '@/lib/execution/call-chain'
1515
import { parseMcpToolId } from '@/lib/mcp/utils'
16+
import { autoPaginate } from '@/lib/paginated-cache/paginate'
1617
import { isCustomTool, isMcpTool } from '@/executor/constants'
1718
import { resolveSkillContent } from '@/executor/handlers/agent/skills-resolver'
1819
import type { ExecutionContext } from '@/executor/types'
@@ -26,7 +27,6 @@ import type {
2627
ToolResponse,
2728
ToolRetryConfig,
2829
} from '@/tools/types'
29-
import { autoPaginate } from '@/lib/paginated-cache/paginate'
3030
import { formatRequestParams, getTool, validateRequiredParametersAfterMerge } from '@/tools/utils'
3131
import * as toolsUtilsServer from '@/tools/utils.server'
3232

@@ -600,6 +600,40 @@ async function processFileOutputs(
600600
}
601601
}
602602

603+
/**
604+
* If the tool has a pagination config and there are more pages, auto-paginate
605+
* and replace the page field with a Redis cache reference.
606+
*/
607+
async function maybeAutoPaginate(
608+
tool: ToolConfig,
609+
finalResult: ToolResponse,
610+
contextParams: Record<string, unknown>,
611+
normalizedToolId: string,
612+
skipPostProcess: boolean,
613+
executionContext?: ExecutionContext
614+
): Promise<ToolResponse> {
615+
if (
616+
!tool.pagination ||
617+
!finalResult.success ||
618+
skipPostProcess ||
619+
!executionContext?.executionId
620+
) {
621+
return finalResult
622+
}
623+
const nextToken = tool.pagination.getNextPageToken(finalResult.output)
624+
if (nextToken === null) {
625+
return finalResult
626+
}
627+
return autoPaginate({
628+
initialResult: finalResult,
629+
params: contextParams,
630+
paginationConfig: tool.pagination,
631+
executeTool,
632+
toolId: normalizedToolId,
633+
executionId: executionContext.executionId,
634+
})
635+
}
636+
603637
/**
604638
* Execute a tool by making the appropriate HTTP request
605639
* All requests go directly - internal routes use regular fetch, external use SSRF-protected fetch
@@ -820,20 +854,14 @@ export async function executeTool(
820854
// Process file outputs if execution context is available
821855
finalResult = await processFileOutputs(finalResult, tool, executionContext)
822856

823-
// Auto-paginate if tool has pagination config and there are more pages
824-
if (tool.pagination && finalResult.success && !skipPostProcess && executionContext?.executionId) {
825-
const nextToken = tool.pagination.getNextPageToken(finalResult.output)
826-
if (nextToken !== null) {
827-
finalResult = await autoPaginate({
828-
initialResult: finalResult,
829-
params: contextParams,
830-
paginationConfig: tool.pagination,
831-
executeTool,
832-
toolId: normalizedToolId,
833-
executionId: executionContext.executionId,
834-
})
835-
}
836-
}
857+
finalResult = await maybeAutoPaginate(
858+
tool,
859+
finalResult,
860+
contextParams,
861+
normalizedToolId,
862+
skipPostProcess,
863+
executionContext
864+
)
837865

838866
// Add timing data to the result
839867
const endTime = new Date()
@@ -890,20 +918,14 @@ export async function executeTool(
890918
// Process file outputs if execution context is available
891919
finalResult = await processFileOutputs(finalResult, tool, executionContext)
892920

893-
// Auto-paginate if tool has pagination config and there are more pages
894-
if (tool.pagination && finalResult.success && !skipPostProcess && executionContext?.executionId) {
895-
const nextToken = tool.pagination.getNextPageToken(finalResult.output)
896-
if (nextToken !== null) {
897-
finalResult = await autoPaginate({
898-
initialResult: finalResult,
899-
params: contextParams,
900-
paginationConfig: tool.pagination,
901-
executeTool,
902-
toolId: normalizedToolId,
903-
executionId: executionContext.executionId,
904-
})
905-
}
906-
}
921+
finalResult = await maybeAutoPaginate(
922+
tool,
923+
finalResult,
924+
contextParams,
925+
normalizedToolId,
926+
skipPostProcess,
927+
executionContext
928+
)
907929

908930
// Add timing data to the result
909931
const endTime = new Date()

0 commit comments

Comments
 (0)